[libvirt] [PATCH] qemu: Build usb controller command line more wisely

Michal Privoznik posted 1 patch 6 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/b133fac356324c3c7992a9cb07c1b5d9eea1e2eb.1521214694.git.mprivozn@redhat.com
Test syntax-check passed
src/qemu/qemu_command.c                      | 44 ++++++++++++++--
tests/qemuxml2argvdata/user-aliases-usb.args | 38 ++++++++++++++
tests/qemuxml2argvdata/user-aliases-usb.xml  | 78 ++++++++++++++++++++++++++++
tests/qemuxml2argvtest.c                     |  3 ++
4 files changed, 158 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuxml2argvdata/user-aliases-usb.args
create mode 100644 tests/qemuxml2argvdata/user-aliases-usb.xml
[libvirt] [PATCH] qemu: Build usb controller command line more wisely
Posted by Michal Privoznik 6 years, 1 month ago
https://bugzilla.redhat.com/show_bug.cgi?id=1552127

When building command line for USB controllers we have to do more
than just put controller's alias onto the command line. QEMU has
concept of these joined USB controllers. For instance ehci and
uhci controllers need to create the same USB bus. To achieve that
the slave controller needs to refer the master controller. This
worked until we've introduced user aliases because both master
and slave had the same alias. With user aliases slave can have
different alias than master. Therefore, when generating command
line for slave we need to look up the master's alias.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_command.c                      | 44 ++++++++++++++--
 tests/qemuxml2argvdata/user-aliases-usb.args | 38 ++++++++++++++
 tests/qemuxml2argvdata/user-aliases-usb.xml  | 78 ++++++++++++++++++++++++++++
 tests/qemuxml2argvtest.c                     |  3 ++
 4 files changed, 158 insertions(+), 5 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/user-aliases-usb.args
 create mode 100644 tests/qemuxml2argvdata/user-aliases-usb.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index fa0aa5d5c3..a8afbd14fa 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2545,8 +2545,34 @@ qemuControllerModelUSBToCaps(int model)
 }
 
 
+static const char *
+qemuBuildUSBControllerFindMasterAlias(const virDomainDef *domainDef,
+                                      const virDomainControllerDef *def)
+{
+    size_t i;
+
+    for (i = 0; i < domainDef->ncontrollers; i++) {
+        const virDomainControllerDef *tmp = domainDef->controllers[i];
+
+        if (tmp->type != VIR_DOMAIN_CONTROLLER_TYPE_USB)
+            continue;
+
+        if (tmp->idx != def->idx)
+            continue;
+
+        if (tmp->info.mastertype == VIR_DOMAIN_CONTROLLER_MASTER_USB)
+            continue;
+
+        return tmp->info.alias;
+    }
+
+    return NULL;
+}
+
+
 static int
-qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def,
+qemuBuildUSBControllerDevStr(const virDomainDef *domainDef,
+                             virDomainControllerDefPtr def,
                              virQEMUCapsPtr qemuCaps,
                              virBuffer *buf)
 {
@@ -2586,11 +2612,19 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def,
                           def->opts.usbopts.ports, def->opts.usbopts.ports);
     }
 
-    if (def->info.mastertype == VIR_DOMAIN_CONTROLLER_MASTER_USB)
+    if (def->info.mastertype == VIR_DOMAIN_CONTROLLER_MASTER_USB) {
+        const char *masterbus;
+
+        if (!(masterbus = qemuBuildUSBControllerFindMasterAlias(domainDef, def))) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("masterbus not found"));
+            return -1;
+        }
         virBufferAsprintf(buf, ",masterbus=%s.0,firstport=%d",
-                          def->info.alias, def->info.master.usb.startport);
-    else
+                          masterbus, def->info.master.usb.startport);
+    } else {
         virBufferAsprintf(buf, ",id=%s", def->info.alias);
+    }
 
     return 0;
 }
@@ -2722,7 +2756,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
         break;
 
     case VIR_DOMAIN_CONTROLLER_TYPE_USB:
-        if (qemuBuildUSBControllerDevStr(def, qemuCaps, &buf) == -1)
+        if (qemuBuildUSBControllerDevStr(domainDef, def, qemuCaps, &buf) == -1)
             goto error;
 
         if (nusbcontroller)
diff --git a/tests/qemuxml2argvdata/user-aliases-usb.args b/tests/qemuxml2argvdata/user-aliases-usb.args
new file mode 100644
index 0000000000..3dfaadc33b
--- /dev/null
+++ b/tests/qemuxml2argvdata/user-aliases-usb.args
@@ -0,0 +1,38 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name gentoo \
+-S \
+-M pc-i440fx-1.4 \
+-m 4096 \
+-smp 4,sockets=4,cores=1,threads=1 \
+-uuid a75aca4b-a02f-2bcb-4a91-c93cd848c34b \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-gentoo/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-global PIIX4_PM.disable_s3=0 \
+-global PIIX4_PM.disable_s4=0 \
+-boot cd \
+-device ich9-usb-ehci1,id=ua-myUSB1,bus=pci.0,addr=0x4.0x7 \
+-device ich9-usb-uhci1,masterbus=ua-myUSB1.0,firstport=0,bus=pci.0,\
+multifunction=on,addr=0x4 \
+-device ich9-usb-uhci2,masterbus=ua-myUSB1.0,firstport=2,bus=pci.0,\
+addr=0x4.0x1 \
+-device ich9-usb-uhci3,masterbus=ua-myUSB1.0,firstport=4,bus=pci.0,\
+addr=0x4.0x2 \
+-device ich9-usb-ehci1,id=ua-myUSB5,bus=pci.0,addr=0x5.0x7 \
+-device ich9-usb-uhci1,masterbus=ua-myUSB5.0,firstport=0,bus=pci.0,\
+multifunction=on,addr=0x5 \
+-device ich9-usb-uhci2,masterbus=ua-myUSB5.0,firstport=2,bus=pci.0,\
+addr=0x5.0x1 \
+-device ich9-usb-uhci3,masterbus=ua-myUSB5.0,firstport=4,bus=pci.0,\
+addr=0x5.0x2 \
+-device usb-host,hostbus=14,hostaddr=6,id=hostdev0,bus=ua-myUSB1.0,port=3 \
+-device usb-host,hostbus=15,hostaddr=6,id=hostdev1,bus=ua-myUSB5.0,port=3 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/user-aliases-usb.xml b/tests/qemuxml2argvdata/user-aliases-usb.xml
new file mode 100644
index 0000000000..668a9c384b
--- /dev/null
+++ b/tests/qemuxml2argvdata/user-aliases-usb.xml
@@ -0,0 +1,78 @@
+<domain type='kvm'>
+  <name>gentoo</name>
+  <uuid>a75aca4b-a02f-2bcb-4a91-c93cd848c34b</uuid>
+  <memory unit='KiB'>4194304</memory>
+  <currentMemory unit='KiB'>4194304</currentMemory>
+  <vcpu placement='static'>4</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc-i440fx-1.4'>hvm</type>
+    <boot dev='hd'/>
+    <boot dev='cdrom'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <pm>
+    <suspend-to-mem enabled='yes'/>
+    <suspend-to-disk enabled='yes'/>
+  </pm>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0' model='ich9-ehci1'>
+      <alias name='ua-myUSB1'/>
+      <address type='pci' domain='0' bus='0' slot='4' function='7'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci1'>
+      <alias name='ua-myUSB2'/>
+      <master startport='0'/>
+      <address type='pci' domain='0' bus='0' slot='4' function='0' multifunction='on'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci2'>
+      <alias name='ua-myUSB3'/>
+      <master startport='2'/>
+      <address type='pci' domain='0' bus='0' slot='4' function='1'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci3'>
+      <alias name='ua-myUSB4'/>
+      <master startport='4'/>
+      <address type='pci' domain='0' bus='0' slot='4' function='2'/>
+    </controller>
+    <controller type='usb' index='1' model='ich9-ehci1'>
+      <alias name='ua-myUSB5'/>
+      <address type='pci' domain='0' bus='0' slot='5' function='7'/>
+    </controller>
+    <controller type='usb' index='1' model='ich9-uhci1'>
+      <alias name='ua-myUSB6'/>
+      <master startport='0'/>
+      <address type='pci' domain='0' bus='0' slot='5' function='0' multifunction='on'/>
+    </controller>
+    <controller type='usb' index='1' model='ich9-uhci2'>
+      <alias name='ua-myUSB7'/>
+      <master startport='2'/>
+      <address type='pci' domain='0' bus='0' slot='5' function='1'/>
+    </controller>
+    <controller type='usb' index='1' model='ich9-uhci3'>
+      <alias name='ua-myUSB8'/>
+      <master startport='4'/>
+      <address type='pci' domain='0' bus='0' slot='5' function='2'/>
+    </controller>
+    <hostdev mode='subsystem' type='usb' managed='yes'>
+      <source>
+        <address bus='14' device='6'/>
+      </source>
+      <address type='usb' bus='0' port='3'/>
+    </hostdev>
+    <hostdev mode='subsystem' type='usb' managed='yes'>
+      <source>
+        <address bus='15' device='6'/>
+      </source>
+      <address type='usb' bus='1' port='3'/>
+    </hostdev>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 731db9ed52..f6a90c2814 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2997,6 +2997,9 @@ mymain(void)
             QEMU_CAPS_DEVICE_ISA_SERIAL,
             QEMU_CAPS_HDA_DUPLEX);
     DO_TEST("user-aliases2", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI);
+    DO_TEST("user-aliases-usb", QEMU_CAPS_KVM,
+            QEMU_CAPS_PIIX_DISABLE_S3, QEMU_CAPS_PIIX_DISABLE_S4,
+            QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_PCI_MULTIFUNCTION);
 
     /* Test disks with format probing enabled for legacy reasons.
      * New tests should not go in this section. */
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Build usb controller command line more wisely
Posted by Ján Tomko 6 years, 1 month ago
On Fri, Mar 16, 2018 at 04:38:14PM +0100, Michal Privoznik wrote:
>https://bugzilla.redhat.com/show_bug.cgi?id=1552127
>
>When building command line for USB controllers we have to do more
>than just put controller's alias onto the command line. QEMU has
>concept of these joined USB controllers. For instance ehci and
>uhci controllers need to create the same USB bus. To achieve that
>the slave controller needs to refer the master controller. This
>worked until we've introduced user aliases because both master
>and slave had the same alias. With user aliases slave can have
>different alias than master. Therefore, when generating command
>line for slave we need to look up the master's alias.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/qemu/qemu_command.c                      | 44 ++++++++++++++--
> tests/qemuxml2argvdata/user-aliases-usb.args | 38 ++++++++++++++
> tests/qemuxml2argvdata/user-aliases-usb.xml  | 78 ++++++++++++++++++++++++++++
> tests/qemuxml2argvtest.c                     |  3 ++
> 4 files changed, 158 insertions(+), 5 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/user-aliases-usb.args
> create mode 100644 tests/qemuxml2argvdata/user-aliases-usb.xml
>

ACK

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