| Message ID | 20200109114816.2293-5-pdurrant@amazon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xl/libxl: domid allocation/preservation changes | expand |
On Thu, Jan 9, 2020 at 6:50 AM Paul Durrant <pdurrant@amazon.com> wrote: > > This patch adds a 'domid' field to libxl_domain_create_info and then > modifies do_domain_create() to use that value if it is valid. Any valid > domid will be checked against the retired domid list before being passed > to libxl__domain_make(). > If the domid value is invalid then Xen will choose the domid, as before, > unless the value is the new special RANDOM_DOMID value added to the API. > This value instructs libxl__domain_make() to select a random domid value, > check it for validity, verify it does not match a retired domain, and then > pass it to Xen's XEN_DOMCTL_createdomain operation. If Xen determines that > it co-incides with an existing domain, a new random value will be > selected and the operation will be re-tried. > > NOTE: libxl__logv() is also modified to only log valid domid values in > messages rather than any domid, valid or otherwise, that is not > INVALID_DOMID. > > Signed-off-by: Paul Durrant <pdurrant@amazon.com> > --- > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Wei Liu <wl@xen.org> > Cc: Anthony PERARD <anthony.perard@citrix.com> > > v2: > - Re-worked to use a value from libxl_domain_create_info > --- > tools/libxl/libxl.h | 9 +++++++++ > tools/libxl/libxl_create.c | 32 +++++++++++++++++++++++++++++++- > tools/libxl/libxl_internal.c | 2 +- > tools/libxl/libxl_types.idl | 1 + > 4 files changed, 42 insertions(+), 2 deletions(-) > <snip> > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > index 1835a5502c..ee76dee364 100644 > --- a/tools/libxl/libxl_create.c > +++ b/tools/libxl/libxl_create.c > @@ -600,9 +600,39 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config, > goto out; > } > > - ret = xc_domain_create(ctx->xch, domid, &create); > + if (libxl_domid_valid_guest(info->domid)) { > + *domid = info->domid; > + > + if (libxl__is_retired_domid(gc, *domid)) { > + LOGED(ERROR, *domid, "domain id is retired"); > + rc = ERROR_FAIL; > + goto out; > + } > + } else if (info->domid == RANDOM_DOMID) { > + *domid = 0; /* Zero-out initial value */ > + } > + > + for (;;) { > + if (info->domid == RANDOM_DOMID) { > + /* Randomize lower order bytes */ > + ret = libxl__random_bytes(gc, (void *)domid, > + sizeof(uint16_t)); Casting to void * assumes little endian. Using a temporary uint16_t would avoid that assumption. Also, masking down to 0x7fff would clear the top bit which is never valid. Regards, Jason > + if (ret < 0) > + break; > + > + if (!libxl_domid_valid_guest(*domid) || > + libxl__is_retired_domid(gc, *domid)) > + continue; > + } > + > + ret = xc_domain_create(ctx->xch, domid, &create); > + if (ret == 0 || errno != EEXIST || info->domid != RANDOM_DOMID) > + break; > + } > + > if (ret < 0) { > LOGED(ERROR, *domid, "domain creation fail"); > + *domid = INVALID_DOMID; > rc = ERROR_FAIL; > goto out; > }
> -----Original Message----- > From: jandryuk@gmail.com <jandryuk@gmail.com> > Sent: 13 January 2020 16:16 > To: Durrant, Paul <pdurrant@amazon.co.uk> > Cc: xen-devel <xen-devel@lists.xenproject.org>; Anthony PERARD > <anthony.perard@citrix.com>; Ian Jackson <ian.jackson@eu.citrix.com>; Wei > Liu <wl@xen.org> > Subject: Re: [Xen-devel] [PATCH v2 4/6] libxl: allow creation of domains > with a specified or random domid > > On Thu, Jan 9, 2020 at 6:50 AM Paul Durrant <pdurrant@amazon.com> wrote: > > > > This patch adds a 'domid' field to libxl_domain_create_info and then > > modifies do_domain_create() to use that value if it is valid. Any valid > > domid will be checked against the retired domid list before being passed > > to libxl__domain_make(). > > If the domid value is invalid then Xen will choose the domid, as before, > > unless the value is the new special RANDOM_DOMID value added to the API. > > This value instructs libxl__domain_make() to select a random domid > value, > > check it for validity, verify it does not match a retired domain, and > then > > pass it to Xen's XEN_DOMCTL_createdomain operation. If Xen determines > that > > it co-incides with an existing domain, a new random value will be > > selected and the operation will be re-tried. > > > > NOTE: libxl__logv() is also modified to only log valid domid values in > > messages rather than any domid, valid or otherwise, that is not > > INVALID_DOMID. > > > > Signed-off-by: Paul Durrant <pdurrant@amazon.com> > > --- > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > > Cc: Wei Liu <wl@xen.org> > > Cc: Anthony PERARD <anthony.perard@citrix.com> > > > > v2: > > - Re-worked to use a value from libxl_domain_create_info > > --- > > tools/libxl/libxl.h | 9 +++++++++ > > tools/libxl/libxl_create.c | 32 +++++++++++++++++++++++++++++++- > > tools/libxl/libxl_internal.c | 2 +- > > tools/libxl/libxl_types.idl | 1 + > > 4 files changed, 42 insertions(+), 2 deletions(-) > > > > <snip> > > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > > index 1835a5502c..ee76dee364 100644 > > --- a/tools/libxl/libxl_create.c > > +++ b/tools/libxl/libxl_create.c > > @@ -600,9 +600,39 @@ int libxl__domain_make(libxl__gc *gc, > libxl_domain_config *d_config, > > goto out; > > } > > > > - ret = xc_domain_create(ctx->xch, domid, &create); > > + if (libxl_domid_valid_guest(info->domid)) { > > + *domid = info->domid; > > + > > + if (libxl__is_retired_domid(gc, *domid)) { > > + LOGED(ERROR, *domid, "domain id is retired"); > > + rc = ERROR_FAIL; > > + goto out; > > + } > > + } else if (info->domid == RANDOM_DOMID) { > > + *domid = 0; /* Zero-out initial value */ > > + } > > + > > + for (;;) { > > + if (info->domid == RANDOM_DOMID) { > > + /* Randomize lower order bytes */ > > + ret = libxl__random_bytes(gc, (void *)domid, > > + sizeof(uint16_t)); > > Casting to void * assumes little endian. I think that's a fairly safe assumption as far as Xen goes... > Using a temporary uint16_t ...but, yes, that might be neater. > would avoid that assumption. Also, masking down to 0x7fff would clear > the top bit which is never valid. That seems like a bit of a layering violation and the check in libxl_domid_valid_guest() is going to cause a pretty fast turn round the loop if the top bit is set so masking is not going to gain that much. Paul > > Regards, > Jason > > > + if (ret < 0) > > + break; > > + > > + if (!libxl_domid_valid_guest(*domid) || > > + libxl__is_retired_domid(gc, *domid)) > > + continue; > > + } > > + > > + ret = xc_domain_create(ctx->xch, domid, &create); > > + if (ret == 0 || errno != EEXIST || info->domid != > RANDOM_DOMID) > > + break; > > + } > > + > > if (ret < 0) { > > LOGED(ERROR, *domid, "domain creation fail"); > > + *domid = INVALID_DOMID; > > rc = ERROR_FAIL; > > goto out; > > }
On Mon, Jan 13, 2020 at 11:55 AM Durrant, Paul <pdurrant@amazon.co.uk> wrote: > > > -----Original Message----- > > From: jandryuk@gmail.com <jandryuk@gmail.com> > > Sent: 13 January 2020 16:16 > > To: Durrant, Paul <pdurrant@amazon.co.uk> > > Cc: xen-devel <xen-devel@lists.xenproject.org>; Anthony PERARD > > <anthony.perard@citrix.com>; Ian Jackson <ian.jackson@eu.citrix.com>; Wei > > Liu <wl@xen.org> > > Subject: Re: [Xen-devel] [PATCH v2 4/6] libxl: allow creation of domains > > with a specified or random domid > > > > On Thu, Jan 9, 2020 at 6:50 AM Paul Durrant <pdurrant@amazon.com> wrote: > > > > > > This patch adds a 'domid' field to libxl_domain_create_info and then > > > modifies do_domain_create() to use that value if it is valid. Any valid > > > domid will be checked against the retired domid list before being passed > > > to libxl__domain_make(). > > > If the domid value is invalid then Xen will choose the domid, as before, > > > unless the value is the new special RANDOM_DOMID value added to the API. > > > This value instructs libxl__domain_make() to select a random domid > > value, > > > check it for validity, verify it does not match a retired domain, and > > then > > > pass it to Xen's XEN_DOMCTL_createdomain operation. If Xen determines > > that > > > it co-incides with an existing domain, a new random value will be > > > selected and the operation will be re-tried. > > > > > > NOTE: libxl__logv() is also modified to only log valid domid values in > > > messages rather than any domid, valid or otherwise, that is not > > > INVALID_DOMID. > > > > > > Signed-off-by: Paul Durrant <pdurrant@amazon.com> > > > --- > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > > > Cc: Wei Liu <wl@xen.org> > > > Cc: Anthony PERARD <anthony.perard@citrix.com> > > > > > > v2: > > > - Re-worked to use a value from libxl_domain_create_info > > > --- > > > tools/libxl/libxl.h | 9 +++++++++ > > > tools/libxl/libxl_create.c | 32 +++++++++++++++++++++++++++++++- > > > tools/libxl/libxl_internal.c | 2 +- > > > tools/libxl/libxl_types.idl | 1 + > > > 4 files changed, 42 insertions(+), 2 deletions(-) > > > > > > > <snip> > > > > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > > > index 1835a5502c..ee76dee364 100644 > > > --- a/tools/libxl/libxl_create.c > > > +++ b/tools/libxl/libxl_create.c > > > @@ -600,9 +600,39 @@ int libxl__domain_make(libxl__gc *gc, > > libxl_domain_config *d_config, > > > goto out; > > > } > > > > > > - ret = xc_domain_create(ctx->xch, domid, &create); > > > + if (libxl_domid_valid_guest(info->domid)) { > > > + *domid = info->domid; > > > + > > > + if (libxl__is_retired_domid(gc, *domid)) { > > > + LOGED(ERROR, *domid, "domain id is retired"); > > > + rc = ERROR_FAIL; > > > + goto out; > > > + } > > > + } else if (info->domid == RANDOM_DOMID) { > > > + *domid = 0; /* Zero-out initial value */ > > > + } > > > + > > > + for (;;) { > > > + if (info->domid == RANDOM_DOMID) { > > > + /* Randomize lower order bytes */ > > > + ret = libxl__random_bytes(gc, (void *)domid, > > > + sizeof(uint16_t)); > > > > Casting to void * assumes little endian. > > I think that's a fairly safe assumption as far as Xen goes... > > > Using a temporary uint16_t > > ...but, yes, that might be neater. > > > would avoid that assumption. Also, masking down to 0x7fff would clear > > the top bit which is never valid. > > That seems like a bit of a layering violation and the check in libxl_domid_valid_guest() is going to cause a pretty fast turn round the loop if the top bit is set so masking is not going to gain that much. Yeah, there isn't a define or constant exposed for 0x7fff, so masking is a little dirty. Since about ~half of random 16bit numbers will have the high bit set, we'll have to read a second one. My natural instinct is to avoid those extra reads :) Regards, Jason
Hi Paul, On 13/01/2020 16:54, Durrant, Paul wrote: >> -----Original Message----- >> From: jandryuk@gmail.com <jandryuk@gmail.com> >> Sent: 13 January 2020 16:16 >> To: Durrant, Paul <pdurrant@amazon.co.uk> >> Cc: xen-devel <xen-devel@lists.xenproject.org>; Anthony PERARD >> <anthony.perard@citrix.com>; Ian Jackson <ian.jackson@eu.citrix.com>; Wei >> Liu <wl@xen.org> >> Subject: Re: [Xen-devel] [PATCH v2 4/6] libxl: allow creation of domains >> with a specified or random domid >> >> On Thu, Jan 9, 2020 at 6:50 AM Paul Durrant <pdurrant@amazon.com> wrote: >>> >>> This patch adds a 'domid' field to libxl_domain_create_info and then >>> modifies do_domain_create() to use that value if it is valid. Any valid >>> domid will be checked against the retired domid list before being passed >>> to libxl__domain_make(). >>> If the domid value is invalid then Xen will choose the domid, as before, >>> unless the value is the new special RANDOM_DOMID value added to the API. >>> This value instructs libxl__domain_make() to select a random domid >> value, >>> check it for validity, verify it does not match a retired domain, and >> then >>> pass it to Xen's XEN_DOMCTL_createdomain operation. If Xen determines >> that >>> it co-incides with an existing domain, a new random value will be >>> selected and the operation will be re-tried. >>> >>> NOTE: libxl__logv() is also modified to only log valid domid values in >>> messages rather than any domid, valid or otherwise, that is not >>> INVALID_DOMID. >>> >>> Signed-off-by: Paul Durrant <pdurrant@amazon.com> >>> --- >>> Cc: Ian Jackson <ian.jackson@eu.citrix.com> >>> Cc: Wei Liu <wl@xen.org> >>> Cc: Anthony PERARD <anthony.perard@citrix.com> >>> >>> v2: >>> - Re-worked to use a value from libxl_domain_create_info >>> --- >>> tools/libxl/libxl.h | 9 +++++++++ >>> tools/libxl/libxl_create.c | 32 +++++++++++++++++++++++++++++++- >>> tools/libxl/libxl_internal.c | 2 +- >>> tools/libxl/libxl_types.idl | 1 + >>> 4 files changed, 42 insertions(+), 2 deletions(-) >>> >> >> <snip> >> >>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c >>> index 1835a5502c..ee76dee364 100644 >>> --- a/tools/libxl/libxl_create.c >>> +++ b/tools/libxl/libxl_create.c >>> @@ -600,9 +600,39 @@ int libxl__domain_make(libxl__gc *gc, >> libxl_domain_config *d_config, >>> goto out; >>> } >>> >>> - ret = xc_domain_create(ctx->xch, domid, &create); >>> + if (libxl_domid_valid_guest(info->domid)) { >>> + *domid = info->domid; >>> + >>> + if (libxl__is_retired_domid(gc, *domid)) { >>> + LOGED(ERROR, *domid, "domain id is retired"); >>> + rc = ERROR_FAIL; >>> + goto out; >>> + } >>> + } else if (info->domid == RANDOM_DOMID) { >>> + *domid = 0; /* Zero-out initial value */ >>> + } >>> + >>> + for (;;) { >>> + if (info->domid == RANDOM_DOMID) { >>> + /* Randomize lower order bytes */ >>> + ret = libxl__random_bytes(gc, (void *)domid, >>> + sizeof(uint16_t)); >> >> Casting to void * assumes little endian. > > I think that's a fairly safe assumption as far as Xen goes... Not really, there are technically nothing (other than bug fixes) preventing us to use a big endian guest on Xen on Arm. I actually did play with big endian on Xen in the past and managed to get a guest running. The main annoying part is Linux as it is assuming to use the same endian as the hypervisor. But other OS may not have this issues... The hypervisor itself is likely going to stay little endian, so does the interface. For the tools, we should aim to not introduce more assumption that the software will be little endian. Cheers,
> -----Original Message----- > From: Julien Grall <julien@xen.org> > Sent: 13 January 2020 22:24 > To: Durrant, Paul <pdurrant@amazon.co.uk>; jandryuk@gmail.com > Cc: Anthony PERARD <anthony.perard@citrix.com>; xen-devel <xen- > devel@lists.xenproject.org>; Ian Jackson <ian.jackson@eu.citrix.com>; Wei > Liu <wl@xen.org> > Subject: Re: [Xen-devel] [PATCH v2 4/6] libxl: allow creation of domains > with a specified or random domid > > Hi Paul, > > On 13/01/2020 16:54, Durrant, Paul wrote: > >> -----Original Message----- > >> From: jandryuk@gmail.com <jandryuk@gmail.com> > >> Sent: 13 January 2020 16:16 > >> To: Durrant, Paul <pdurrant@amazon.co.uk> > >> Cc: xen-devel <xen-devel@lists.xenproject.org>; Anthony PERARD > >> <anthony.perard@citrix.com>; Ian Jackson <ian.jackson@eu.citrix.com>; > Wei > >> Liu <wl@xen.org> > >> Subject: Re: [Xen-devel] [PATCH v2 4/6] libxl: allow creation of > domains > >> with a specified or random domid > >> > >> On Thu, Jan 9, 2020 at 6:50 AM Paul Durrant <pdurrant@amazon.com> > wrote: > >>> > >>> This patch adds a 'domid' field to libxl_domain_create_info and then > >>> modifies do_domain_create() to use that value if it is valid. Any > valid > >>> domid will be checked against the retired domid list before being > passed > >>> to libxl__domain_make(). > >>> If the domid value is invalid then Xen will choose the domid, as > before, > >>> unless the value is the new special RANDOM_DOMID value added to the > API. > >>> This value instructs libxl__domain_make() to select a random domid > >> value, > >>> check it for validity, verify it does not match a retired domain, and > >> then > >>> pass it to Xen's XEN_DOMCTL_createdomain operation. If Xen determines > >> that > >>> it co-incides with an existing domain, a new random value will be > >>> selected and the operation will be re-tried. > >>> > >>> NOTE: libxl__logv() is also modified to only log valid domid values in > >>> messages rather than any domid, valid or otherwise, that is not > >>> INVALID_DOMID. > >>> > >>> Signed-off-by: Paul Durrant <pdurrant@amazon.com> > >>> --- > >>> Cc: Ian Jackson <ian.jackson@eu.citrix.com> > >>> Cc: Wei Liu <wl@xen.org> > >>> Cc: Anthony PERARD <anthony.perard@citrix.com> > >>> > >>> v2: > >>> - Re-worked to use a value from libxl_domain_create_info > >>> --- > >>> tools/libxl/libxl.h | 9 +++++++++ > >>> tools/libxl/libxl_create.c | 32 +++++++++++++++++++++++++++++++- > >>> tools/libxl/libxl_internal.c | 2 +- > >>> tools/libxl/libxl_types.idl | 1 + > >>> 4 files changed, 42 insertions(+), 2 deletions(-) > >>> > >> > >> <snip> > >> > >>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > >>> index 1835a5502c..ee76dee364 100644 > >>> --- a/tools/libxl/libxl_create.c > >>> +++ b/tools/libxl/libxl_create.c > >>> @@ -600,9 +600,39 @@ int libxl__domain_make(libxl__gc *gc, > >> libxl_domain_config *d_config, > >>> goto out; > >>> } > >>> > >>> - ret = xc_domain_create(ctx->xch, domid, &create); > >>> + if (libxl_domid_valid_guest(info->domid)) { > >>> + *domid = info->domid; > >>> + > >>> + if (libxl__is_retired_domid(gc, *domid)) { > >>> + LOGED(ERROR, *domid, "domain id is retired"); > >>> + rc = ERROR_FAIL; > >>> + goto out; > >>> + } > >>> + } else if (info->domid == RANDOM_DOMID) { > >>> + *domid = 0; /* Zero-out initial value */ > >>> + } > >>> + > >>> + for (;;) { > >>> + if (info->domid == RANDOM_DOMID) { > >>> + /* Randomize lower order bytes */ > >>> + ret = libxl__random_bytes(gc, (void *)domid, > >>> + sizeof(uint16_t)); > >> > >> Casting to void * assumes little endian. > > > > I think that's a fairly safe assumption as far as Xen goes... > > Not really, there are technically nothing (other than bug fixes) > preventing us to use a big endian guest on Xen on Arm. > Ok. > I actually did play with big endian on Xen in the past and managed to > get a guest running. The main annoying part is Linux as it is assuming > to use the same endian as the hypervisor. But other OS may not have this > issues... > > The hypervisor itself is likely going to stay little endian, so does the > interface. For the tools, we should aim to not introduce more assumption > that the software will be little endian. > Fair enough. If there's a realistic possibility of running a BE tools domain then I'll code accordingly. Paul > Cheers, > > -- > Julien Grall
> -----Original Message----- > From: jandryuk@gmail.com <jandryuk@gmail.com> > Sent: 13 January 2020 19:35 > To: Durrant, Paul <pdurrant@amazon.co.uk> > Cc: xen-devel <xen-devel@lists.xenproject.org>; Anthony PERARD > <anthony.perard@citrix.com>; Ian Jackson <ian.jackson@eu.citrix.com>; Wei > Liu <wl@xen.org> > Subject: Re: [Xen-devel] [PATCH v2 4/6] libxl: allow creation of domains > with a specified or random domid > > On Mon, Jan 13, 2020 at 11:55 AM Durrant, Paul <pdurrant@amazon.co.uk> > wrote: > > > > > -----Original Message----- > > > From: jandryuk@gmail.com <jandryuk@gmail.com> > > > Sent: 13 January 2020 16:16 > > > To: Durrant, Paul <pdurrant@amazon.co.uk> > > > Cc: xen-devel <xen-devel@lists.xenproject.org>; Anthony PERARD > > > <anthony.perard@citrix.com>; Ian Jackson <ian.jackson@eu.citrix.com>; > Wei > > > Liu <wl@xen.org> > > > Subject: Re: [Xen-devel] [PATCH v2 4/6] libxl: allow creation of > domains > > > with a specified or random domid > > > > > > On Thu, Jan 9, 2020 at 6:50 AM Paul Durrant <pdurrant@amazon.com> > wrote: > > > > > > > > This patch adds a 'domid' field to libxl_domain_create_info and then > > > > modifies do_domain_create() to use that value if it is valid. Any > valid > > > > domid will be checked against the retired domid list before being > passed > > > > to libxl__domain_make(). > > > > If the domid value is invalid then Xen will choose the domid, as > before, > > > > unless the value is the new special RANDOM_DOMID value added to the > API. > > > > This value instructs libxl__domain_make() to select a random domid > > > value, > > > > check it for validity, verify it does not match a retired domain, > and > > > then > > > > pass it to Xen's XEN_DOMCTL_createdomain operation. If Xen > determines > > > that > > > > it co-incides with an existing domain, a new random value will be > > > > selected and the operation will be re-tried. > > > > > > > > NOTE: libxl__logv() is also modified to only log valid domid values > in > > > > messages rather than any domid, valid or otherwise, that is > not > > > > INVALID_DOMID. > > > > > > > > Signed-off-by: Paul Durrant <pdurrant@amazon.com> > > > > --- > > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > > > > Cc: Wei Liu <wl@xen.org> > > > > Cc: Anthony PERARD <anthony.perard@citrix.com> > > > > > > > > v2: > > > > - Re-worked to use a value from libxl_domain_create_info > > > > --- > > > > tools/libxl/libxl.h | 9 +++++++++ > > > > tools/libxl/libxl_create.c | 32 +++++++++++++++++++++++++++++++- > > > > tools/libxl/libxl_internal.c | 2 +- > > > > tools/libxl/libxl_types.idl | 1 + > > > > 4 files changed, 42 insertions(+), 2 deletions(-) > > > > > > > > > > <snip> > > > > > > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > > > > index 1835a5502c..ee76dee364 100644 > > > > --- a/tools/libxl/libxl_create.c > > > > +++ b/tools/libxl/libxl_create.c > > > > @@ -600,9 +600,39 @@ int libxl__domain_make(libxl__gc *gc, > > > libxl_domain_config *d_config, > > > > goto out; > > > > } > > > > > > > > - ret = xc_domain_create(ctx->xch, domid, &create); > > > > + if (libxl_domid_valid_guest(info->domid)) { > > > > + *domid = info->domid; > > > > + > > > > + if (libxl__is_retired_domid(gc, *domid)) { > > > > + LOGED(ERROR, *domid, "domain id is retired"); > > > > + rc = ERROR_FAIL; > > > > + goto out; > > > > + } > > > > + } else if (info->domid == RANDOM_DOMID) { > > > > + *domid = 0; /* Zero-out initial value */ > > > > + } > > > > + > > > > + for (;;) { > > > > + if (info->domid == RANDOM_DOMID) { > > > > + /* Randomize lower order bytes */ > > > > + ret = libxl__random_bytes(gc, (void *)domid, > > > > + sizeof(uint16_t)); > > > > > > Casting to void * assumes little endian. > > > > I think that's a fairly safe assumption as far as Xen goes... > > > > > Using a temporary uint16_t > > > > ...but, yes, that might be neater. > > > > > would avoid that assumption. Also, masking down to 0x7fff would clear > > > the top bit which is never valid. > > > > That seems like a bit of a layering violation and the check in > libxl_domid_valid_guest() is going to cause a pretty fast turn round the > loop if the top bit is set so masking is not going to gain that much. > > Yeah, there isn't a define or constant exposed for 0x7fff, so masking > is a little dirty. Since about ~half of random 16bit numbers will > have the high bit set, we'll have to read a second one. My natural > instinct is to avoid those extra reads :) > Perhaps I should try adding a DOMID_MASK definition somewhere. Paul > Regards, > Jason
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 18c1a2d6bf..7e60ee1c8b 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -1268,6 +1268,14 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src); */ #define LIBXL_HAVE_DOMAIN_NEED_MEMORY_CONFIG +/* + * LIBXL_HAVE_CREATEINFO_DOMID + * + * libxl_domain_create_new() and libxl_domain_create_restore() will use + * a domid specified in libxl_domain_create_info(). + */ +#define LIBXL_HAVE_CREATEINFO_DOMID + typedef char **libxl_string_list; void libxl_string_list_dispose(libxl_string_list *sl); int libxl_string_list_length(const libxl_string_list *sl); @@ -1528,6 +1536,7 @@ int libxl_ctx_free(libxl_ctx *ctx /* 0 is OK */); /* domain related functions */ #define INVALID_DOMID ~0 +#define RANDOM_DOMID (INVALID_DOMID - 1) /* If the result is ERROR_ABORTED, the domain may or may not exist * (in a half-created state). *domid will be valid and will be the diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 1835a5502c..ee76dee364 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -600,9 +600,39 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config, goto out; } - ret = xc_domain_create(ctx->xch, domid, &create); + if (libxl_domid_valid_guest(info->domid)) { + *domid = info->domid; + + if (libxl__is_retired_domid(gc, *domid)) { + LOGED(ERROR, *domid, "domain id is retired"); + rc = ERROR_FAIL; + goto out; + } + } else if (info->domid == RANDOM_DOMID) { + *domid = 0; /* Zero-out initial value */ + } + + for (;;) { + if (info->domid == RANDOM_DOMID) { + /* Randomize lower order bytes */ + ret = libxl__random_bytes(gc, (void *)domid, + sizeof(uint16_t)); + if (ret < 0) + break; + + if (!libxl_domid_valid_guest(*domid) || + libxl__is_retired_domid(gc, *domid)) + continue; + } + + ret = xc_domain_create(ctx->xch, domid, &create); + if (ret == 0 || errno != EEXIST || info->domid != RANDOM_DOMID) + break; + } + if (ret < 0) { LOGED(ERROR, *domid, "domain creation fail"); + *domid = INVALID_DOMID; rc = ERROR_FAIL; goto out; } diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c index ba5637358e..dc6aaa9c9f 100644 --- a/tools/libxl/libxl_internal.c +++ b/tools/libxl/libxl_internal.c @@ -234,7 +234,7 @@ void libxl__logv(libxl_ctx *ctx, xentoollog_level msglevel, int errnoval, fileline[sizeof(fileline)-1] = 0; domain[0] = 0; - if (domid != INVALID_DOMID) + if (libxl_domid_valid_guest(domid)) snprintf(domain, sizeof(domain), "Domain %"PRIu32":", domid); x: xtl_log(ctx->lg, msglevel, errnoval, "libxl", diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 7921950f6a..d0d431614f 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -409,6 +409,7 @@ libxl_domain_create_info = Struct("domain_create_info",[ ("ssidref", uint32), ("ssid_label", string), ("name", string), + ("domid", libxl_domid), ("uuid", libxl_uuid), ("xsdata", libxl_key_value_list), ("platformdata", libxl_key_value_list),
This patch adds a 'domid' field to libxl_domain_create_info and then modifies do_domain_create() to use that value if it is valid. Any valid domid will be checked against the retired domid list before being passed to libxl__domain_make(). If the domid value is invalid then Xen will choose the domid, as before, unless the value is the new special RANDOM_DOMID value added to the API. This value instructs libxl__domain_make() to select a random domid value, check it for validity, verify it does not match a retired domain, and then pass it to Xen's XEN_DOMCTL_createdomain operation. If Xen determines that it co-incides with an existing domain, a new random value will be selected and the operation will be re-tried. NOTE: libxl__logv() is also modified to only log valid domid values in messages rather than any domid, valid or otherwise, that is not INVALID_DOMID. Signed-off-by: Paul Durrant <pdurrant@amazon.com> --- Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Wei Liu <wl@xen.org> Cc: Anthony PERARD <anthony.perard@citrix.com> v2: - Re-worked to use a value from libxl_domain_create_info --- tools/libxl/libxl.h | 9 +++++++++ tools/libxl/libxl_create.c | 32 +++++++++++++++++++++++++++++++- tools/libxl/libxl_internal.c | 2 +- tools/libxl/libxl_types.idl | 1 + 4 files changed, 42 insertions(+), 2 deletions(-)