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

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

 



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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature

--
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]