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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
