[libvirt] [PATCH v2 1/5] qemu: change the logic of setting default USB controller

[Andrea Bolognani]

On Thu, 2017-04-27 at 18:09 +0200, Pavel Hrdina wrote:
> @@ -3200,11 +3200,17 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
>               * when the relevant device is not available.
>               *
>               * See qemuBuildControllerDevCommandLine() */
> -            if (ARCH_IS_S390(def->os.arch) &&
> -                cont->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> -                /* set the default USB model to none for s390 unless an
> -                 * address is found */
> -                cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE;
> +
> +            /* Default USB controller is piix3-uhci if available. */
> +            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI))
> +                cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
> +
> +            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 */
> +                    cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE;
> +                }

You should add an else branch where you unset cont->model
explicitly, like you do for ppc64 below, otherwise s390
guests will start being assigned piix3-uhci controllers in
some situations.

We don't have checks for that in our test suite, and it's
probably not going to be much of an issue in the real world
at least as far as downstream distros are concerned, but it
should be avoided nonetheless.


Overall I'm not convinced this is any more readable than
what we had before or that it makes subsequent changes
any easier, so I'd prefer if you'd just drop this patch.

-- 
Andrea Bolognani / Red Hat / Virtualization