Fix explicit usage of default video PCI slots

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.

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d723dc8d16f7199fd57d1dc12a249b2091ac72fc..2a64cd1f2e4aebb39d96f0ddca6ed5c5949aaf26 100644 (file)
--- 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 847e96f8560d9b64591b5514369fafefba6e9540..5a2bfccf19f6c51a45579b54336d0eeb70eeb624 100644 (file)
--- 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);