[PATCH 30/31] qemu: Finish cleaning up qemuDomainDefaultUSBControllerModel()

Andrea Bolognani via Devel posted 31 patches 5 months, 3 weeks ago
There is a newer version of this series
[PATCH 30/31] qemu: Finish cleaning up qemuDomainDefaultUSBControllerModel()
Posted by Andrea Bolognani via Devel 5 months, 3 weeks ago
At this point the USB controller model selection logic is
mostly sane, but there are still a few remaining oddities.

First of all, piix3-uhci (USB1) is used in way too many
contexts, either as default or fallback choice, while it
really should be relegated to x86 guests only.

Additionally, we're explicitly defaulting to pci-ohci (USB1)
for a couple of Arm machine types, and limiting the default
of qemu-xhci (USB3) to aarch64 only instead of using it for
32-bit guests as well.

Address all of the aforementioned quirks. The result is a
reasonably consistent experience across architectures, with
USB3 generally being used whenever available, Intel-specific
USB controllers only showing up in x86 guests, and pci-ohci
acting as the vaguely reasonable falllback across the board.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/qemu/qemu_domain.c                        | 28 ++++++++-----------
 ...iew-minimal.aarch64-latest.abi-update.args |  2 +-
 ...view-minimal.aarch64-latest.abi-update.xml |  2 +-
 ...rch64-realview-minimal.aarch64-latest.args |  2 +-
 ...arch64-realview-minimal.aarch64-latest.xml |  2 +-
 ...lepb-minimal.armv7l-latest.abi-update.args |  2 +-
 ...ilepb-minimal.armv7l-latest.abi-update.xml |  2 +-
 ...v7l-versatilepb-minimal.armv7l-latest.args |  2 +-
 ...mv7l-versatilepb-minimal.armv7l-latest.xml |  2 +-
 tests/qemuxmlconfdata/ppc-dtb.ppc-latest.args |  2 +-
 tests/qemuxmlconfdata/ppc-dtb.ppc-latest.xml  |  2 +-
 ...c-mac99-minimal.ppc-latest.abi-update.args |  2 +-
 ...pc-mac99-minimal.ppc-latest.abi-update.xml |  2 +-
 .../ppc-mac99-minimal.ppc-latest.args         |  2 +-
 .../ppc-mac99-minimal.ppc-latest.xml          |  2 +-
 .../ppce500-serial.ppc-latest.args            |  2 +-
 .../ppce500-serial.ppc-latest.xml             |  2 +-
 ...ler-automatic-realview.aarch64-latest.args |  2 +-
 ...ller-automatic-realview.aarch64-latest.xml |  2 +-
 ...r-automatic-versatilepb.armv7l-latest.args |  2 +-
 ...er-automatic-versatilepb.armv7l-latest.xml |  2 +-
 ...ault-fallback-realview.aarch64-latest.args |  2 +-
 ...fault-fallback-realview.aarch64-latest.xml |  2 +-
 ...ontroller-default-mac99ppc.ppc-latest.args |  2 +-
 ...controller-default-mac99ppc.ppc-latest.xml |  2 +-
 ...ler-default-versatilepb.armv7l-latest.args |  2 +-
 ...ller-default-versatilepb.armv7l-latest.xml |  2 +-
 tests/qemuxmlconftest.c                       | 16 +++++------
 28 files changed, 46 insertions(+), 50 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index df64ddbec1..2c7d91f597 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4334,7 +4334,7 @@ qemuDomainDefaultUSBControllerModel(const virDomainDef *def,
         return VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;
     }
 
-    if (def->os.arch == VIR_ARCH_AARCH64) {
+    if (ARCH_IS_ARM(def->os.arch)) {
         /* Prefer USB3 */
         if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI))
             return VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI;
@@ -4366,14 +4366,17 @@ qemuDomainDefaultUSBControllerModel(const virDomainDef *def,
         return VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;
     }
 
-    /* The default USB controller is piix3-uhci (USB1) if available.
-     * This choice is a fairly poor one, rooted primarily in
-     * historical reasons; thankfully, in most cases we will have
-     * picked a much more reasonable value before ever getting here */
-    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI))
-        return VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
-    else if (!ARCH_IS_X86(def->os.arch) &&
-             virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI))
+    if (ARCH_IS_X86(def->os.arch)) {
+        /* Use piix3-uhci (USB1) for backwards compatibility */
+        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI))
+            return VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
+        return VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;
+    }
+
+    /* Most common architectures and machine types have been already
+     * handled above; for the remaining cases, pci-ohci (USB1) is a
+     * sensible enough fallback */
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI))
         return VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI;
 
     return VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;
@@ -4425,13 +4428,6 @@ qemuDomainDefaultUSBControllerModelAutoAdded(const virDomainDef *def,
         }
     }
 
-    if (ARCH_IS_ARM(def->os.arch)) {
-        if (STREQ(def->os.machine, "versatilepb") ||
-            STRPREFIX(def->os.machine, "realview-eb"))
-            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI))
-                return VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI;
-    }
-
     return VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;
 }
 
diff --git a/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.abi-update.args b/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.abi-update.args
index 0d956241fc..ccbff79f35 100644
--- a/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.abi-update.args
+++ b/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.abi-update.args
@@ -25,7 +25,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.config \
 -rtc base=utc \
 -no-shutdown \
 -boot strict=on \
--device '{"driver":"pci-ohci","id":"usb","bus":"pci","addr":"0x1"}' \
+-device '{"driver":"qemu-xhci","id":"usb","bus":"pci","addr":"0x1"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.abi-update.xml b/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.abi-update.xml
index c31c7b2bbc..056e5e56a8 100644
--- a/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.abi-update.xml
+++ b/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.abi-update.xml
@@ -15,7 +15,7 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-aarch64</emulator>
-    <controller type='usb' index='0' model='pci-ohci'>
+    <controller type='usb' index='0' model='qemu-xhci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.args b/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.args
index 0d956241fc..ccbff79f35 100644
--- a/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.args
+++ b/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.args
@@ -25,7 +25,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.config \
 -rtc base=utc \
 -no-shutdown \
 -boot strict=on \
--device '{"driver":"pci-ohci","id":"usb","bus":"pci","addr":"0x1"}' \
+-device '{"driver":"qemu-xhci","id":"usb","bus":"pci","addr":"0x1"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.xml b/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.xml
index c31c7b2bbc..056e5e56a8 100644
--- a/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.xml
+++ b/tests/qemuxmlconfdata/aarch64-realview-minimal.aarch64-latest.xml
@@ -15,7 +15,7 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-aarch64</emulator>
-    <controller type='usb' index='0' model='pci-ohci'>
+    <controller type='usb' index='0' model='qemu-xhci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.abi-update.args b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.abi-update.args
index a05a413290..88fe2b62e8 100644
--- a/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.abi-update.args
+++ b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.abi-update.args
@@ -26,7 +26,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-armv7ltest/.config \
 -rtc base=utc \
 -no-shutdown \
 -boot strict=on \
--device '{"driver":"pci-ohci","id":"usb","bus":"pci","addr":"0x1"}' \
+-device '{"driver":"qemu-xhci","id":"usb","bus":"pci","addr":"0x1"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.abi-update.xml b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.abi-update.xml
index 7b21b24927..a9da2a3b13 100644
--- a/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.abi-update.xml
+++ b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.abi-update.xml
@@ -17,7 +17,7 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-armv7l</emulator>
-    <controller type='usb' index='0' model='pci-ohci'>
+    <controller type='usb' index='0' model='qemu-xhci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.args b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.args
index a05a413290..88fe2b62e8 100644
--- a/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.args
+++ b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.args
@@ -26,7 +26,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-armv7ltest/.config \
 -rtc base=utc \
 -no-shutdown \
 -boot strict=on \
--device '{"driver":"pci-ohci","id":"usb","bus":"pci","addr":"0x1"}' \
+-device '{"driver":"qemu-xhci","id":"usb","bus":"pci","addr":"0x1"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.xml b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.xml
index 7b21b24927..a9da2a3b13 100644
--- a/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.xml
+++ b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.xml
@@ -17,7 +17,7 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-armv7l</emulator>
-    <controller type='usb' index='0' model='pci-ohci'>
+    <controller type='usb' index='0' model='qemu-xhci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.args b/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.args
index 8032ad7f0e..bdc86620c8 100644
--- a/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.args
+++ b/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.args
@@ -29,7 +29,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
 -initrd /media/ram/ramdisk \
 -append 'root=/dev/ram rw console=ttyS0,115200' \
 -dtb /media/ram/test.dtb \
--device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1"}' \
+-device '{"driver":"pci-ohci","id":"usb","bus":"pci.0","addr":"0x1"}' \
 -chardev pty,id=charserial0 \
 -serial chardev:charserial0 \
 -audiodev '{"id":"audio1","driver":"none"}' \
diff --git a/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.xml b/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.xml
index 400f749eb0..31fcc3d053 100644
--- a/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.xml
+++ b/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.xml
@@ -18,7 +18,7 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-ppc</emulator>
-    <controller type='usb' index='0' model='piix3-uhci'>
+    <controller type='usb' index='0' model='pci-ohci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.args b/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.args
index 8600eec328..9c7c884c83 100644
--- a/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.args
+++ b/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.args
@@ -25,7 +25,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \
 -rtc base=utc \
 -no-shutdown \
 -boot strict=on \
--device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1"}' \
+-device '{"driver":"pci-ohci","id":"usb","bus":"pci.0","addr":"0x1"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
 -device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
diff --git a/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.xml b/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.xml
index 215c196fbf..633aa684da 100644
--- a/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.xml
+++ b/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.xml
@@ -14,7 +14,7 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-ppc</emulator>
-    <controller type='usb' index='0' model='piix3-uhci'>
+    <controller type='usb' index='0' model='pci-ohci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.args b/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.args
index 8600eec328..9c7c884c83 100644
--- a/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.args
+++ b/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.args
@@ -25,7 +25,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \
 -rtc base=utc \
 -no-shutdown \
 -boot strict=on \
--device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1"}' \
+-device '{"driver":"pci-ohci","id":"usb","bus":"pci.0","addr":"0x1"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
 -device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
diff --git a/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.xml b/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.xml
index 215c196fbf..633aa684da 100644
--- a/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.xml
+++ b/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.xml
@@ -14,7 +14,7 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-ppc</emulator>
-    <controller type='usb' index='0' model='piix3-uhci'>
+    <controller type='usb' index='0' model='pci-ohci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxmlconfdata/ppce500-serial.ppc-latest.args b/tests/qemuxmlconfdata/ppce500-serial.ppc-latest.args
index 84abffdc26..21aa64b323 100644
--- a/tests/qemuxmlconfdata/ppce500-serial.ppc-latest.args
+++ b/tests/qemuxmlconfdata/ppce500-serial.ppc-latest.args
@@ -28,7 +28,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
 -kernel /media/ram/uImage \
 -initrd /media/ram/ramdisk \
 -append 'root=/dev/ram rw console=ttyS0,115200' \
--device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1"}' \
+-device '{"driver":"pci-ohci","id":"usb","bus":"pci.0","addr":"0x1"}' \
 -chardev pty,id=charserial0 \
 -serial chardev:charserial0 \
 -audiodev '{"id":"audio1","driver":"none"}' \
diff --git a/tests/qemuxmlconfdata/ppce500-serial.ppc-latest.xml b/tests/qemuxmlconfdata/ppce500-serial.ppc-latest.xml
index c6c41d7726..05076022a3 100644
--- a/tests/qemuxmlconfdata/ppce500-serial.ppc-latest.xml
+++ b/tests/qemuxmlconfdata/ppce500-serial.ppc-latest.xml
@@ -17,7 +17,7 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-ppc</emulator>
-    <controller type='usb' index='0' model='piix3-uhci'>
+    <controller type='usb' index='0' model='pci-ohci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxmlconfdata/usb-controller-automatic-realview.aarch64-latest.args b/tests/qemuxmlconfdata/usb-controller-automatic-realview.aarch64-latest.args
index 0d956241fc..ccbff79f35 100644
--- a/tests/qemuxmlconfdata/usb-controller-automatic-realview.aarch64-latest.args
+++ b/tests/qemuxmlconfdata/usb-controller-automatic-realview.aarch64-latest.args
@@ -25,7 +25,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.config \
 -rtc base=utc \
 -no-shutdown \
 -boot strict=on \
--device '{"driver":"pci-ohci","id":"usb","bus":"pci","addr":"0x1"}' \
+-device '{"driver":"qemu-xhci","id":"usb","bus":"pci","addr":"0x1"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxmlconfdata/usb-controller-automatic-realview.aarch64-latest.xml b/tests/qemuxmlconfdata/usb-controller-automatic-realview.aarch64-latest.xml
index 250da1b5e0..8bb25de20a 100644
--- a/tests/qemuxmlconfdata/usb-controller-automatic-realview.aarch64-latest.xml
+++ b/tests/qemuxmlconfdata/usb-controller-automatic-realview.aarch64-latest.xml
@@ -15,7 +15,7 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-aarch64</emulator>
-    <controller type='usb' index='0' model='pci-ohci'>
+    <controller type='usb' index='0' model='qemu-xhci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxmlconfdata/usb-controller-automatic-versatilepb.armv7l-latest.args b/tests/qemuxmlconfdata/usb-controller-automatic-versatilepb.armv7l-latest.args
index a05a413290..88fe2b62e8 100644
--- a/tests/qemuxmlconfdata/usb-controller-automatic-versatilepb.armv7l-latest.args
+++ b/tests/qemuxmlconfdata/usb-controller-automatic-versatilepb.armv7l-latest.args
@@ -26,7 +26,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-armv7ltest/.config \
 -rtc base=utc \
 -no-shutdown \
 -boot strict=on \
--device '{"driver":"pci-ohci","id":"usb","bus":"pci","addr":"0x1"}' \
+-device '{"driver":"qemu-xhci","id":"usb","bus":"pci","addr":"0x1"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxmlconfdata/usb-controller-automatic-versatilepb.armv7l-latest.xml b/tests/qemuxmlconfdata/usb-controller-automatic-versatilepb.armv7l-latest.xml
index 482029e3b0..8a12dda0f7 100644
--- a/tests/qemuxmlconfdata/usb-controller-automatic-versatilepb.armv7l-latest.xml
+++ b/tests/qemuxmlconfdata/usb-controller-automatic-versatilepb.armv7l-latest.xml
@@ -17,7 +17,7 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-armv7l</emulator>
-    <controller type='usb' index='0' model='pci-ohci'>
+    <controller type='usb' index='0' model='qemu-xhci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxmlconfdata/usb-controller-default-fallback-realview.aarch64-latest.args b/tests/qemuxmlconfdata/usb-controller-default-fallback-realview.aarch64-latest.args
index ccbff79f35..0d956241fc 100644
--- a/tests/qemuxmlconfdata/usb-controller-default-fallback-realview.aarch64-latest.args
+++ b/tests/qemuxmlconfdata/usb-controller-default-fallback-realview.aarch64-latest.args
@@ -25,7 +25,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.config \
 -rtc base=utc \
 -no-shutdown \
 -boot strict=on \
--device '{"driver":"qemu-xhci","id":"usb","bus":"pci","addr":"0x1"}' \
+-device '{"driver":"pci-ohci","id":"usb","bus":"pci","addr":"0x1"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxmlconfdata/usb-controller-default-fallback-realview.aarch64-latest.xml b/tests/qemuxmlconfdata/usb-controller-default-fallback-realview.aarch64-latest.xml
index 8bb25de20a..250da1b5e0 100644
--- a/tests/qemuxmlconfdata/usb-controller-default-fallback-realview.aarch64-latest.xml
+++ b/tests/qemuxmlconfdata/usb-controller-default-fallback-realview.aarch64-latest.xml
@@ -15,7 +15,7 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-aarch64</emulator>
-    <controller type='usb' index='0' model='qemu-xhci'>
+    <controller type='usb' index='0' model='pci-ohci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxmlconfdata/usb-controller-default-mac99ppc.ppc-latest.args b/tests/qemuxmlconfdata/usb-controller-default-mac99ppc.ppc-latest.args
index dcb2b49a53..da5437bc24 100644
--- a/tests/qemuxmlconfdata/usb-controller-default-mac99ppc.ppc-latest.args
+++ b/tests/qemuxmlconfdata/usb-controller-default-mac99ppc.ppc-latest.args
@@ -25,7 +25,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
 -rtc base=utc \
 -no-shutdown \
 -boot strict=on \
--device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1"}' \
+-device '{"driver":"pci-ohci","id":"usb","bus":"pci.0","addr":"0x1"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxmlconfdata/usb-controller-default-mac99ppc.ppc-latest.xml b/tests/qemuxmlconfdata/usb-controller-default-mac99ppc.ppc-latest.xml
index 02cd54d7f7..2fbca012c9 100644
--- a/tests/qemuxmlconfdata/usb-controller-default-mac99ppc.ppc-latest.xml
+++ b/tests/qemuxmlconfdata/usb-controller-default-mac99ppc.ppc-latest.xml
@@ -14,7 +14,7 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-ppc</emulator>
-    <controller type='usb' index='0' model='piix3-uhci'>
+    <controller type='usb' index='0' model='pci-ohci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxmlconfdata/usb-controller-default-versatilepb.armv7l-latest.args b/tests/qemuxmlconfdata/usb-controller-default-versatilepb.armv7l-latest.args
index 8636c54dfe..88fe2b62e8 100644
--- a/tests/qemuxmlconfdata/usb-controller-default-versatilepb.armv7l-latest.args
+++ b/tests/qemuxmlconfdata/usb-controller-default-versatilepb.armv7l-latest.args
@@ -26,7 +26,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-armv7ltest/.config \
 -rtc base=utc \
 -no-shutdown \
 -boot strict=on \
--device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci","addr":"0x1"}' \
+-device '{"driver":"qemu-xhci","id":"usb","bus":"pci","addr":"0x1"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxmlconfdata/usb-controller-default-versatilepb.armv7l-latest.xml b/tests/qemuxmlconfdata/usb-controller-default-versatilepb.armv7l-latest.xml
index e1a607c256..8a12dda0f7 100644
--- a/tests/qemuxmlconfdata/usb-controller-default-versatilepb.armv7l-latest.xml
+++ b/tests/qemuxmlconfdata/usb-controller-default-versatilepb.armv7l-latest.xml
@@ -17,7 +17,7 @@
   <on_crash>destroy</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-armv7l</emulator>
-    <controller type='usb' index='0' model='piix3-uhci'>
+    <controller type='usb' index='0' model='qemu-xhci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </controller>
     <controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index 84e37c4cb3..fe681d47d7 100644
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -1946,7 +1946,7 @@ mymain(void)
                  ARG_CAPS_ARCH, "armv7l",
                  ARG_CAPS_VER, "latest",
                  ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR,
-                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_LAST,
+                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_LAST,
                  ARG_END);
 
     DO_TEST_CAPS_ARCH_LATEST("usb-controller-automatic-realview", "aarch64");
@@ -1954,7 +1954,7 @@ mymain(void)
                  ARG_CAPS_ARCH, "aarch64",
                  ARG_CAPS_VER, "latest",
                  ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR,
-                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_LAST,
+                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_LAST,
                  ARG_END);
 
     DO_TEST_FULL("usb-controller-automatic-unavailable-pseries", ".ppc64-latest",
@@ -2046,26 +2046,26 @@ mymain(void)
     DO_TEST_FULL("usb-controller-default-fallback-versatilepb", ".armv7l-latest",
                  ARG_CAPS_ARCH, "armv7l",
                  ARG_CAPS_VER, "latest",
-                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_LAST,
+                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_LAST,
                  ARG_END);
     DO_TEST_FULL("usb-controller-default-unavailable-versatilepb", ".armv7l-latest",
                  ARG_CAPS_ARCH, "armv7l",
                  ARG_CAPS_VER, "latest",
                  ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR,
-                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_LAST,
+                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_LAST,
                  ARG_END);
 
     DO_TEST_CAPS_ARCH_LATEST("usb-controller-default-realview", "aarch64");
     DO_TEST_FULL("usb-controller-default-fallback-realview", ".aarch64-latest",
                  ARG_CAPS_ARCH, "aarch64",
                  ARG_CAPS_VER, "latest",
-                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_LAST,
+                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_LAST,
                  ARG_END);
     DO_TEST_FULL("usb-controller-default-unavailable-realview", ".aarch64-latest",
                  ARG_CAPS_ARCH, "aarch64",
                  ARG_CAPS_VER, "latest",
                  ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR,
-                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_LAST,
+                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_LAST,
                  ARG_END);
 
     /* The '-nousb' test case tests machine without a built-in USB controller */
@@ -2099,13 +2099,13 @@ mymain(void)
     DO_TEST_FULL("usb-controller-default-fallback-mac99ppc", ".ppc-latest",
                  ARG_CAPS_ARCH, "ppc",
                  ARG_CAPS_VER, "latest",
-                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_LAST,
+                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_LAST,
                  ARG_END);
     DO_TEST_FULL("usb-controller-default-unavailable-mac99ppc", ".ppc-latest",
                  ARG_CAPS_ARCH, "ppc",
                  ARG_CAPS_VER, "latest",
                  ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR,
-                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_LAST,
+                 ARG_QEMU_CAPS_DEL, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_LAST,
                  ARG_END);
 
     DO_TEST_FULL("usb-controller-default-fallback-powernv9", ".ppc64-latest",
-- 
2.50.1
Re: [PATCH 30/31] qemu: Finish cleaning up qemuDomainDefaultUSBControllerModel()
Posted by Peter Krempa via Devel 4 months, 3 weeks ago
On Tue, Aug 19, 2025 at 18:22:34 +0200, Andrea Bolognani via Devel wrote:
> At this point the USB controller model selection logic is
> mostly sane, but there are still a few remaining oddities.
> 
> First of all, piix3-uhci (USB1) is used in way too many
> contexts, either as default or fallback choice, while it
> really should be relegated to x86 guests only.
> 
> Additionally, we're explicitly defaulting to pci-ohci (USB1)
> for a couple of Arm machine types, and limiting the default
> of qemu-xhci (USB3) to aarch64 only instead of using it for
> 32-bit guests as well.
> 
> Address all of the aforementioned quirks. The result is a
> reasonably consistent experience across architectures, with
> USB3 generally being used whenever available, Intel-specific
> USB controllers only showing up in x86 guests, and pci-ohci
> acting as the vaguely reasonable falllback across the board.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  src/qemu/qemu_domain.c                        | 28 ++++++++-----------
>  ...iew-minimal.aarch64-latest.abi-update.args |  2 +-
>  ...view-minimal.aarch64-latest.abi-update.xml |  2 +-
>  ...rch64-realview-minimal.aarch64-latest.args |  2 +-
>  ...arch64-realview-minimal.aarch64-latest.xml |  2 +-
>  ...lepb-minimal.armv7l-latest.abi-update.args |  2 +-
>  ...ilepb-minimal.armv7l-latest.abi-update.xml |  2 +-
>  ...v7l-versatilepb-minimal.armv7l-latest.args |  2 +-
>  ...mv7l-versatilepb-minimal.armv7l-latest.xml |  2 +-
>  tests/qemuxmlconfdata/ppc-dtb.ppc-latest.args |  2 +-
>  tests/qemuxmlconfdata/ppc-dtb.ppc-latest.xml  |  2 +-
>  ...c-mac99-minimal.ppc-latest.abi-update.args |  2 +-
>  ...pc-mac99-minimal.ppc-latest.abi-update.xml |  2 +-
>  .../ppc-mac99-minimal.ppc-latest.args         |  2 +-
>  .../ppc-mac99-minimal.ppc-latest.xml          |  2 +-
>  .../ppce500-serial.ppc-latest.args            |  2 +-
>  .../ppce500-serial.ppc-latest.xml             |  2 +-
>  ...ler-automatic-realview.aarch64-latest.args |  2 +-
>  ...ller-automatic-realview.aarch64-latest.xml |  2 +-
>  ...r-automatic-versatilepb.armv7l-latest.args |  2 +-
>  ...er-automatic-versatilepb.armv7l-latest.xml |  2 +-
>  ...ault-fallback-realview.aarch64-latest.args |  2 +-
>  ...fault-fallback-realview.aarch64-latest.xml |  2 +-
>  ...ontroller-default-mac99ppc.ppc-latest.args |  2 +-
>  ...controller-default-mac99ppc.ppc-latest.xml |  2 +-
>  ...ler-default-versatilepb.armv7l-latest.args |  2 +-
>  ...ller-default-versatilepb.armv7l-latest.xml |  2 +-
>  tests/qemuxmlconftest.c                       | 16 +++++------
>  28 files changed, 46 insertions(+), 50 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index df64ddbec1..2c7d91f597 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4334,7 +4334,7 @@ qemuDomainDefaultUSBControllerModel(const virDomainDef *def,
>          return VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;
>      }
>  
> -    if (def->os.arch == VIR_ARCH_AARCH64) {
> +    if (ARCH_IS_ARM(def->os.arch)) {
>          /* Prefer USB3 */
>          if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI))
>              return VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI;
> @@ -4366,14 +4366,17 @@ qemuDomainDefaultUSBControllerModel(const virDomainDef *def,
>          return VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;
>      }
>  
> -    /* The default USB controller is piix3-uhci (USB1) if available.
> -     * This choice is a fairly poor one, rooted primarily in
> -     * historical reasons; thankfully, in most cases we will have
> -     * picked a much more reasonable value before ever getting here */
> -    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI))
> -        return VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
> -    else if (!ARCH_IS_X86(def->os.arch) &&
> -             virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI))
> +    if (ARCH_IS_X86(def->os.arch)) {
> +        /* Use piix3-uhci (USB1) for backwards compatibility */
> +        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI))
> +            return VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
> +        return VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;
> +    }
> +
> +    /* Most common architectures and machine types have been already
> +     * handled above; for the remaining cases, pci-ohci (USB1) is a
> +     * sensible enough fallback */
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI))
>          return VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI;
>  
>      return VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;


This hunk is partially what I'd want to see in patch 26 (making x86 a
separate case with explicit logic).



> @@ -4425,13 +4428,6 @@ qemuDomainDefaultUSBControllerModelAutoAdded(const virDomainDef *def,
>          }
>      }
>  
> -    if (ARCH_IS_ARM(def->os.arch)) {
> -        if (STREQ(def->os.machine, "versatilepb") ||
> -            STRPREFIX(def->os.machine, "realview-eb"))
> -            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI))
> -                return VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI;
> -    }
> -
>      return VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;
>  }

[...]


> diff --git a/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.args b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.args
> index a05a413290..88fe2b62e8 100644
> --- a/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.args
> +++ b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.args
> @@ -26,7 +26,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-armv7ltest/.config \
>  -rtc base=utc \
>  -no-shutdown \
>  -boot strict=on \
> --device '{"driver":"pci-ohci","id":"usb","bus":"pci","addr":"0x1"}' \
> +-device '{"driver":"qemu-xhci","id":"usb","bus":"pci","addr":"0x1"}' \

This change seems to have happened also in code paths not allowing ABI
update at least according to the filename.


>  -audiodev '{"id":"audio1","driver":"none"}' \
>  -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
>  -msg timestamp=on
> diff --git a/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.xml b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.xml
> index 7b21b24927..a9da2a3b13 100644
> --- a/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.xml
> +++ b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.xml
> @@ -17,7 +17,7 @@
>    <on_crash>destroy</on_crash>
>    <devices>
>      <emulator>/usr/bin/qemu-system-armv7l</emulator>
> -    <controller type='usb' index='0' model='pci-ohci'>
> +    <controller type='usb' index='0' model='qemu-xhci'>
>        <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
>      </controller>
>      <controller type='pci' index='0' model='pci-root'/>
> diff --git a/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.args b/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.args
> index 8032ad7f0e..bdc86620c8 100644
> --- a/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.args
> +++ b/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.args
> @@ -29,7 +29,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
>  -initrd /media/ram/ramdisk \
>  -append 'root=/dev/ram rw console=ttyS0,115200' \
>  -dtb /media/ram/test.dtb \
> --device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1"}' \
> +-device '{"driver":"pci-ohci","id":"usb","bus":"pci.0","addr":"0x1"}' \
>  -chardev pty,id=charserial0 \
>  -serial chardev:charserial0 \
>  -audiodev '{"id":"audio1","driver":"none"}' \
> diff --git a/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.xml b/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.xml
> index 400f749eb0..31fcc3d053 100644
> --- a/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.xml
> +++ b/tests/qemuxmlconfdata/ppc-dtb.ppc-latest.xml
> @@ -18,7 +18,7 @@
>    <on_crash>destroy</on_crash>
>    <devices>
>      <emulator>/usr/bin/qemu-system-ppc</emulator>
> -    <controller type='usb' index='0' model='piix3-uhci'>
> +    <controller type='usb' index='0' model='pci-ohci'>
>        <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
>      </controller>
>      <controller type='pci' index='0' model='pci-root'/>
> diff --git a/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.args b/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.args
> index 8600eec328..9c7c884c83 100644
> --- a/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.args
> +++ b/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.args
> @@ -25,7 +25,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \
>  -rtc base=utc \
>  -no-shutdown \
>  -boot strict=on \
> --device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1"}' \
> +-device '{"driver":"pci-ohci","id":"usb","bus":"pci.0","addr":"0x1"}' \
>  -audiodev '{"id":"audio1","driver":"none"}' \
>  -device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \
>  -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
> diff --git a/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.xml b/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.xml
> index 215c196fbf..633aa684da 100644
> --- a/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.xml
> +++ b/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.xml
> @@ -14,7 +14,7 @@
>    <on_crash>destroy</on_crash>
>    <devices>
>      <emulator>/usr/bin/qemu-system-ppc</emulator>
> -    <controller type='usb' index='0' model='piix3-uhci'>
> +    <controller type='usb' index='0' model='pci-ohci'>

I think I need to think about this a bit more. I agree that the
controller we picked didn't make sense for this machine, but in cases
when it did work (e.g. when you run linux with the proper driver) you
get something which doesn't resemble real hardware but likely works
better than 'pci-ohci'.

So I'm not sure about the downgrade in this case, although we're
unlikely to break anything that'd be used widely in this case, breaking
it would go against our philosophy
Re: [PATCH 30/31] qemu: Finish cleaning up qemuDomainDefaultUSBControllerModel()
Posted by Andrea Bolognani via Devel 4 months, 3 weeks ago
On Fri, Sep 19, 2025 at 11:39:05AM +0200, Peter Krempa wrote:
> On Tue, Aug 19, 2025 at 18:22:34 +0200, Andrea Bolognani via Devel wrote:
> > +++ b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.args
> > @@ -26,7 +26,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-armv7ltest/.config \
> >  -rtc base=utc \
> >  -no-shutdown \
> >  -boot strict=on \
> > --device '{"driver":"pci-ohci","id":"usb","bus":"pci","addr":"0x1"}' \
> > +-device '{"driver":"qemu-xhci","id":"usb","bus":"pci","addr":"0x1"}' \
>
> This change seems to have happened also in code paths not allowing ABI
> update at least according to the filename.

It only affects domains for which a model was not picked in the past.
So effectively only new domains, regardless of the flags. Existing
domains will keep using whatever model they're configured to use.

> > +++ b/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.xml
> > @@ -14,7 +14,7 @@
> >    <on_crash>destroy</on_crash>
> >    <devices>
> >      <emulator>/usr/bin/qemu-system-ppc</emulator>
> > -    <controller type='usb' index='0' model='piix3-uhci'>
> > +    <controller type='usb' index='0' model='pci-ohci'>
>
> I think I need to think about this a bit more. I agree that the
> controller we picked didn't make sense for this machine, but in cases
> when it did work (e.g. when you run linux with the proper driver) you
> get something which doesn't resemble real hardware but likely works
> better than 'pci-ohci'.
>
> So I'm not sure about the downgrade in this case, although we're
> unlikely to break anything that'd be used widely in this case, breaking
> it would go against our philosophy

Is pci-ohci really a downgrade compared to piix3-uhci? They're both
USB1 controllers. They should both have wide driver support and
provide basic USB functionality. One just looks silly for non-x86
machines.

Plus as mentioned above existing guests will retain the existing
model, so no actual breakage will happen.

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 30/31] qemu: Finish cleaning up qemuDomainDefaultUSBControllerModel()
Posted by Peter Krempa via Devel 4 months, 3 weeks ago
On Fri, Sep 19, 2025 at 10:34:39 -0500, Andrea Bolognani wrote:
> On Fri, Sep 19, 2025 at 11:39:05AM +0200, Peter Krempa wrote:
> > On Tue, Aug 19, 2025 at 18:22:34 +0200, Andrea Bolognani via Devel wrote:
> > > +++ b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.args
> > > @@ -26,7 +26,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-armv7ltest/.config \
> > >  -rtc base=utc \
> > >  -no-shutdown \
> > >  -boot strict=on \
> > > --device '{"driver":"pci-ohci","id":"usb","bus":"pci","addr":"0x1"}' \
> > > +-device '{"driver":"qemu-xhci","id":"usb","bus":"pci","addr":"0x1"}' \
> >
> > This change seems to have happened also in code paths not allowing ABI
> > update at least according to the filename.
> 
> It only affects domains for which a model was not picked in the past.
> So effectively only new domains, regardless of the flags. Existing
> domains will keep using whatever model they're configured to use.

Beware that we historically considered also virDomainCreateXML
(transient wit possibly not fully filled XML) to be compatible across
runs. I know that we already didn't obey this in some cases but we
should consider this every time.

Especially in case of these limited boards the hardware you'd normally
built in will certainly not be XHCI

> > > +++ b/tests/qemuxmlconfdata/ppc-mac99-minimal.ppc-latest.abi-update.xml
> > > @@ -14,7 +14,7 @@
> > >    <on_crash>destroy</on_crash>
> > >    <devices>
> > >      <emulator>/usr/bin/qemu-system-ppc</emulator>
> > > -    <controller type='usb' index='0' model='piix3-uhci'>
> > > +    <controller type='usb' index='0' model='pci-ohci'>
> >
> > I think I need to think about this a bit more. I agree that the
> > controller we picked didn't make sense for this machine, but in cases
> > when it did work (e.g. when you run linux with the proper driver) you
> > get something which doesn't resemble real hardware but likely works
> > better than 'pci-ohci'.
> >
> > So I'm not sure about the downgrade in this case, although we're
> > unlikely to break anything that'd be used widely in this case, breaking
> > it would go against our philosophy
> 
> Is pci-ohci really a downgrade compared to piix3-uhci? They're both
> USB1 controllers. They should both have wide driver support and
> provide basic USB functionality. One just looks silly for non-x86
> machines.

Uh, yeah. I somehow thought this was 'ehci'/usb2.

> Plus as mentioned above existing guests will retain the existing
> model, so no actual breakage will happen.

See above.
Re: [PATCH 30/31] qemu: Finish cleaning up qemuDomainDefaultUSBControllerModel()
Posted by Andrea Bolognani via Devel 4 months, 3 weeks ago
On Fri, Sep 19, 2025 at 07:00:17PM +0200, Peter Krempa wrote:
> On Fri, Sep 19, 2025 at 10:34:39 -0500, Andrea Bolognani wrote:
> > On Fri, Sep 19, 2025 at 11:39:05AM +0200, Peter Krempa wrote:
> > > On Tue, Aug 19, 2025 at 18:22:34 +0200, Andrea Bolognani via Devel wrote:
> > > > +++ b/tests/qemuxmlconfdata/armv7l-versatilepb-minimal.armv7l-latest.args
> > > > @@ -26,7 +26,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-armv7ltest/.config \
> > > >  -rtc base=utc \
> > > >  -no-shutdown \
> > > >  -boot strict=on \
> > > > --device '{"driver":"pci-ohci","id":"usb","bus":"pci","addr":"0x1"}' \
> > > > +-device '{"driver":"qemu-xhci","id":"usb","bus":"pci","addr":"0x1"}' \
> > >
> > > This change seems to have happened also in code paths not allowing ABI
> > > update at least according to the filename.
> >
> > It only affects domains for which a model was not picked in the past.
> > So effectively only new domains, regardless of the flags. Existing
> > domains will keep using whatever model they're configured to use.
>
> Beware that we historically considered also virDomainCreateXML
> (transient wit possibly not fully filled XML) to be compatible across
> runs. I know that we already didn't obey this in some cases but we
> should consider this every time.

Fair point. I feel that the actual impact on users is going to be
basically zero in this case, so I'm inclined to focus more on
reaching well-defined, reasonably consistent behavior across the
board that's implemented in a maintainable manner rather than
necessarily preserving the existing behavior for all niche machine
types in all niche scenarios.

> Especially in case of these limited boards the hardware you'd normally
> built in will certainly not be XHCI

I'd be fine using pci-ohci as the baseline for everything that is not
one of the "explicitly supported" machines (that is, virt across
architectures, i440fx and q35 on x86, pseries on ppc64 and
s390-ccw-virtio on s390x). That's pretty much exactly how things look
once this series has been applied.

In this case, however, I'm simply extending the existing default for
64-bit Arm machines to 32-bit ones.

I don't think it makes sense to differentiate the model based on the
number of bits - QEMU doesn't. So either we downgrade the 64-bit
machines or upgrade the 32-bit ones.

Looking at what QEMU does, the default for this machine type is
pci-ohci. So I'd say let's go with that.

-- 
Andrea Bolognani / Red Hat / Virtualization