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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux