[libvirt] [PATCHv2] Fix explicit usage of default video PCI slots

Ján Tomko jtomko at redhat.com
Fri Jan 3 11:06:20 UTC 2014


Do not leave the PCI address of the primary video card set
to the legacy default (0000:00:02.0) if we're doing two-pass
allocation.

Since QEMU 1.6 (QEMU_CAPS_VIDEO_PRIMARY) we allow the primary
video card to be on other slots than 0000:00:02.0 (as we use
-device instead of -vga).

However we fail to assign it an address if:
* another device explicitly uses 0000:00:02.0 and
* the primary video device has no address specified

On the first pass, we have set the address to default, then checked
if it's available, leaving it set even if it wasn't. This address
got picked up by the second pass, resulting in a conflict:

XML error: Attempted double use of PCI slot 0000:00:02.0
(may need "multifunction='on'" for device on function 0)

Also fix the test that was supposed to catch this.
---
v1: https://www.redhat.com/archives/libvir-list/2013-December/msg01125.html
v2: no functional change
    rewrote the commit message to mention that this only happens
        with QEMU 1.6 and (hopefully) explain what 'this' is more clearly

 src/qemu/qemu_command.c  | 50 +++++++++++++++++++++---------------------------
 tests/qemuxml2argvtest.c |  3 ++-
 2 files changed, 24 insertions(+), 29 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d723dc8..2a64cd1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2379,7 +2379,6 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
     size_t i;
     virDevicePCIAddress tmp_addr;
     bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
-    virDevicePCIAddressPtr addrptr;
     char *addrStr = NULL;
     qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_HOTPLUGGABLE | QEMU_PCI_CONNECT_TYPE_PCI;
 
@@ -2446,22 +2445,17 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
          */
         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 (!(addrStr = qemuDomainPCIAddressAsString(addrptr)))
+            memset(&tmp_addr, 0, sizeof(tmp_addr));
+            tmp_addr.slot = 2;
+
+            if (!(addrStr = qemuDomainPCIAddressAsString(&tmp_addr)))
                 goto cleanup;
-            if (!qemuDomainPCIAddressValidate(addrs, addrptr,
+            if (!qemuDomainPCIAddressValidate(addrs, &tmp_addr,
                                               addrStr, flags, false))
                 goto cleanup;
 
-            if (qemuDomainPCIAddressSlotInUse(addrs, addrptr)) {
+            if (qemuDomainPCIAddressSlotInUse(addrs, &tmp_addr)) {
                 if (qemuDeviceVideoUsable) {
-                    virResetLastError();
                     if (qemuDomainPCIAddressReserveNextSlot(addrs,
                                                             &primaryVideo->info,
                                                             flags) < 0)
@@ -2472,8 +2466,11 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
                                      "QEMU needs it for primary video"));
                     goto cleanup;
                 }
-            } else if (qemuDomainPCIAddressReserveSlot(addrs, addrptr, flags) < 0) {
-                goto cleanup;
+            } else {
+                if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0)
+                    goto cleanup;
+                primaryVideo->info.addr.pci = tmp_addr;
+                primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
             }
         } else if (!qemuDeviceVideoUsable) {
             if (primaryVideo->info.addr.pci.domain != 0 ||
@@ -2535,7 +2532,6 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
     size_t i;
     virDevicePCIAddress tmp_addr;
     bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
-    virDevicePCIAddressPtr addrptr;
     char *addrStr = NULL;
     qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_TYPE_PCIE;
 
@@ -2619,22 +2615,17 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
          */
         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 = 1;
-            primaryVideo->info.addr.pci.function = 0;
-            addrptr = &primaryVideo->info.addr.pci;
-
-            if (!(addrStr = qemuDomainPCIAddressAsString(addrptr)))
+            memset(&tmp_addr, 0, sizeof(tmp_addr));
+            tmp_addr.slot = 1;
+
+            if (!(addrStr = qemuDomainPCIAddressAsString(&tmp_addr)))
                 goto cleanup;
-            if (!qemuDomainPCIAddressValidate(addrs, addrptr,
+            if (!qemuDomainPCIAddressValidate(addrs, &tmp_addr,
                                               addrStr, flags, false))
                 goto cleanup;
 
-            if (qemuDomainPCIAddressSlotInUse(addrs, addrptr)) {
+            if (qemuDomainPCIAddressSlotInUse(addrs, &tmp_addr)) {
                 if (qemuDeviceVideoUsable) {
-                    virResetLastError();
                     if (qemuDomainPCIAddressReserveNextSlot(addrs,
                                                             &primaryVideo->info,
                                                             flags) < 0)
@@ -2645,8 +2636,11 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
                                      "QEMU needs it for primary video"));
                     goto cleanup;
                 }
-            } else if (qemuDomainPCIAddressReserveSlot(addrs, addrptr, flags) < 0) {
-                goto cleanup;
+            } else {
+                if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0)
+                    goto cleanup;
+                primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+                primaryVideo->info.addr.pci = tmp_addr;
             }
         } else if (!qemuDeviceVideoUsable) {
             if (primaryVideo->info.addr.pci.domain != 0 ||
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 847e96f..5a2bfcc 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1193,7 +1193,8 @@ mymain(void)
     DO_TEST("video-device-pciaddr-default",
             QEMU_CAPS_KVM, QEMU_CAPS_VNC,
             QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
-            QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_DEVICE_QXL_VGA);
+            QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_DEVICE_QXL_VGA,
+            QEMU_CAPS_DEVICE_PCI_BRIDGE);
 
     DO_TEST("virtio-rng-default", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_VIRTIO_RNG,
             QEMU_CAPS_OBJECT_RNG_RANDOM);
-- 
1.8.3.2




More information about the libvir-list mailing list