[libvirt] [PATCH 1/6] extend usb controller model to support xen pvusb

[Juergen Gross]

On 14/06/16 07:02, Jim Fehlig wrote:
> On 06/12/2016 10:53 AM, Chunyan Liu wrote:
>> According to libxl implementation, it supports pvusb
>> controller of version 1.1 and version 2.0, and it
>> supports two types of backend, 'pvusb' (dom0 backend)
>> and 'qusb' (qemu backend).
> 
> IIUC, the pvusb backend has not gained any traction in the kernel and will not
> be accepted upstream. Adding Juergen to confirm. If that is the case, I think it
> should be dropped.

Correct. Upstream Linux doesn't have a pvusb backend and it seems
unlikely it will have one.

You are talking about dropping it from libvirt, right?

> 
>>
>> To match libxl support, extend usb controller schema
>> to support more models: pvusb1 (pvusb, version 1.1),
>> pvusb2 (pvusb, version 2.0), qusb1 (qusb, version 1.1)
>> and 'qusb2' (qusb version 2.0).
>>
>> Signed-off-by: Chunyan Liu <cyliu at suse.com>
>> ---
>>  docs/formatdomain.html.in     | 6 +++++-
>>  docs/schemas/domaincommon.rng | 4 ++++
>>  src/conf/domain_conf.c        | 4 ++++
>>  src/conf/domain_conf.h        | 4 ++++
>>  src/qemu/qemu_command.c       | 4 ++++
>>  5 files changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index fb3ec5e..1492915 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -3063,7 +3063,11 @@
>>          <dd>A <code>usb</code> controller has an optional attribute
>>          <code>model</code>, which is one of "piix3-uhci", "piix4-uhci",
>>          "ehci", "ich9-ehci1", "ich9-uhci1", "ich9-uhci2", "ich9-uhci3",
>> -        "vt82c686b-uhci", "pci-ohci" or "nec-xhci".  Additionally,
>> +        "vt82c686b-uhci", "pci-ohci", "nec-xhci", "pvusb1" (xen pvusb
>> +        with dom0 backend, version 1.1), "pvusb2" (xen pvusb with dom0
>> +        backend, version 2.0), "qusb1" (xen pvusb with qemu backend,
>> +        version 1.1) or "qusb2" (xen pvusb with qemu backend, version 2.0).
>> +        Additionally,
>>           <span class="since">since 0.10.0</span>, if the USB bus needs to
>>           be explicitly disabled for the guest, <code>model='none'</code>
>>           may be used.  <span class="since">Since 1.0.5</span>, no default
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 02078d7..5afa15d 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -1768,6 +1768,10 @@
>>                    <value>pci-ohci</value>
>>                    <value>nec-xhci</value>
>>                    <value>none</value>
>> +                  <value>pvusb1</value>
>> +                  <value>pvusb2</value>
>> +                  <value>qusb1</value>
>> +                  <value>qusb2</value>
>>                  </choice>
>>                </attribute>
>>              </optional>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 10e61da..ff3285f 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -356,6 +356,10 @@ VIR_ENUM_IMPL(virDomainControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST,
>>                "vt82c686b-uhci",
>>                "pci-ohci",
>>                "nec-xhci",
>> +              "pvusb1",
>> +              "pvusb2",
>> +              "qusb1",
>> +              "qusb2",
>>                "none")
>>  
>>  VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index 3792562..6733b2d 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -675,6 +675,10 @@ typedef enum {
>>      VIR_DOMAIN_CONTROLLER_MODEL_USB_VT82C686B_UHCI,
>>      VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI,
>>      VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI,
>> +    VIR_DOMAIN_CONTROLLER_MODEL_USB_PVUSB1,
>> +    VIR_DOMAIN_CONTROLLER_MODEL_USB_PVUSB2,
>> +    VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB1,
>> +    VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2,
>>      VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE,
>>  
>>      VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 490260f..5c3de83 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -133,6 +133,10 @@ VIR_ENUM_IMPL(qemuControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST,
>>                "vt82c686b-usb-uhci",
>>                "pci-ohci",
>>                "nec-usb-xhci",
>> +              "pvusb1",
>> +              "pvusb2",
>> +              "qusb1",
>> +              "qusb2",
>>                "none");
> 
> It seems odd that these need to be added to qemu_command.c. But sadly, I'm not
> familiar with how USB controllers are handled in the qemu driver to give much
> useful feedback at this time. I can certainly start investigating that, but in
> the meantime I've added Laine to the cc list. He has done quite a bit of work in
> this area in the past and might have a few minutes free to comment on these changes.

qusb (and pvusb, too) don't require any qemu interaction besides what
is already done in libxl. There are no qmp commands or whatever
involved, all communication is done via Xenstore.

> I know you will be leaving soon to pursue other opportunities (/me sheds a tear
> since one of the few libxl driver contributors will be departing), but I can
> continue this work if it is not merged before your departure. Thanks a lot
> Chunyan for all your work on Xen-related matters in libvirt over the years!

Yeah, thanks especially for your pvusb work!


Juergen