2:23 p.m.
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 | 2 +-
src/qemu/qemu_domain_address.c | 64 ++++++++++------------
.../qemuxml2argv-pci-autofill-addr.args | 25 +++++++++
.../qemuxml2argv-pci-autofill-addr.xml | 35 ++++++++++++
tests/qemuxml2argvtest.c | 1 +
.../qemuxml2xmlout-pci-autofill-addr.xml | 41 ++++++++++++++
tests/qemuxml2xmltest.c | 1 +
7 files changed, 134 insertions(+), 35 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pci-autofill-addr.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pci-autofill-addr.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-autofill-addr.xml
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index acd8ce6..794270d 100644
--- 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 9d09b3a..7bd8ee5 100644
--- 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
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
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 3bfb5c4..cd4fe22 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1522,6 +1522,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
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 30cf3e8..24852d2 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -591,6 +591,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,
--
2.5.5