[libvirt] [PATCH 9/9] qemu: command: Simplify USB controller model selection

[Ján Tomko]

On Fri, Jul 29, 2016 at 07:46:29PM +0200, Andrea Bolognani wrote:
>Since we now pick the default USB controller model when parsing
>the guest XML, we can get rid of some duplicated code so that
>the default model selection happens in one place only.
>
>Add some comments as well.
>---
> src/qemu/qemu_command.c | 60 ++++++++++++++++++++++++-------------------------
> 1 file changed, 30 insertions(+), 30 deletions(-)
>
>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>index 5325f48..d53620c 100644
>--- a/src/qemu/qemu_command.c
>+++ b/src/qemu/qemu_command.c
>@@ -2387,8 +2387,7 @@ qemuControllerModelUSBToCaps(int model)
>
>
> static int
>-qemuBuildUSBControllerDevStr(const virDomainDef *domainDef,
>-                             virDomainControllerDefPtr def,
>+qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def,
>                              virQEMUCapsPtr qemuCaps,
>                              virBuffer *buf)
> {
>@@ -2398,10 +2397,9 @@ qemuBuildUSBControllerDevStr(const virDomainDef *domainDef,
>     model = def->model;
>
>     if (model == -1) {
>-        if ARCH_IS_PPC64(domainDef->os.arch)
>-            model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI;
>-        else
>-            model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
>+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+                       "%s", _("no model provided for USB controller"));
>+        return -1;
>     }
>
>     smodel = qemuControllerModelUSBTypeToString(model);
>@@ -2630,7 +2628,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
>         break;
>
>     case VIR_DOMAIN_CONTROLLER_TYPE_USB:
>-        if (qemuBuildUSBControllerDevStr(domainDef, def, qemuCaps, &buf) == -1)
>+        if (qemuBuildUSBControllerDevStr(def, qemuCaps, &buf) == -1)
>             goto error;
>
>         if (nusbcontroller)
>@@ -3010,30 +3008,29 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
>
>             if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
>                 cont->model == -1 &&
>-                !qemuDomainMachineIsQ35(def)) {
>-                bool need_legacy = false;
>-
>-                /* We're not using legacy usb controller for q35 */
>-                if (ARCH_IS_PPC64(def->os.arch)) {
>-                    /* For ppc64 the legacy was OHCI */
>-                    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI))
>-                        need_legacy = true;
>-                } else {
>-                    /* For anything else, we used PIIX3_USB_UHCI */
>-                    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI))
>-                        need_legacy = true;
>-                }
>-
>-                if (need_legacy) {
>-                    if (usblegacy) {
>-                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>-                                       _("Multiple legacy USB controllers are "
>-                                         "not supported"));
>-                        return -1;
>-                    }
>-                    usblegacy = true;
>-                    continue;
>+                !qemuDomainMachineIsQ35(def) &&
>+                !qemuDomainMachineIsVirt(def)) {
>+
>+                /* An appropriate default USB controller model should already
>+                 * have been selected in qemuDomainDeviceDefPostParse(); if
>+                 * we still have no model by now, we have to fall back to the
>+                 * legacy USB controller.
>+                 *
>+                 * Note that we *don't* want to end up with the legacy USB
>+                 * controller for q35 and virt machines, so we go ahead and
>+                 * fail in qemuBuildControllerDevStr(); on the other hand,
>+                 * for s390 machines we want to ignore any USB controller
>+                 * (see 548ba43028 for the full story), so we skip
>+                 * qemuBuildControllerDevStr() but we don't ultimately end
>+                 * up adding the legacy USB controller */

I wish this comment weren't necessary.

ACK

Jan

>+                if (usblegacy) {
>+                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>+                                   _("Multiple legacy USB controllers are "
>+                                     "not supported"));
>+                    return -1;
>                 }
>+                usblegacy = true;
>+                continue;
>             }
>
>             virCommandAddArg(cmd, "-device");
>@@ -3045,6 +3042,9 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
>         }
>     }
>
>+    /* We haven't added any USB controller yet, but we haven't been asked
>+     * not to add one either. Add a legacy USB controller, unless we're
>+     * creating a kind of guest we want to keep legacy-free */
>     if (usbcontroller == 0 &&
>         !qemuDomainMachineIsQ35(def) &&
>         !qemuDomainMachineIsVirt(def) &&
>-- 
>2.7.4
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160804/39ba6b61/attachment-0001.sig>