qemu: auto-assign addresses when <address type='pci'/> is specified

Rather than only assigning a PCI address when no address is given at
all, also do it when the config says that the address type is 'pci',
but it gives no address (virDeviceInfoPCIAddressWanted()).

There are also several places after parsing but prior to address
assignment where code previously expected that any info with address
type='pci' would have a *valid* PCI address, which isn't always the
case - now we check not only for type='pci', but also for a valid
address (virDeviceInfoPCIAddressPresent()).

The test case added in this patch was directly copied from Cole's patch titled:

    qemu: Wire up address type=pci auto_allocate

src/conf/domain_addr.c

@@ -471,7 +471,7 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs,
     if (!(addrStr = virDomainPCIAddressAsString(&dev->addr.pci)))
         goto cleanup;
 
-    if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+    if (virDeviceInfoPCIAddressPresent(dev)) {
         /* We do not support hotplug multi-function PCI device now, so we should
          * reserve the whole slot. The function of the PCI device must be 0.
          */

src/qemu/qemu_domain_address.c

@@ -431,9 +431,9 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
     virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE |
                                       VIR_PCI_CONNECT_TYPE_PCI_DEVICE);
 
-    if ((info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
-        || ((device->type == VIR_DOMAIN_DEVICE_HOSTDEV) &&
-            (device->data.hostdev->parent.type != VIR_DOMAIN_DEVICE_NONE))) {
+    if (!virDeviceInfoPCIAddressPresent(info) ||
+        ((device->type == VIR_DOMAIN_DEVICE_HOSTDEV) &&
+         (device->data.hostdev->parent.type != VIR_DOMAIN_DEVICE_NONE))) {
         /* If a hostdev has a parent, its info will be a part of the
          * parent, and will have its address collected during the scan
          * of the parent's device type.
@@ -633,7 +633,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
         /* 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 (virDeviceInfoPCIAddressPresent(&def->controllers[i]->info)) {
                 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 ||
@@ -653,7 +653,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
                    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 (virDeviceInfoPCIAddressPresent(&def->controllers[i]->info)) {
                 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 ||
@@ -690,7 +690,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
          * at slot 2.
          */
         virDomainVideoDefPtr primaryVideo = def->videos[0];
-        if (primaryVideo->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+        if (virDeviceInfoPCIAddressWanted(&primaryVideo->info)) {
             memset(&tmp_addr, 0, sizeof(tmp_addr));
             tmp_addr.slot = 2;
 
@@ -769,7 +769,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
              * address.
              */
             if (def->controllers[i]->idx == 0) {
-                if (def->controllers[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+                if (virDeviceInfoPCIAddressPresent(&def->controllers[i]->info)) {
                     if (def->controllers[i]->info.addr.pci.domain != 0 ||
                         def->controllers[i]->info.addr.pci.bus != 0 ||
                         def->controllers[i]->info.addr.pci.slot != 0x1F ||
@@ -881,7 +881,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
          * on pc machinetypes).
          */
         virDomainVideoDefPtr primaryVideo = def->videos[0];
-        if (primaryVideo->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+        if (virDeviceInfoPCIAddressWanted(&primaryVideo->info)) {
             memset(&tmp_addr, 0, sizeof(tmp_addr));
             tmp_addr.slot = 1;
 
@@ -1030,9 +1030,9 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
         if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
             virDomainControllerModelPCI model = def->controllers[i]->model;
 
-            if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
-                model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT ||
-                model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
+            if (model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT ||
+                model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT ||
+                !virDeviceInfoPCIAddressWanted(&def->controllers[i]->info))
                 continue;
 
             /* convert the type of controller into a "CONNECT_TYPE"
@@ -1055,7 +1055,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
     flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI_DEVICE;
 
     for (i = 0; i < def->nfss; i++) {
-        if (def->fss[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+        if (!virDeviceInfoPCIAddressWanted(&def->fss[i]->info))
             continue;
 
         /* Only support VirtIO-9p-pci so far. If that changes,
@@ -1072,7 +1072,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
          * instead of here.
          */
         if ((def->nets[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) ||
-            (def->nets[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)) {
+            !virDeviceInfoPCIAddressWanted(&def->nets[i]->info)) {
             continue;
         }
         if (virDomainPCIAddressReserveNextSlot(addrs, &def->nets[i]->info,
@@ -1082,7 +1082,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
 
     /* Sound cards */
     for (i = 0; i < def->nsounds; i++) {
-        if (def->sounds[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+        if (!virDeviceInfoPCIAddressWanted(&def->sounds[i]->info))
             continue;
         /* Skip ISA sound card, PCSPK and usb-audio */
         if (def->sounds[i]->model == VIR_DOMAIN_SOUND_MODEL_SB16 ||
@@ -1117,7 +1117,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
             def->controllers[i]->idx == 0)
             continue;
 
-        if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+        if (!virDeviceInfoPCIAddressWanted(&def->controllers[i]->info))
             continue;
 
         /* USB2 needs special handling to put all companions in the same slot */
@@ -1129,8 +1129,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
             for (j = 0; j < def->ncontrollers; j++) {
                 if (IS_USB2_CONTROLLER(def->controllers[j]) &&
                     def->controllers[j]->idx == def->controllers[i]->idx &&
-                    def->controllers[j]->info.type
-                    == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+                    virDeviceInfoPCIAddressPresent(&def->controllers[j]->info)) {
                     addr = def->controllers[j]->info.addr.pci;
                     foundAddr = true;
                     break;
@@ -1191,7 +1190,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
             continue;
 
         /* don't touch s390 devices */
-        if (def->disks[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI ||
+        if (virDeviceInfoPCIAddressPresent(&def->disks[i]->info) ||
             def->disks[i]->info.type ==
             VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 ||
             def->disks[i]->info.type ==
@@ -1204,7 +1203,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
             virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MMIO))
             continue;
 
-        if (def->disks[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+        if (!virDeviceInfoPCIAddressWanted(&def->disks[i]->info)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("virtio disk cannot have an address of type '%s'"),
                            virDomainDeviceAddressTypeToString(def->disks[i]->info.type));
@@ -1218,7 +1217,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
 
     /* Host PCI devices */
     for (i = 0; i < def->nhostdevs; i++) {
-        if (def->hostdevs[i]->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+        if (!virDeviceInfoPCIAddressWanted(def->hostdevs[i]->info))
             continue;
         if (def->hostdevs[i]->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
             def->hostdevs[i]->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
@@ -1233,7 +1232,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
     /* VirtIO balloon */
     if (def->memballoon &&
         def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO &&
-        def->memballoon->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+        virDeviceInfoPCIAddressWanted(&def->memballoon->info)) {
         if (virDomainPCIAddressReserveNextSlot(addrs,
                                                &def->memballoon->info,
                                                flags) < 0)
@@ -1243,7 +1242,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
     /* VirtIO RNG */
     for (i = 0; i < def->nrngs; i++) {
         if (def->rngs[i]->model != VIR_DOMAIN_RNG_MODEL_VIRTIO ||
-            def->rngs[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+            !virDeviceInfoPCIAddressWanted(&def->rngs[i]->info))
             continue;
 
         if (virDomainPCIAddressReserveNextSlot(addrs,
@@ -1254,7 +1253,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
     /* A watchdog - check if it is a PCI device */
     if (def->watchdog &&
         def->watchdog->model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB &&
-        def->watchdog->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+        virDeviceInfoPCIAddressWanted(&def->watchdog->info)) {
         if (virDomainPCIAddressReserveNextSlot(addrs, &def->watchdog->info,
                                                flags) < 0)
             goto error;
@@ -1263,7 +1262,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
     /* Assign a PCI slot to the primary video card if there is not an
      * assigned address. */
     if (def->nvideos > 0 &&
-        def->videos[0]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+        virDeviceInfoPCIAddressWanted(&def->videos[0]->info)) {
         if (virDomainPCIAddressReserveNextSlot(addrs, &def->videos[0]->info,
                                                flags) < 0)
             goto error;
@@ -1276,7 +1275,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
                            _("non-primary video device must be type of 'qxl'"));
             goto error;
         }
-        if (def->videos[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+        if (!virDeviceInfoPCIAddressWanted(&def->videos[i]->info))
             continue;
         if (virDomainPCIAddressReserveNextSlot(addrs, &def->videos[i]->info,
                                                flags) < 0)
@@ -1285,7 +1284,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
 
     /* Shared Memory */
     for (i = 0; i < def->nshmems; i++) {
-        if (def->shmems[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+        if (!virDeviceInfoPCIAddressWanted(&def->shmems[i]->info))
             continue;
 
         if (virDomainPCIAddressReserveNextSlot(addrs,
@@ -1293,9 +1292,8 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
             goto error;
     }
     for (i = 0; i < def->ninputs; i++) {
-        if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_VIRTIO)
-            continue;
-        if (def->inputs[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+        if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_VIRTIO ||
+            !virDeviceInfoPCIAddressWanted(&def->inputs[i]->info))
             continue;
 
         if (virDomainPCIAddressReserveNextSlot(addrs,
@@ -1308,10 +1306,8 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
     for (i = 0; i < def->nserials; i++) {
         virDomainChrDefPtr chr = def->serials[i];
 
-        if (chr->targetType != VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI)
-            continue;
-
-        if (chr->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+        if (chr->targetType != VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI ||
+            !virDeviceInfoPCIAddressWanted(&chr->info))
             continue;
 
         if (virDomainPCIAddressReserveNextSlot(addrs, &chr->info, flags) < 0)
@@ -1677,7 +1673,7 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
         virDomainCCWAddressReleaseAddr(priv->ccwaddrs, info) < 0)
         VIR_WARN("Unable to release CCW address on %s",
                  NULLSTR(devstr));
-    else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
+    else if (virDeviceInfoPCIAddressPresent(info) &&
              virDomainPCIAddressReleaseSlot(priv->pciaddrs,
                                             &info->addr.pci) < 0)
         VIR_WARN("Unable to release PCI address on %s",

tests/qemuxml2argvdata/qemuxml2argv-pci-autofill-addr.args

+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/libexec/qemu-kvm \
+-name fdr-br \
+-S \
+-M pc-1.2 \
+-m 2048 \
+-smp 2 \
+-uuid 3ec6cbe1-b5a2-4515-b800-31a61855df41 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-fdr-br/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=/var/iso/f18kde.iso,format=raw,if=none,media=cdrom,\
+id=drive-virtio-disk0 \
+-device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\
+id=virtio-disk0 \
+-vga cirrus \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4

tests/qemuxml2argvdata/qemuxml2argv-pci-autofill-addr.xml

+<domain type='qemu'>
+  <name>fdr-br</name>
+  <uuid>3ec6cbe1-b5a2-4515-b800-31a61855df41</uuid>
+  <memory unit='KiB'>2097152</memory>
+  <currentMemory unit='KiB'>2097152</currentMemory>
+  <vcpu placement='static' cpuset='0-1'>2</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc-1.2'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <devices>
+    <emulator>/usr/libexec/qemu-kvm</emulator>
+    <disk type='file' device='cdrom'>
+      <driver name='qemu' type='raw'/>
+      <source file='/var/iso/f18kde.iso'/>
+      <target dev='vda' bus='virtio'/>
+      <readonly/>
+      <address type='pci'/>
+    </disk>
+    <controller type='usb' index='0'>
+      <address type='pci'/>
+    </controller>
+    <controller type='ide' index='0'>
+      <address type='pci'/>
+    </controller>
+    <input type='mouse' bus='ps2'/>
+    <video>
+      <model type='cirrus' vram='16384' heads='1'/>
+      <address type='pci'/>
+    </video>
+    <memballoon model='virtio'>
+      <address type='pci'/>
+    </memballoon>
+  </devices>
+</domain>

tests/qemuxml2argvtest.c

@@ -1527,6 +1527,7 @@ mymain(void)
 
     DO_TEST("pci-autoadd-addr", QEMU_CAPS_DEVICE_PCI_BRIDGE);
     DO_TEST("pci-autoadd-idx", QEMU_CAPS_DEVICE_PCI_BRIDGE);
+    DO_TEST("pci-autofill-addr", NONE);
     DO_TEST("pci-many",
             QEMU_CAPS_DEVICE_PCI_BRIDGE);
     DO_TEST("pci-bridge-many-disks",

tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-autofill-addr.xml

+<domain type='qemu'>
+  <name>fdr-br</name>
+  <uuid>3ec6cbe1-b5a2-4515-b800-31a61855df41</uuid>
+  <memory unit='KiB'>2097152</memory>
+  <currentMemory unit='KiB'>2097152</currentMemory>
+  <vcpu placement='static' cpuset='0-1'>2</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc-1.2'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/libexec/qemu-kvm</emulator>
+    <disk type='file' device='cdrom'>
+      <driver name='qemu' type='raw'/>
+      <source file='/var/iso/f18kde.iso'/>
+      <target dev='vda' bus='virtio'/>
+      <readonly/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </disk>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='ide' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <video>
+      <model type='cirrus' vram='16384' heads='1' primary='yes'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+    </video>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>

tests/qemuxml2xmltest.c

@@ -597,6 +597,7 @@ mymain(void)
                  QEMU_CAPS_DEVICE_PCI_BRIDGE);
     DO_TEST_FULL("pci-autoadd-idx", WHEN_ACTIVE, GIC_NONE,
                  QEMU_CAPS_DEVICE_PCI_BRIDGE);
+    DO_TEST("pci-autofill-addr");
 
     DO_TEST_FULL("q35", WHEN_ACTIVE, GIC_NONE,
                  QEMU_CAPS_DEVICE_PCI_BRIDGE,