On Wed, Jan 24, 2024 at 20:37:39 +0100, Andrea Bolognani wrote:
Extract the logic from qemuDomainControllerDefPostParse().
The code is mostly unchanged, but there's a subtle difference: the piix3-uhci has been moved from the top of the chunk to the bottom. This is because the original code set cont->model directly, which made it okay to start with a suboptimal default and subsequently overwrite it with a better one; now that we return the selected value instead, we need to make sure that we only ever consider piix3-uhci if everything else has failed.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_domain.c | 100 +++++++++++++++++++++++------------------ 1 file changed, 56 insertions(+), 44 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7475fb4f39..09f572b0b5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4151,6 +4151,61 @@ qemuDomainDefaultSCSIControllerModel(const virDomainDef *def, }
+static int +qemuDomainDefaultUSBControllerModel(const virDomainDef *def, + const virDomainControllerDef *cont, + virQEMUCaps *qemuCaps, + unsigned int parseFlags) +{ + /* Pick a suitable default model for the USB controller if none + * has been selected by the user and we have the qemuCaps for + * figuring out which controllers are supported. + * + * We rely on device availability instead of setting the model + * unconditionally because, for some machine types, there's a + * chance we will get away with using the legacy USB controller + * when the relevant device is not available. + * + * See qemuBuildControllersCommandLine() */ + + if (ARCH_IS_S390(def->os.arch)) { + if (cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + /* set the default USB model to none for s390 unless an + * address is found */ + return VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE; + } + } else if (ARCH_IS_PPC64(def->os.arch)) { + /* To not break migration we need to set default USB controller + * for ppc64 to pci-ohci if we cannot change ABI of the VM. + * The nec-usb-xhci or qemu-xhci controller is used as default + * only for newly defined domains or devices. */ + if ((parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI)) { + return VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI; + } else if ((parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI)) { + return VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI; + } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI)) { + return VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI; + } else { + /* Explicitly fallback to legacy USB controller for PPC64. */ + return -1;
So this is extracting the logic of passing VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT as model, which is -1, thus at this point faithful representation what the code did. The very next commit though declares -1 to be an error in the function comment which is not true. Extract it here as the proper enum value.
+ } + } else if (def->os.arch == VIR_ARCH_AARCH64) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI)) + return VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI; + else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI)) + return VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI; + } + + /* Default USB controller is piix3-uhci if available. */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI)) + return VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI; + + return -1;
This is later on documented via a comment but also should use the proper enum value. Reviewed-by: Peter Krempa <pkrempa@redhat.com>