Re: [Xen-devel] [PATCH v1 01/12] libxl/hotplug: add support for getting domid

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-10-02 at 10:36 +0100, Andrew Cooper wrote:
> On 02/10/13 10:24, Roger Pau Monne wrote:
> > This patch writes Dom0 domid on xenstore (like it's done for other
> > guests), and adds a libxl helper function to fetch that domid from
> > xenstore.
> >
> > Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
> > Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> > ---
> >  tools/hotplug/Linux/init.d/xencommons |    1 +
> >  tools/libxl/libxl.c                   |   17 +++++++++++++++++
> >  tools/libxl/libxl_internal.h          |    3 +++
> >  3 files changed, 21 insertions(+), 0 deletions(-)
> >
> > diff --git a/tools/hotplug/Linux/init.d/xencommons 
> > b/tools/hotplug/Linux/init.d/xencommons
> > index a2e633b..43e09bc 100644
> > --- a/tools/hotplug/Linux/init.d/xencommons
> > +++ b/tools/hotplug/Linux/init.d/xencommons
> > @@ -110,6 +110,7 @@ do_start () {
> >  
> >             echo Setting domain 0 name...
> >             ${BINDIR}/xenstore-write "/local/domain/0/name" "Domain-0"
> > +           ${BINDIR}/xenstore-write "/local/domain/0/domid" "0"
> >     fi
> >  
> >     echo Starting xenconsoled...
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index 1bce4bb..5417b48 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -1688,6 +1688,23 @@ int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t 
> > domid, int autopass)
> >      return ERROR_FAIL;
> >  }
> >  
> > +int libxl__get_domid(libxl__gc *gc, uint32_t *domid)
> > +{
> > +    int rc;
> > +    const char *xs_domid;
> > +
> > +    rc = libxl__xs_read_checked(gc, XBT_NULL, DOMID_XS_PATH, &xs_domid);
> > +    if (rc || !xs_domid) {
> 
> NULL domid check should be done before xenstore read, as an xs read is
> quite a long operation.

xs_domid is the result of the xs_read, not a precursor.

> > +        rc = rc ? rc : ERROR_FAIL;
> > +        goto out;
> > +    }
> > +
> > +    *domid = atoi(xs_domid);
> 
> atoi() lacks any error reporting.  Use strtoul() and perhaps even a
> sanity check on the return value against the valid bounds for a domid.

We use atoi quite a lot and this is reading a toolstack controlled value
I think, so does it matter that much?
> 
> ~Andrew
> 
> > +
> > +out:
> > +    return rc;
> > +}
> > +
> >  
> > /******************************************************************************/
> >  
> >  /* generic callback for devices that only need to set ao_complete */
> > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> > index f051d91..3b74726 100644
> > --- a/tools/libxl/libxl_internal.h
> > +++ b/tools/libxl/libxl_internal.h
> > @@ -101,6 +101,7 @@
> >  #define STUBDOM_SPECIAL_CONSOLES 3
> >  #define TAP_DEVICE_SUFFIX "-emu"
> >  #define DISABLE_UDEV_PATH "libxl/disable_udev"
> > +#define DOMID_XS_PATH "domid"
> >  
> >  #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
> >  
> > @@ -978,6 +979,8 @@ _hidden const char *libxl__device_nic_devname(libxl__gc 
> > *gc,
> >                                                uint32_t devid,
> >                                                libxl_nic_type type);
> >  
> > +_hidden int libxl__get_domid(libxl__gc *gc, uint32_t *domid);
> > +
> >  /*
> >   * libxl__ev_devstate - waits a given time for a device to
> >   * reach a given state.  Follows the libxl_ev_* conventions.
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel