Re: [PATCH 6 of 9 v2] libxl: execute hotplug scripts directly from libxl

["Roger Pau Monné" <roger.pau@entel.upc.edu>]

2011/11/18 Ian Campbell <Ian.Campbell@citrix.com>:
> On Fri, 2011-11-18 at 11:59 +0000, Roger Pau Monne wrote:
>> # HG changeset patch
>> # User Roger Pau Monne <roger.pau@entel.upc.edu>
>> # Date 1317386335 -7200
>> # Node ID 1d81d1c4c851c0b07696373304801df56a221e90
>> # Parent  4ad998fddb16a299cb230ea23ba944d84adbd2bf
>> libxl: execute hotplug scripts directly from libxl.
>>
>> Added the necessary handlers to execute hotplug scripts when necessary
>> from libxl. Split NetBSD and Linux hotplug calls into two separate
>> files, because parameters for hotplug scripts are different. Linux
>> has empty functions, since the calling of hotplug scripts is still
>> done using udev.
>>
>> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
>>
>> diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/Makefile
>> --- a/tools/libxl/Makefile      Fri Nov 18 11:29:14 2011 +0100
>> +++ b/tools/libxl/Makefile      Fri Sep 30 14:38:55 2011 +0200
>> @@ -34,8 +34,10 @@ LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpu
>
> Should be:
>
>>  ifeq ($(CONFIG_NetBSD),y)
>>  LIBXL_OBJS-y += libxl_phybackend.o
>> +LIBXL_OBJS-y += hotplug_netbsd.o
>   elsif ($(CONFIG_Linux),y)
>>  LIBXL_OBJS-y += libxl_nophybackend.o
>> +LIBXL_OBJS-y += hotplug_linux.o
>   else
>   $(error A message of some sort)
>>  endif

Done, I've moved PHY backend selection and hotplug specific functions
to libxl_netbsd.c and libxl_linux.c, and added an error message in
case someone tries to use a different OS.

>> diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/libxl_device.c
>> --- a/tools/libxl/libxl_device.c        Fri Nov 18 11:29:14 2011 +0100
>> +++ b/tools/libxl/libxl_device.c        Fri Sep 30 14:38:55 2011 +0200
>> @@ -68,6 +68,24 @@ int libxl__parse_backend_path(libxl__gc
>>      return libxl__device_kind_from_string(strkind, &dev->backend_kind);
>>  }
>>
>> +int libxl__device_execute_hotplug(libxl__gc *gc, libxl__device *dev)
>
> No need for a add/remove type parameter?

Although you could pass this kind of parameter, the action to perform
is based on the state of the device in the corresponding xenstore
entry. State 2 means that the device should be connected, state 6
means the device should be disconnected.

>> +{
>> +    int rc = 0;
>> +
>> +    switch(dev->kind) {
>> +    case LIBXL__DEVICE_KIND_VIF:
>> +        rc = libxl__nic_hotplug(gc, dev);
>> +        break;
>> +    case LIBXL__DEVICE_KIND_VBD:
>> +        rc = libxl__disk_hotplug(gc, dev);
>> +        break;
>> +    default:
>> +        break;
>> +    }
>> +
>> +    return rc;
>> +}
>> +
>>  int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
>>                               char **bents, char **fents)
>>  {
>> @@ -449,6 +467,7 @@ int libxl__device_remove(libxl__gc *gc,
>>      if (!state)
>>          goto out;
>>      if (atoi(state) != 4) {
>> +        libxl__device_execute_hotplug(gc, dev);
>>          libxl__device_destroy_tapdisk(gc, be_path);
>>          xs_rm(ctx->xsh, XBT_NULL, be_path);
>>          goto out;
>> @@ -492,6 +511,12 @@ int libxl__device_destroy(libxl__gc *gc,
>>      char *be_path = libxl__device_backend_path(gc, dev);
>>      char *fe_path = libxl__device_frontend_path(gc, dev);
>>
>> +    /*
>> +     * Run hotplug scripts, which will probably not be able to
>> +     * execute successfully since the device may still be plugged
>
> What does this mean? If we don't expect it to work why are we calling
> them?

If you call the libxl_domain_destroy function with force == 1, the
destroy process doesn't wait for devices to be disconnected, but we
might as well try to execute hotplug scripts, since we don't know the
actual state of the device. If we are lucky, the device might be
disconnected (state == 6), and we should be able to execute hotplug
scripts successfully.

>> +     */
>> +    libxl__device_execute_hotplug(gc, dev);
>> +
>>      xs_rm(ctx->xsh, XBT_NULL, be_path);
>>      xs_rm(ctx->xsh, XBT_NULL, fe_path);
>>
>> diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/libxl_internal.h
>> --- a/tools/libxl/libxl_internal.h      Fri Nov 18 11:29:14 2011 +0100
>> +++ b/tools/libxl/libxl_internal.h      Fri Sep 30 14:38:55 2011 +0200
>> @@ -287,6 +287,34 @@ _hidden int libxl__wait_for_device_state
>>   */
>>  _hidden int libxl__try_phy_backend(mode_t st_mode);
>>
>> +/* hotplug functions */
>> +/*
>> + * libxl__device_execute_hotplug - generic function to execute hotplug scripts
>> + * gc: allocation pool
>> + * dev: reference to the device that executes the hotplug scripts
>> + *
>> + * Returns 0 on success, and < 0 on error.
>> + */
>> +_hidden int libxl__device_execute_hotplug(libxl__gc *gc, libxl__device *dev);
>> +
>> +/*
>> + * libxl__disk_hotplug - execute hotplug script for a disk type device
>> + * gc: allocation pool
>> + * dev: reference to the disk device
>> + *
>> + * Returns 0 on success, and < 0 on error.
>> + */
>> +_hidden int libxl__disk_hotplug(libxl__gc *gc, libxl__device *dev);
>> +
>> +/*
>> + * libxl__nic_hotplug - execute hotplug script for a nic type device
>> + * gc: allocation pool
>> + * dev: reference to the nic device
>> + *
>> + * Returns 0 on success, and < 0 on error.
>> + */
>> +_hidden int libxl__nic_hotplug(libxl__gc *gc, libxl__device *dev);
>> +
>>  /* from libxl_pci */
>>
>>  _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting);
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>
>
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel