[3/3] qemu: support configuring usb3 controller port count

Message ID d2b7a5286594061143cc30e827e297c439adf321.1461781613.git.crobinso@redhat.com
State New
Headers show

Commit Message

Cole Robinson April 27, 2016, 6:29 p.m.
This adds a ports= attribute to usb controller XML, like

  <controller type='usb' model='nec-xhci' ports='8'/>

This maps to:

  qemu -device nec-usb-xhci,p2=8,p3=8

Meaning, 8 ports that support both usb2 and usb3 devices. Gerd
suggested to just expose them as one knob.

https://bugzilla.redhat.com/show_bug.cgi?id=1271408
---
 docs/formatdomain.html.in                          |  6 ++--
 docs/schemas/domaincommon.rng                      |  8 ++++-
 src/conf/domain_conf.c                             | 39 ++++++++++++++--------
 src/conf/domain_conf.h                             |  7 ++++
 src/qemu/qemu_command.c                            | 13 ++++++++
 .../qemuxml2argv-usb-controller-xhci.args          | 22 ++++++++++++
 .../qemuxml2argv-usb-controller-xhci.xml           | 16 +++++++++
 tests/qemuxml2argvtest.c                           |  3 ++
 8 files changed, 98 insertions(+), 16 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-controller-xhci.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-controller-xhci.xml

-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Comments

Cole Robinson May 3, 2016, 1:01 p.m. | #1
On 05/02/2016 07:49 PM, John Ferlan wrote:
> 

> 

> On 04/27/2016 02:29 PM, Cole Robinson wrote:

>> This adds a ports= attribute to usb controller XML, like

>>

>>   <controller type='usb' model='nec-xhci' ports='8'/>

>>

>> This maps to:

>>

>>   qemu -device nec-usb-xhci,p2=8,p3=8

>>

>> Meaning, 8 ports that support both usb2 and usb3 devices. Gerd

>> suggested to just expose them as one knob.

>>

>> https://bugzilla.redhat.com/show_bug.cgi?id=1271408

>> ---

>>  docs/formatdomain.html.in                          |  6 ++--

>>  docs/schemas/domaincommon.rng                      |  8 ++++-

>>  src/conf/domain_conf.c                             | 39 ++++++++++++++--------

>>  src/conf/domain_conf.h                             |  7 ++++

>>  src/qemu/qemu_command.c                            | 13 ++++++++

>>  .../qemuxml2argv-usb-controller-xhci.args          | 22 ++++++++++++

>>  .../qemuxml2argv-usb-controller-xhci.xml           | 16 +++++++++

>>  tests/qemuxml2argvtest.c                           |  3 ++

>>  8 files changed, 98 insertions(+), 16 deletions(-)

>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-controller-xhci.args

>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-controller-xhci.xml

>>

> Do you need any virDomainControllerDefCheckABIStability checks with

> these changes?  w/r/t ports value

> 

> Other than that things looked fine to me (couldn't run through normal

> building process because I didn't want to deal with patch2 merges).

> 

> ACK - I trust you'll make the right check if you need it.

> 


Thanks, added that check and pushed now

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patch hide | download patch | download mbox

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index d2db53b..dc15664 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3028,8 +3028,10 @@ 
       <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 USB controller
-      will be built on s390.  The PowerPC64 "spapr-vio" addresses do not have an
-      associated controller.
+      will be built on s390.  <span class="since">Since 1.3.5</span>, USB
+      controllers accept a <code>ports</code> attribute to configure how
+      many devices can be connected to the controller. The PowerPC64
+      "spapr-vio" addresses do not have an associated controller.
     </p>
 
     <p>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f77bbcf..2deea34 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1743,7 +1743,8 @@ 
               </attribute>
             </optional>
           </group>
-          <!-- usb has an optional attribute "model", and optional subelement "master" -->
+          <!-- usb has an optional attribute "model",
+               and optional subelements "master" and "ports" -->
           <group>
             <attribute name="type">
               <value>usb</value>
@@ -1768,6 +1769,11 @@ 
             <optional>
               <ref name="usbmaster"/>
             </optional>
+            <optional>
+              <attribute name="ports">
+                <ref name="unsignedInt"/>
+              </attribute>
+            </optional>
           </group>
           <!-- pci has an optional attribute "model" -->
           <group>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 04817bd..fa560f3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1654,6 +1654,9 @@  virDomainControllerDefNew(virDomainControllerType type)
         def->opts.vioserial.ports = -1;
         def->opts.vioserial.vectors = -1;
         break;
+    case VIR_DOMAIN_CONTROLLER_TYPE_USB:
+        def->opts.usbopts.ports = -1;
+        break;
     case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
         def->opts.pciopts.chassisNr = -1;
         def->opts.pciopts.chassis = -1;
@@ -1666,7 +1669,6 @@  virDomainControllerDefNew(virDomainControllerType type)
     case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
     case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
     case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
-    case VIR_DOMAIN_CONTROLLER_TYPE_USB:
     case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
         break;
     }
@@ -7824,6 +7826,8 @@  virDomainControllerDefParseXML(xmlNodePtr node,
     char *busNr = NULL;
     int numaNode = -1;
     char *ioeventfd = NULL;
+    char *portsStr = NULL;
+    int ports = -1;
     xmlNodePtr saved = ctxt->node;
     int rc;
 
@@ -7936,20 +7940,19 @@  virDomainControllerDefParseXML(xmlNodePtr node,
         goto error;
     }
 
+    portsStr = virXMLPropString(node, "ports");
+    if (portsStr) {
+        int r = virStrToLong_i(portsStr, NULL, 10, &ports);
+        if (r != 0 || ports < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Invalid ports: %s"), portsStr);
+            goto error;
+        }
+    }
+
     switch (def->type) {
     case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: {
-        char *ports = virXMLPropString(node, "ports");
-        if (ports) {
-            int r = virStrToLong_i(ports, NULL, 10,
-                                   &def->opts.vioserial.ports);
-            if (r != 0 || def->opts.vioserial.ports < 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Invalid ports: %s"), ports);
-                VIR_FREE(ports);
-                goto error;
-            }
-        }
-        VIR_FREE(ports);
+        def->opts.vioserial.ports = ports;
 
         char *vectors = virXMLPropString(node, "vectors");
         if (vectors) {
@@ -7985,6 +7988,8 @@  virDomainControllerDefParseXML(xmlNodePtr node,
             def->info.mastertype = VIR_DOMAIN_CONTROLLER_MASTER_USB;
             def->info.master.usb.startport = masterPort;
         }
+
+        def->opts.usbopts.ports = ports;
         break;
     }
     case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
@@ -8112,6 +8117,7 @@  virDomainControllerDefParseXML(xmlNodePtr node,
     VIR_FREE(port);
     VIR_FREE(busNr);
     VIR_FREE(ioeventfd);
+    VIR_FREE(portsStr);
 
     return def;
 
@@ -19429,6 +19435,13 @@  virDomainControllerDefFormat(virBufferPtr buf,
         }
         break;
 
+    case VIR_DOMAIN_CONTROLLER_TYPE_USB:
+        if (def->opts.usbopts.ports != -1) {
+            virBufferAsprintf(buf, " ports='%d'",
+                              def->opts.usbopts.ports);
+        }
+        break;
+
     case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
         if (def->opts.pciopts.pcihole64)
             pcihole64 = true;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index d98f052..9071fe0 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -845,6 +845,12 @@  struct _virDomainPCIControllerOpts {
     int numaNode;
 };
 
+typedef struct _virDomainUSBControllerOpts virDomainUSBControllerOpts;
+typedef virDomainUSBControllerOpts *virDomainUSBControllerOptsPtr;
+struct _virDomainUSBControllerOpts {
+    int ports;   /* -1 == undef */
+};
+
 /* Stores the virtual disk controller configuration */
 struct _virDomainControllerDef {
     int type;
@@ -857,6 +863,7 @@  struct _virDomainControllerDef {
     union {
         virDomainVirtioSerialOpts vioserial;
         virDomainPCIControllerOpts pciopts;
+        virDomainUSBControllerOpts usbopts;
     } opts;
     virDomainDeviceInfo info;
 };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9597b30..7530c4a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2180,6 +2180,19 @@  qemuBuildUSBControllerDevStr(const virDomainDef *domainDef,
 
     virBufferAsprintf(buf, "%s", smodel);
 
+    if (def->opts.usbopts.ports != -1) {
+        if (model != VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI ||
+            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI_PORTS)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("usb controller type %s doesn't support 'ports' "
+                             "with this QEMU binary"), smodel);
+            return -1;
+        }
+
+        virBufferAsprintf(buf, ",p2=%d,p3=%d",
+                          def->opts.usbopts.ports, def->opts.usbopts.ports);
+    }
+
     if (def->info.mastertype == VIR_DOMAIN_CONTROLLER_MASTER_USB)
         virBufferAsprintf(buf, ",masterbus=%s.0,firstport=%d",
                           def->info.alias, def->info.master.usb.startport);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-xhci.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-xhci.args
new file mode 100644
index 0000000..b91adc4
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-xhci.args
@@ -0,0 +1,22 @@ 
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefconfig \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-device nec-usb-xhci,p2=8,p3=8,id=usb,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-xhci.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-xhci.xml
new file mode 100644
index 0000000..f5b7c56
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-controller-xhci.xml
@@ -0,0 +1,16 @@ 
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <controller type='usb' index='0' model='nec-xhci' ports='8'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 8842b2f..31a0ce1 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1172,6 +1172,9 @@  mymain(void)
                     QEMU_CAPS_DEVICE_PCI_BRIDGE,
                     QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_PCI_OHCI,
                     QEMU_CAPS_PIIX3_USB_UHCI);
+    DO_TEST("usb-controller-xhci",
+            QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_PIIX3_USB_UHCI,
+            QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_NEC_USB_XHCI_PORTS);
 
     DO_TEST("smbios", QEMU_CAPS_SMBIOS_TYPE);
     DO_TEST_PARSE_ERROR("smbios-date", QEMU_CAPS_SMBIOS_TYPE);