Re: [Xen-devel] [PATCH RFC V2 1/5] libxl: add pvusb definitions

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

On 03/03/2015 11:10 AM, Ian Campbell wrote:
> On Mon, 2015-01-19 at 16:28 +0800, Chunyan Liu wrote:
> 
> Sorry for the long delay in replying.
> 
>> To attach a usb device, a virtual usb controller should be created first.
>> This patch defines usbctrl and usbdevice related structs.
> 
> Per <54CA17DF0200006600095E3D@xxxxxxxxxxxxxxxxxxxxxxx> please could you
> mention here that the HVM guest related parts (i.e.
> LIBXL_USBCTRL_TYPE_DEVICEMODEL) and libxl_usb_type are placeholders for
> emulated HVM support.
> 
> In fact I wonder if it should just be omitted, we will need a LIBXL_HAVE
> for HVM USB support anyway once it is implemented so we can add the enum
> then.
> 
> Or will we -- do we think returning an error for such an HVM guest with
> USB devices configured now is acceptable and for it to silently start
> working at some point in the future is OK?

I think that was my idea back when I was writing the HVM hotplug patches.

> 
>>
>> Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx>
>> Signed-off-by: Simon Cao <caobosimon@xxxxxxxxx>
>> ---
>>  tools/libxl/libxl_types.idl          | 58 
>> +++++++++++++++++++++++++++++++++++-
>>  tools/libxl/libxl_types_internal.idl |  1 +
>>  2 files changed, 58 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> index 1214d2e..0639434 100644
>> --- a/tools/libxl/libxl_types.idl
>> +++ b/tools/libxl/libxl_types.idl
>> @@ -521,6 +531,27 @@ libxl_device_pci = Struct("device_pci", [
>>      ("seize", bool),
>>      ])
>>  
>> +libxl_device_usbctrl = Struct("device_usbctrl", [
>> +    ("name", string),
>> +    ("type", libxl_usbctrl_type),
>> +    ("backend_domid", libxl_domid),
>> +    ("backend_domname", string),
>> +    ("devid", libxl_devid),
>> +    ("usb_version", uint8),
>> +    ("num_ports", uint8),
> 
> I think int would be fine for both of these last two (and is a bit
> kinder to language bindings).
> 
>> +    ])
>> +
>> +libxl_device_usb = Struct("device_usb", [
>> +    ("ctrl", integer),
> 
> Is this an index into something? If so what?
> 
> There seems to be no usbctrl array added to the domain_config struct, so
> I'm unsure how this is used.

It looks like this is meant to specify which USB controller you're
plugging your device into; and if you set it to -1, then it will
automatically try to plug it into the best one (and also automatically
create one for you if one doesn't exist).

This should probably be a libxl_devid I'm guessing, then?

 -George


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