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

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index acd8ce6ed33b391abc3a226366b6e601a6549258..794270dadb51fc1d7c4a83f8aba5f1bc511f4978 100644 (file)
--- a/src/conf/domain_addr.c
+++ b/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.
          */

diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 9d09b3a606471dfafa5b860b7ee2b2c11e2ae75e..7bd8ee58c755e7ddd6b4cef29b1a505d05c5a74f 100644 (file)
--- a/src/qemu/qemu_domain_address.c
+++ b/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",

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pci-autofill-addr.args b/tests/qemuxml2argvdata/qemuxml2argv-pci-autofill-addr.args
new file mode 100644 (file)
index 0000000..ddb8c8d
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pci-autofill-addr.args
@@ -0,0 +1,25 @@
+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

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pci-autofill-addr.xml b/tests/qemuxml2argvdata/qemuxml2argv-pci-autofill-addr.xml
new file mode 100644 (file)
index 0000000..e5256fe
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pci-autofill-addr.xml
@@ -0,0 +1,35 @@
+<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>

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 58b58e70832a8d070a213f85b7fc50d6a8fa401b..f28bd084cf0f55497437eb79111348d9ebf31eb4 100644 (file)
--- a/tests/qemuxml2argvtest.c
+++ b/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",

diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-autofill-addr.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-autofill-addr.xml
new file mode 100644 (file)
index 0000000..6259044
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-autofill-addr.xml
@@ -0,0 +1,41 @@
+<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>

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index b8fac591474a0eebd82972e6d835efacbbb99644..cbb1118f6ae8306c773f5d5bb40d8791401d0bdc 100644 (file)
--- a/tests/qemuxml2xmltest.c
+++ b/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,