[libvirt] [PATCH 2/5] qemu: only check for PIIX3-specific device addrs on pc-* machinetypes

Laine Stump laine at laine.org
Tue Jul 23 14:44:52 UTC 2013


The implicit IDE, USB, and video controllers provided by the PIIX3
chipset in the pc-* machinetypes are not present on other
machinetypes, so we shouldn't be doing the special checking for them.

The diffs for this patch look hairy, but that's just because a large
section was reindented (to be placed inside a conditional) and git
couldn't figure out a sane diff. It really is just 1) determining if
this system uses PIIX3, 2) put the stuff that's PIIX3-specific inside
an if.

(Note that, according to the qemuxml2argv-pseries-usb-multi test, ppc
"pseries" machines also have a PIIX3 chip (since that test file adds a
"piix3-uhci" usb controller). I don't know if this is really the case
or not, but had to include that machine type in the checks in order
for make check to succeed with no changes to the test data.)
---
 src/qemu/qemu_command.c | 190 +++++++++++++++++++++++++-----------------------
 1 file changed, 99 insertions(+), 91 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7f5dab9..f85e896 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1993,111 +1993,119 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
     bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
     virDevicePCIAddress tmp_addr;
     virDevicePCIAddressPtr addrptr;
-
-    /* Verify that first IDE and USB controllers (if any) is on the PIIX3, fn 1 */
-    for (i = 0; i < def->ncontrollers; i++) {
-        /* First IDE controller lives on the PIIX3 at slot=1, function=1 */
-        if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE &&
-            def->controllers[i]->idx == 0) {
-            if (def->controllers[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
-                if (def->controllers[i]->info.addr.pci.domain != 0 ||
-                    def->controllers[i]->info.addr.pci.bus != 0 ||
-                    def->controllers[i]->info.addr.pci.slot != 1 ||
-                    def->controllers[i]->info.addr.pci.function != 1) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                   _("Primary IDE controller must have PCI address 0:0:1.1"));
-                    goto error;
+    bool isPIIX3 = STRPREFIX(def->os.machine, "pc-0.") ||
+        STRPREFIX(def->os.machine, "pc-1.") ||
+        STRPREFIX(def->os.machine, "pc-i440") ||
+        STREQ(def->os.machine, "pc") ||
+        STRPREFIX(def->os.machine, "rhel") ||
+        STREQ(def->os.machine, "pseries");
+
+    if (isPIIX3) {
+        /* Verify that first IDE and USB controllers (if any) is on the PIIX3, fn 1 */
+        for (i = 0; i < def->ncontrollers; i++) {
+            /* First IDE controller lives on the PIIX3 at slot=1, function=1 */
+            if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE &&
+                def->controllers[i]->idx == 0) {
+                if (def->controllers[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+                    if (def->controllers[i]->info.addr.pci.domain != 0 ||
+                        def->controllers[i]->info.addr.pci.bus != 0 ||
+                        def->controllers[i]->info.addr.pci.slot != 1 ||
+                        def->controllers[i]->info.addr.pci.function != 1) {
+                        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                       _("Primary IDE controller must have PCI address 0:0:1.1"));
+                        goto error;
+                    }
+                } else {
+                    def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+                    def->controllers[i]->info.addr.pci.domain = 0;
+                    def->controllers[i]->info.addr.pci.bus = 0;
+                    def->controllers[i]->info.addr.pci.slot = 1;
+                    def->controllers[i]->info.addr.pci.function = 1;
                 }
-            } else {
-                def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-                def->controllers[i]->info.addr.pci.domain = 0;
-                def->controllers[i]->info.addr.pci.bus = 0;
-                def->controllers[i]->info.addr.pci.slot = 1;
-                def->controllers[i]->info.addr.pci.function = 1;
-            }
-        } else if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
-                   def->controllers[i]->idx == 0 &&
-                   (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI ||
-                    def->controllers[i]->model == -1)) {
-            if (def->controllers[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
-                if (def->controllers[i]->info.addr.pci.domain != 0 ||
-                    def->controllers[i]->info.addr.pci.bus != 0 ||
-                    def->controllers[i]->info.addr.pci.slot != 1 ||
-                    def->controllers[i]->info.addr.pci.function != 2) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                   _("PIIX3 USB controller must have PCI address 0:0:1.2"));
-                    goto error;
+            } else if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
+                       def->controllers[i]->idx == 0 &&
+                       (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI ||
+                        def->controllers[i]->model == -1)) {
+                if (def->controllers[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+                    if (def->controllers[i]->info.addr.pci.domain != 0 ||
+                        def->controllers[i]->info.addr.pci.bus != 0 ||
+                        def->controllers[i]->info.addr.pci.slot != 1 ||
+                        def->controllers[i]->info.addr.pci.function != 2) {
+                        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                       _("PIIX3 USB controller must have PCI address 0:0:1.2"));
+                        goto error;
+                    }
+                } else {
+                    def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+                    def->controllers[i]->info.addr.pci.domain = 0;
+                    def->controllers[i]->info.addr.pci.bus = 0;
+                    def->controllers[i]->info.addr.pci.slot = 1;
+                    def->controllers[i]->info.addr.pci.function = 2;
                 }
-            } else {
-                def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-                def->controllers[i]->info.addr.pci.domain = 0;
-                def->controllers[i]->info.addr.pci.bus = 0;
-                def->controllers[i]->info.addr.pci.slot = 1;
-                def->controllers[i]->info.addr.pci.function = 2;
             }
         }
-    }
-
-    /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB controller)
-     * hardcoded slot=1, multifunction device
-     */
-    if (addrs->nbuses) {
-        memset(&tmp_addr, 0, sizeof(tmp_addr));
-        tmp_addr.slot = 1;
-        if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr) < 0)
-            goto error;
-    }
 
-    if (def->nvideos > 0) {
-        virDomainVideoDefPtr primaryVideo = def->videos[0];
-        if (primaryVideo->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
-            primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-            primaryVideo->info.addr.pci.domain = 0;
-            primaryVideo->info.addr.pci.bus = 0;
-            primaryVideo->info.addr.pci.slot = 2;
-            primaryVideo->info.addr.pci.function = 0;
-            addrptr = &primaryVideo->info.addr.pci;
-
-            if (!qemuPCIAddressValidate(addrs, addrptr))
+        /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB controller)
+         * hardcoded slot=1, multifunction device
+         */
+        if (addrs->nbuses) {
+            memset(&tmp_addr, 0, sizeof(tmp_addr));
+            tmp_addr.slot = 1;
+            if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr) < 0)
                 goto error;
+        }
 
-            if (qemuDomainPCIAddressSlotInUse(addrs, addrptr)) {
-                if (qemuDeviceVideoUsable) {
-                    virResetLastError();
-                    if (qemuDomainPCIAddressSetNextAddr(addrs, &primaryVideo->info) < 0)
+        if (def->nvideos > 0) {
+            virDomainVideoDefPtr primaryVideo = def->videos[0];
+            if (primaryVideo->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+                primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+                primaryVideo->info.addr.pci.domain = 0;
+                primaryVideo->info.addr.pci.bus = 0;
+                primaryVideo->info.addr.pci.slot = 2;
+                primaryVideo->info.addr.pci.function = 0;
+                addrptr = &primaryVideo->info.addr.pci;
+
+                if (!qemuPCIAddressValidate(addrs, addrptr))
+                    goto error;
+
+                if (qemuDomainPCIAddressSlotInUse(addrs, addrptr)) {
+                    if (qemuDeviceVideoUsable) {
+                        virResetLastError();
+                        if (qemuDomainPCIAddressSetNextAddr(addrs, &primaryVideo->info) < 0)
+                            goto error;
+                    } else {
+                        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                       _("PCI address 0:0:2.0 is in use, "
+                                         "QEMU needs it for primary video"));
                         goto error;
-                } else {
+                    }
+                } else if (qemuDomainPCIAddressReserveSlot(addrs, addrptr) < 0) {
+                    goto error;
+                }
+            } else if (!qemuDeviceVideoUsable) {
+                if (primaryVideo->info.addr.pci.domain != 0 ||
+                    primaryVideo->info.addr.pci.bus != 0 ||
+                    primaryVideo->info.addr.pci.slot != 2 ||
+                    primaryVideo->info.addr.pci.function != 0) {
                     virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                   _("PCI address 0:0:2.0 is in use, "
-                                     "QEMU needs it for primary video"));
+                                   _("Primary video card must have PCI address 0:0:2.0"));
                     goto error;
                 }
-            } else if (qemuDomainPCIAddressReserveSlot(addrs, addrptr) < 0) {
-                goto error;
+                /* If TYPE==PCI, then qemuCollectPCIAddress() function
+                 * has already reserved the address, so we must skip */
             }
-        } else if (!qemuDeviceVideoUsable) {
-            if (primaryVideo->info.addr.pci.domain != 0 ||
-                primaryVideo->info.addr.pci.bus != 0 ||
-                primaryVideo->info.addr.pci.slot != 2 ||
-                primaryVideo->info.addr.pci.function != 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                               _("Primary video card must have PCI address 0:0:2.0"));
+        } else if (addrs->nbuses && !qemuDeviceVideoUsable) {
+            memset(&tmp_addr, 0, sizeof(tmp_addr));
+            tmp_addr.slot = 2;
+
+            if (qemuDomainPCIAddressSlotInUse(addrs, &tmp_addr)) {
+                VIR_DEBUG("PCI address 0:0:2.0 in use, future addition of a video"
+                          " device will not be possible without manual"
+                          " intervention");
+                virResetLastError();
+            } else if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr) < 0) {
                 goto error;
             }
-            /* If TYPE==PCI, then qemuCollectPCIAddress() function
-             * has already reserved the address, so we must skip */
-        }
-    } else if (addrs->nbuses && !qemuDeviceVideoUsable) {
-        memset(&tmp_addr, 0, sizeof(tmp_addr));
-        tmp_addr.slot = 2;
-
-        if (qemuDomainPCIAddressSlotInUse(addrs, &tmp_addr)) {
-            VIR_DEBUG("PCI address 0:0:2.0 in use, future addition of a video"
-                      " device will not be possible without manual"
-                      " intervention");
-            virResetLastError();
-        } else if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr) < 0) {
-            goto error;
         }
     }
 
-- 
1.7.11.7




More information about the libvir-list mailing list