Re: [PATCH v7 RFC 1/2] libxl: Introduce functions to add and remove USB devices to an HVM guest

[George Dunlap <george.dunlap@eu.citrix.com>]

On 06/18/2014 02:08 PM, Ian Campbell wrote:
> On Mon, 2014-06-02 at 14:44 +0100, George Dunlap wrote:
>> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
>> index c7aa817..963e650 100644
>> --- a/tools/libxl/libxl.h
>> +++ b/tools/libxl/libxl.h
>> @@ -82,6 +82,12 @@
>>   #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1
>>   
>>   /*
>> + * LIBXL_HAVE_DEVICE_USB indicates the functions for doing hot-plug of
>> + * USB devices.
>> + */
>> +#define LIBXL_HAVE_DEVICE_USB 1
>> +
>> +/*
>>    * LIBXL_HAVE_BUILDINFO_HVM_VENDOR_DEVICE indicates that the
>>    * libxl_vendor_device field is present in the hvm sections of
>>    * libxl_domain_build_info. This field tells libxl which
>> @@ -924,6 +930,40 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
>>                          const libxl_asyncop_how *ao_how)
>>                          LIBXL_EXTERNAL_CALLERS_ONLY;
>>   
>> +/*
>> + * USB
>> + *
>> + * For each device removed or added, one of these protocols is available:
>> + * - PV (i.e., PVUSB)
>> + * - DEVICEMODEL (i.e, qemu)
>> + *
>> + * PV is available for either PV or HVM domains.  DEVICEMODEL is only
>> + * available for HVM domains.  The caller can additionally specify
>> + * "AUTO", in which case the library will try to determine the best
>> + * protocol automatically.
>> + *
>> + * At the moment, the only protocol implemented is DEVICEMODEL, and the only
>> + * device type implemented is HOSTDEV.
>> + *
>> + * This uses the qmp functionality, and is thus only available for
>> + * qemu-xen, not qemu-traditional.
>> + */
>> +int libxl_device_usb_add(libxl_ctx *ctx, uint32_t domid,
>> +                         libxl_device_usb *dev,
>> +                         const libxl_asyncop_how *ao_how)
>> +                         LIBXL_EXTERNAL_CALLERS_ONLY;
>> +int libxl_device_usb_remove(libxl_ctx *ctx, uint32_t domid,
>> +                            libxl_device_usb *dev,
>> +                            const libxl_asyncop_how *ao_how)
>> +                            LIBXL_EXTERNAL_CALLERS_ONLY;
>> +int libxl_device_usb_destroy(libxl_ctx *ctx, uint32_t domid,
>> +                             libxl_device_usb *dev,
>> +                             const libxl_asyncop_how *ao_how)
>> +                             LIBXL_EXTERNAL_CALLERS_ONLY;
>> +libxl_device_usb *libxl_device_usb_list(libxl_ctx *ctx, uint32_t domid,
>> +                                        int *num)
>> +                          LIBXL_EXTERNAL_CALLERS_ONLY;
> No _getinfo? (Might only make sense with the PV stuff I guess)

IIRC the pattern I saw for other devices was:
* libxl_device_$FOO struct is used to add, remove, destroy
* libl_device_$FOO_list returns an array of libxl_device_$FOO
* libl_device_$FOO_getinfo is only used when there's information you 
need which is not in libxl_device_$FOO struct (and hence isn't returned 
by _list).


>> +        ("backend_domid",    libxl_domid),
>> +        ("backend_domname",  string),
>> +        ("u", KeyedUnion(None, libxl_device_usb_type, "type",
>> +                         [("hostdev", Struct(None, [
>> +                                ("hostbus",   integer),
>> +                                ("hostaddr",  integer) ]))
> No need to express the host topology I think (because you can build that
> from the bus,addr tuples)?

I don't really follow.  You mean, we can drop 'host' from the last two 
elements, and just call them "bus" and "addr"?

  -George