Re: [Xen-devel] [PATCH v5 08/10] libxl: call hotplug scripts for disk devices from libxl

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("[PATCH v5 08/10] 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.
...

> +static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev)
...
> +out:
> +    libxl__ev_time_deregister(gc, &aodev->ev);
> +    aodev->rc = rc;
> +    aodev->callback(egc, aodev);

This little set of things - deregistering the timeout, and calling the
callback, occur a few times.  It would also be a good idea to check
that the child has been reaped, or we may reenter this confusingly
later.

Perhaps it would be a good idea to have:

  static void device_hotplug_done(blah blah)
  {
      libxl__ev_time_deregister(gc, &aodev->ev);
      assert(!libxl__ev_child_inuse(&aodev->child));
      aodev->callback(egc, aodev);
  }

And this could be used on all of the completion paths.  (You'll have
to initialise the ev_child at the top of device_hotplug of course.)

That way the whole thing has one entry and one exit.

> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 5a2a87a..9dd2404 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -72,6 +72,7 @@
> 
>  #define LIBXL_INIT_TIMEOUT 10
>  #define LIBXL_DESTROY_TIMEOUT 10
> +#define LIBXL_HOTPLUG_TIMEOUT 10
>  #define LIBXL_DEVICE_MODEL_START_TIMEOUT 10
>  #define LIBXL_XENCONSOLE_LIMIT 1048576
>  #define LIBXL_XENCONSOLE_PROTOCOL "vt100"
> @@ -1846,6 +1847,10 @@ struct libxl__ao_device {
>      int rc;
>      libxl__ev_devstate ds;
>      void *base;
> +    /* device hotplug execution */
> +    char *what;

`const char *what;' ?

Ian.

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