Re: [Xen-devel] [PATCH v2 4/6] libxl: allow creation of domains with a specified or random domid

["Durrant, Paul" <pdurrant@xxxxxxxxxxxx>]

> -----Original Message-----
> From: jandryuk@xxxxxxxxx <jandryuk@xxxxxxxxx>
> Sent: 13 January 2020 16:16
> To: Durrant, Paul <pdurrant@xxxxxxxxxxxx>
> Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>; Anthony PERARD
> <anthony.perard@xxxxxxxxxx>; Ian Jackson <ian.jackson@xxxxxxxxxxxxx>; Wei
> Liu <wl@xxxxxxx>
> 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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Wei Liu <wl@xxxxxxx>
> > Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> >
> > 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;
> >          }
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel