On Thu, Feb 15, 2024 at 05:01:44PM +0100, Peter Krempa wrote:
On Wed, Feb 14, 2024 at 18:11:18 +0100, Andrea Bolognani wrote:
Right now we call qemuValidateDomainDeviceDefControllerUSB() quite late, just as we're generating the QEMU command line.
The intention here is to prevent configurations from being rejected, even though a default USB controller model could not be found, because using -usb could work as a last resort.
As it turns out, this premise is flawed, as can easily be demonstrated by using a build of QEMU which has the default USB controller compiled out:
$ qemu-system-x86_64 -M pc -device piix3-usb-uhci 'piix3-usb-uhci' is not a valid device model name
$ qemu-system-x86_64 -M pc -usb missing object type 'piix3-usb-uhci'
Could you please elaborate how you've got this? I had to disable the CONFIG_I440FX=n option to achieve that (without messing with the machine dependencies in the first place) which also means that that the 'pc' machine type will not work:
'piix3-usb-uhci' is provided by 'hw/usb/hcd-uhci.c', which is conditionally compiled based on the follwing rule:
system_ss.add(when: 'CONFIG_USB_UHCI', if_true: files('hcd-uhci.c'))
USB_UHCI is selected(required) by 'config PIIX' (hw/isa/Kconfig).
PIIX is selected by 'config I440FX'.
Thus it is impossible to build qemu with a 'pc' machine type but missing 'piix3-usb-uhci.
By breaking the 'config PIIX' option you can achieve that.
Thus by definition all 'pc' machine-type based devices must have the 'piix3-usb-uhci' type compiled in.
And which thus implies that '-usb' will never be used in such case.
The exact diff is at the bottom of the message, but yeah, I've had to hack things up quite a bit just to convince QEMU's build system to produce a binary that contains the pc machine type but no piix3-usb-uhci device. To be honest I don't understand QEMU's build system very well, so it's possible that the only reason why I had to go to such lengths is lack of knowledge. The experience you describe above seems to suggest otherwise though :)
In other words, if the device that needs to be built into QEMU in order for -usb to work was available, we would have detected it and used it via -device instead; if we didn't, using -usb won't change anything.
So if the above is the case, I'd rephrase this paragraph to say that qemu can't be built without the explicitly detectable controller.
Now the question is only whether that applies for qemu-4.2 and other machines. as well. But if yes, all the -usb thing can be removed.
Note that we're discussing two slightly different claims: * that -usb cannot work when the default USB controller has been compiled out; * that the default USB controller cannot be compiled out. Either one being true across architectures and machine types is sufficient ground for dropping -usb usage from libvirt. I'm quite convinced that the former always holds true, and that's what I was referring to in the commit message. Anyway, we can just ask an actual QEMU developer to confirm.
+++ b/src/qemu/qemu_validate.c @@ -3523,6 +3523,69 @@ qemuValidateDomainDeviceDefControllerSCSI(const virDomainControllerDef *controll }
+static int +qemuControllerModelUSBToCaps(int model)
Any reason why 'model' doesn't use the proper type: virDomainControllerModelUSB and the return value type isn't virQEMUCapsFlags?
I've just moved the code exacty as is, modulo adding the if (def->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) return 0; bit without which it would clearly not work correctly. I can add a further commit that cleans things up a bit.
The summary and first paragraph of the commit message makes it seem that this is just moving when the validation happens but the commit is in fact severly reworking the semantics of the validation too. Please clarify that in the commit message; especially the summary.
Okay, I'll try to come up with something that does a better job at explaining the changes. diff --git a/configs/devices/i386-softmmu/default.mak b/configs/devices/i386-softmmu/default.mak index 598c6646df..bbf783db19 100644 --- a/configs/devices/i386-softmmu/default.mak +++ b/configs/devices/i386-softmmu/default.mak @@ -26,7 +26,7 @@ # Boards: # -CONFIG_ISAPC=y +CONFIG_ISAPC=n CONFIG_I440FX=y CONFIG_Q35=y CONFIG_MICROVM=y diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig index 040a18c070..13cd91e587 100644 --- a/hw/isa/Kconfig +++ b/hw/isa/Kconfig @@ -42,7 +42,6 @@ config PIIX select IDE_PIIX select ISA_BUS select MC146818RTC - select USB_UHCI config VT82C686 bool @@ -51,7 +50,6 @@ config VT82C686 select ACPI_SMBUS select SERIAL_ISA select FDC_ISA - select USB_UHCI select APM select I8254 select I8257 diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig index 0f486764ed..feb4bb5b5a 100644 --- a/hw/usb/Kconfig +++ b/hw/usb/Kconfig @@ -3,7 +3,7 @@ config USB config USB_UHCI bool - default y if PCI_DEVICES + default n depends on PCI select USB -- Andrea Bolognani / Red Hat / Virtualization