Re: [Xen-devel] [PATCH v3 5/7] libxl: introduce libxl__alloc_vdev

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Stefano Stabellini writes ("[Xen-devel] [PATCH v3 5/7] libxl: introduce 
libxl__alloc_vdev"):
> +static char * libxl__alloc_vdev(libxl__gc *gc, uint32_t domid,
> +        char *blkdev_start, xs_transaction_t t)
> +{
> +    int devid = 0;
> +    char *vdev = libxl__strdup(gc, blkdev_start);
> +    char *dompath = libxl__xs_get_dompath(gc, domid);
> +
> +    do {
> +        devid = libxl__device_disk_dev_number(vdev, NULL, NULL);
> +        if (libxl__xs_read(gc, t,
> +                    libxl__sprintf(gc, "%s/device/vbd/%d/backend",
> +                        dompath, devid)) == NULL)
> +            return libxl__devid_to_vdev(gc, devid);

What if the error is not ENOENT ?

> +        vdev[strlen(vdev) - 1]++;
> +    } while(vdev[strlen(vdev) - 1] <= 'z');
              ^
Missing whitespace.

> +_hidden char *libxl__devid_to_vdev(libxl__gc *gc, int devid);

I think the terminology here needs to be corrected.  "vdev" is the
virtual device name supplied to libxl - a symbolic way of specifying a
devid in a guest-independent way.

Whereas what we actually mean is the non-portable name in this guest
OS for a device.  How about
  libxl__devid_to_localdev
?

> +static char *encode_disk_name(char *ptr, unsigned int n)

There is no clearly defined upper bound on the buffer space needed by
this function.

> diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c
> index 9e0ed6d..c8977ac 100644
> --- a/tools/libxl/libxl_netbsd.c
> +++ b/tools/libxl/libxl_netbsd.c
...
> +char *libxl__devid_to_vdev(libxl__gc *gc, int devid)
> +{
> +    /* TODO */
> +    return NULL;
> +}

I guess this is going to be fixed in a future version of the patch ?

Ian.


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