[PATCH 0/4] qemu: Automatically add USB controller if USB devices are present
Makes things more user-friendly for those machine types where a default USB controller is normally not automatically added. https://issues.redhat.com/browse/RHEL-2455 Andrea Bolognani (4): tests: Add usb-controller-automatic-with-devices-virt-aarch64 conf: Rename virDomainDefHasUSB() to virDomainDefHasUSBControllers() conf: Introduce virDomainDefHasUSBDevices() qemu: Automatically add USB controller if USB devices are present src/conf/domain_conf.c | 31 +++++++++++++- src/conf/domain_conf.h | 3 +- src/conf/domain_validate.c | 2 +- src/libvirt_private.syms | 3 +- src/qemu/qemu_domain_address.c | 2 +- src/qemu/qemu_postparse.c | 11 +++++ ...h-devices-virt-aarch64.aarch64-latest.args | 36 ++++++++++++++++ ...th-devices-virt-aarch64.aarch64-latest.xml | 41 +++++++++++++++++++ ...er-automatic-with-devices-virt-aarch64.xml | 14 +++++++ tests/qemuxmlconftest.c | 2 + 10 files changed, 139 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.args create mode 100644 tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.xml create mode 100644 tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.xml -- 2.52.0
This demonstrates how, when a machine type for which a default USB controller is usually not added automatically is used, including USB devices in the configuration results in an error. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- ...th-devices-virt-aarch64.aarch64-latest.err | 1 + ...th-devices-virt-aarch64.aarch64-latest.xml | 28 +++++++++++++++++++ ...er-automatic-with-devices-virt-aarch64.xml | 14 ++++++++++ tests/qemuxmlconftest.c | 2 ++ 4 files changed, 45 insertions(+) create mode 100644 tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.err create mode 100644 tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.xml create mode 100644 tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.xml diff --git a/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.err b/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.err new file mode 100644 index 0000000000..90050be94b --- /dev/null +++ b/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.err @@ -0,0 +1 @@ +unsupported configuration: USB is disabled for this domain, but USB devices are present in the domain XML diff --git a/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.xml b/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.xml new file mode 100644 index 0000000000..431ff6703b --- /dev/null +++ b/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.xml @@ -0,0 +1,28 @@ +<domain type='qemu'> + <name>test</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='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <gic version='2'/> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>cortex-a15</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <input type='tablet' bus='usb'/> + <input type='keyboard' bus='usb'/> + <audio id='1' type='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.xml b/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.xml new file mode 100644 index 0000000000..1d0e1bb3c5 --- /dev/null +++ b/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.xml @@ -0,0 +1,14 @@ +<domain type='qemu'> + <name>test</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <input type='tablet' bus='usb'/> + <input type='keyboard' bus='usb'/> + </devices> +</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 89b8ad1a35..835a388263 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2141,6 +2141,8 @@ mymain(void) 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_FAILURE("usb-controller-automatic-with-devices-virt-aarch64", "aarch64"); + DO_TEST_CAPS_LATEST_PARSE_ERROR("usb-controller-default-isapc"); DO_TEST_CAPS_LATEST_PARSE_ERROR("usb-controller-default-microvm"); DO_TEST_CAPS_LATEST("usb-controller-default-i440fx"); -- 2.52.0
On Mon, Jan 12, 2026 at 15:49:13 +0100, Andrea Bolognani via Devel wrote:
This demonstrates how, when a machine type for which a default USB controller is usually not added automatically is used, including USB devices in the configuration results in an error.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- ...th-devices-virt-aarch64.aarch64-latest.err | 1 + ...th-devices-virt-aarch64.aarch64-latest.xml | 28 +++++++++++++++++++ ...er-automatic-with-devices-virt-aarch64.xml | 14 ++++++++++ tests/qemuxmlconftest.c | 2 ++ 4 files changed, 45 insertions(+) create mode 100644 tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.err create mode 100644 tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.xml create mode 100644 tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.xml
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The new name more accurately describes what the function actually does. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 2 +- src/conf/domain_validate.c | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain_address.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9ca5c2450c..e7fa4cd769 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5413,7 +5413,7 @@ virDomainSCSIDriveAddressIsUsed(const virDomainDef *def, bool -virDomainDefHasUSB(const virDomainDef *def) +virDomainDefHasUSBControllers(const virDomainDef *def) { size_t i; @@ -29921,7 +29921,7 @@ virDomainDefCompatibleDevice(virDomainDef *def, } } - if (!virDomainDefHasUSB(def) && + if (!virDomainDefHasUSBControllers(def) && def->os.type != VIR_DOMAIN_OSTYPE_EXE && virDomainDeviceIsUSB(dev, false)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cb35ff06bd..a2684ea78d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3682,7 +3682,7 @@ bool virDomainSCSIDriveAddressIsUsed(const virDomainDef *def, const virDomainDeviceDriveAddress *addr); -bool virDomainDefHasUSB(const virDomainDef *def); +bool virDomainDefHasUSBControllers(const virDomainDef *def); bool virDomainDeviceAliasIsUserAlias(const char *aliasStr); diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 7346a61731..647cb23b19 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1166,7 +1166,7 @@ virDomainRedirdevDefValidate(const virDomainDef *def, const virDomainRedirdevDef *redirdev) { if (redirdev->bus == VIR_DOMAIN_REDIRDEV_BUS_USB && - !virDomainDefHasUSB(def)) { + !virDomainDefHasUSBControllers(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("cannot add redirected USB device: USB is disabled for this domain")); return -1; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4e57e4a8f6..3fc97bd124 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -349,7 +349,7 @@ virDomainDefHasOldStyleROUEFI; virDomainDefHasOldStyleUEFI; virDomainDefHasPCIHostdev; virDomainDefHasTimer; -virDomainDefHasUSB; +virDomainDefHasUSBControllers; virDomainDefHasVcpusOffline; virDomainDefHasVDPANet; virDomainDefIDsParseString; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 7233df888c..e835266f40 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -3038,7 +3038,7 @@ qemuDomainUSBAddressAddHubs(virDomainDef *def) &data, false)); - if (data.count > 0 && !virDomainDefHasUSB(def)) { + if (data.count > 0 && !virDomainDefHasUSBControllers(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("USB is disabled for this domain, but USB devices are present in the domain XML")); return -1; -- 2.52.0
On Mon, Jan 12, 2026 at 15:49:14 +0100, Andrea Bolognani via Devel wrote:
The new name more accurately describes what the function actually does.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 2 +- src/conf/domain_validate.c | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain_address.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This is similar to the existing virDomainDefHasUSBControllers() function, except that it looks for USB devices in the domain definition instead. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 27 +++++++++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + 3 files changed, 29 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e7fa4cd769..61a3280e9e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5427,6 +5427,33 @@ virDomainDefHasUSBControllers(const virDomainDef *def) } +static int +virDomainDefHasUSBDevicesIter(virDomainDeviceInfo *info G_GNUC_UNUSED, + void *opaque) +{ + bool *found = (bool *) opaque; + + *found = true; + + return 0; +} + + +bool +virDomainDefHasUSBDevices(const virDomainDef *def) +{ + bool found = false; + + if (virDomainUSBDeviceDefForeach((virDomainDef *) def, + virDomainDefHasUSBDevicesIter, + &found, false) < 0) { + return false; + } + + return found; +} + + bool virDomainDefLifecycleActionAllowed(virDomainLifecycle type, virDomainLifecycleAction action) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a2684ea78d..448361ab95 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3683,6 +3683,7 @@ virDomainSCSIDriveAddressIsUsed(const virDomainDef *def, const virDomainDeviceDriveAddress *addr); bool virDomainDefHasUSBControllers(const virDomainDef *def); +bool virDomainDefHasUSBDevices(const virDomainDef *def); bool virDomainDeviceAliasIsUserAlias(const char *aliasStr); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3fc97bd124..ce93a13216 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -350,6 +350,7 @@ virDomainDefHasOldStyleUEFI; virDomainDefHasPCIHostdev; virDomainDefHasTimer; virDomainDefHasUSBControllers; +virDomainDefHasUSBDevices; virDomainDefHasVcpusOffline; virDomainDefHasVDPANet; virDomainDefIDsParseString; -- 2.52.0
The current behavior is to error out, but that's not very helpful. Automatically add the necessary USB controller instead. Resolves: https://issues.redhat.com/browse/RHEL-2455 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_postparse.c | 11 ++++++ ...h-devices-virt-aarch64.aarch64-latest.args | 36 +++++++++++++++++++ ...th-devices-virt-aarch64.aarch64-latest.err | 1 - ...th-devices-virt-aarch64.aarch64-latest.xml | 13 +++++++ tests/qemuxmlconftest.c | 2 +- 5 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.args delete mode 100644 tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.err diff --git a/src/qemu/qemu_postparse.c b/src/qemu/qemu_postparse.c index 8940cb09b3..d3548b24d8 100644 --- a/src/qemu/qemu_postparse.c +++ b/src/qemu/qemu_postparse.c @@ -1329,6 +1329,17 @@ qemuDomainDefAddDefaultDevices(virQEMUDriver *driver, return -1; } + /* If the default UBS controller would normally not be added for the + * machine but USB devices are present in the configuration, it's more + * user-friendly to automatically add the USB controller instead of + * requiring the user to take care of that manually. Of course this is + * still subject to the logic below, i.e. we will only add the + * controller if a suitable model can be figured out and an explicit + * one was not already present */ + if (!addDefaultUSB && virDomainDefHasUSBDevices(def)) { + addDefaultUSB = true; + } + if (addDefaultUSB && usbModel == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT) { usbModel = qemuDomainDefaultUSBControllerModelAutoAdded(def, qemuCaps); diff --git a/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.args b/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.args new file mode 100644 index 0000000000..850eaa7ec0 --- /dev/null +++ b/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.args @@ -0,0 +1,36 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-test \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-test/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-test/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-test/.config \ +/usr/bin/qemu-system-aarch64 \ +-name guest=test,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-test/master-key.aes"}' \ +-machine virt,usb=off,gic-version=2,dump-guest-core=off,memory-backend=mach-virt.ram,acpi=off \ +-accel tcg \ +-cpu cortex-a15 \ +-m size=219136k \ +-object '{"qom-type":"memory-backend-ram","id":"mach-virt.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device '{"driver":"pcie-root-port","port":8,"chassis":1,"id":"pci.1","bus":"pcie.0","multifunction":true,"addr":"0x1"}' \ +-device '{"driver":"pcie-root-port","port":9,"chassis":2,"id":"pci.2","bus":"pcie.0","addr":"0x1.0x1"}' \ +-device '{"driver":"qemu-xhci","id":"usb","bus":"pci.1","addr":"0x0"}' \ +-device '{"driver":"usb-tablet","id":"input0","bus":"usb.0","port":"1"}' \ +-device '{"driver":"usb-kbd","id":"input1","bus":"usb.0","port":"2"}' \ +-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-with-devices-virt-aarch64.aarch64-latest.err b/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.err deleted file mode 100644 index 90050be94b..0000000000 --- a/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.err +++ /dev/null @@ -1 +0,0 @@ -unsupported configuration: USB is disabled for this domain, but USB devices are present in the domain XML diff --git a/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.xml b/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.xml index 431ff6703b..7923087433 100644 --- a/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.xml +++ b/tests/qemuxmlconfdata/usb-controller-automatic-with-devices-virt-aarch64.aarch64-latest.xml @@ -20,7 +20,20 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' index='0' model='qemu-xhci'> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='1' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='2' port='0x9'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> <input type='tablet' bus='usb'/> <input type='keyboard' bus='usb'/> <audio id='1' type='none'/> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 835a388263..4c1ecddc4a 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2141,7 +2141,7 @@ mymain(void) 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_FAILURE("usb-controller-automatic-with-devices-virt-aarch64", "aarch64"); + DO_TEST_CAPS_ARCH_LATEST("usb-controller-automatic-with-devices-virt-aarch64", "aarch64"); DO_TEST_CAPS_LATEST_PARSE_ERROR("usb-controller-default-isapc"); DO_TEST_CAPS_LATEST_PARSE_ERROR("usb-controller-default-microvm"); -- 2.52.0
On Mon, Jan 12, 2026 at 15:49:16 +0100, Andrea Bolognani via Devel wrote:
The current behavior is to error out, but that's not very helpful. Automatically add the necessary USB controller instead.
IMO this creates the same situation we're in currently with the addition of SCSI controllers. We add 'lsilogic' as controller if you don't have any which is sub-optimal, but it's the default and the behaviour we've standardized on thus we can't really change it. The auto-added USB controller will be likely sub-optimal either now or in the future. While it seems helpful to add it it can create headaches. To me it seems that the users simply should add the controller manually if the platform doesn't have one.
participants (2)
-
Andrea Bolognani -
Peter Krempa