Re: [Xen-devel] [PATCH v2 12/15] libxl: call hotplug scripts for disk devices from libxl

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("[PATCH v2 12/15] libxl: call hotplug scripts for disk 
devices from libxl"):
> Since most of the needed work is already done in previous patches,
> this patch only contains the necessary code to call hotplug scripts
> for disk devices, that should be called when the device is added or
> removed from a guest.
> 
> We will chain the launch of the disk hotplug scripts after the
> device_backend_callback callback, or directly from
> libxl__initiate_device_{add,remove} if the device is already in the
> desired state.
...
> +static void device_hotplug_timeout_cb(libxl__egc *egc, libxl__ev_time *ev,
> +                                      const struct timeval *requested_abs)
> +{
> +    libxl__ao_device *aodev = CONTAINER_OF(ev, *aodev, ev);
> +    STATE_AO_GC(aodev->ao);
> +
> +    if (!aodev) return;

Uh, what is this for ?  How can aodev be null ?

> +
> +/* Hotplug scripts helpers */
> +
> +static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
> +{
...

How about, to avoid mistakes with the array size:

       int arraysize = 9;
> +
> +    GCNEW_ARRAY(env, 9);
       GCNEW_ARRAY(env, arraysize);

> +    env[nr++] = "script";
> +    env[nr++] = script;
> +    env[nr++] = "XENBUS_TYPE";
> +    env[nr++] = libxl__strdup(gc, type);
> +    env[nr++] = "XENBUS_PATH";
> +    env[nr++] = GCSPRINTF("backend/%s/%u/%d", type, dev->domid, dev->devid);
> +    env[nr++] = "XENBUS_BASE_PATH";
> +    env[nr++] = "backend";
> +    env[nr++] = NULL;

       assert(nr == arraysize);

We should really have something better than this as there is a lot of
this kind of thing in libxl but now is not the time.

Ian.

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