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