Re: RFC: Proposed libxl USB hot-plug interface

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

On 03/04/13 16:46, Ian Jackson wrote:
> George Dunlap writes ("RFC: Proposed libxl USB hot-plug interface"):
>> OK, based on the feedback, what about an interface like the following?
> This looks plausible.
>
>> The idea would be that long-term, AUTO would always do PV for PV VMs,
>> and would somehow decide whether to do HVM or PV for HVM domains.
> Right.
>
>> The "type" and the "union" fields are designed to allow the interface to
>> be extended to include other types of devices, such as adding emulated
>> tablets, mice, keyboards, usb sticks, &c.
> Right.
>
>> "list" should return all available USB devices including the "handle"
>> that can be used to remove a device.
>> struct libxl_device_usb {
>>       uint16_t protocol; /* AUTO, PV, HVM */
>>       uint16_t type; /* HOST; later can be emulated devices like tablet,
>> disk, &c */
>>       uint32_t backend_domain_id; /* For PVUSB */
> I don't see why the backend domain is necessarily invalid for hvm
> domains.  In the stub-dm, you're going to have both an emulated
> interface (at the guest-dm interface) and a PV one (at the dm-backend
> interface).  (Not that this will necessarily be implemented right
> away!)

Hmm -- I hadn't thought of using qemu-usb in a stubdom.  But in any 
case, I think the comment is still valid -- the backend_domain_id is 
being used for the PVUSB part of the "connection".  (And if an HVM 
domain has a usbfront drivers, it can of course use this field as well.)

>
>>       uint64_t handle;  /* OUT: Unique (per domain) handle that must be
>> used to remove a device */
> I'm not sure mixing in and out parameters in the single struct is a
> good idea.

Well it's particularly interesting I think to have it here for the 
"usb-list" function (in which case the entire structure is written by 
the library).  We could I suppose have usb-add just put it in the return 
value instead of modifying the structure in-place.
>>       int
>>       union {
>>           struct {
>>               int hostbus, hostaddr, vendorid, productid;
>>           } host;
> This needs some more documentation.  Why is it a union ?  What are its
> other options, etc. ?

This was just meant to be a brief description to talk about -- 
definitely not the final version. :-)

Initially this would be a union because I had envisioned something like 
this:

union {
     struct { blah blah } host;
     struct { blah blah } wacom-tablet;
     struct { blah blah blah } disk;
};

Then it's the same functions to add/remove/list an emulated tablet, or 
an emulated USB disk.

> I guess you can set the values to -1 to mean "wildcard" ?

or LIBXL_USB_DEVICE_HOST_ANY, which is #defined to -1. :-)

  -George