10:08 a.m.
Rebase of
https://www.redhat.com/archives/libvir-list/2017-December/msg00154.html
on top of master, now that patch 1/3 has been merged as part of
John's work to move controller checks from command line generation
to guest validation.
Andrea Bolognani (2):
tests: Add some tests for PCI controller options
qemu: Clean up PCI controller options
src/qemu/qemu_domain.c | 117 +++++++++++++++++++++
.../i440fx-controllers-pciopts.args | 24 +++++
.../i440fx-controllers-pciopts.xml | 36 +++++++
.../pseries-controllers-pciopts.args | 22 ++++
.../pseries-controllers-pciopts.xml | 36 +++++++
.../qemuxml2argvdata/q35-controllers-pciopts.args | 28 +++++
tests/qemuxml2argvdata/q35-controllers-pciopts.xml | 60 +++++++++++
tests/qemuxml2argvtest.c | 17 +++
.../i440fx-controllers-pciopts.xml | 42 ++++++++
tests/qemuxml2xmloutdata/pcie-expander-bus.xml | 4 +-
.../pseries-controllers-pciopts.xml | 41 ++++++++
.../qemuxml2xmloutdata/q35-controllers-pciopts.xml | 63 +++++++++++
tests/qemuxml2xmltest.c | 17 +++
13 files changed, 504 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
create mode 100644 tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml
create mode 100644 tests/qemuxml2argvdata/pseries-controllers-pciopts.args
create mode 100644 tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
create mode 100644 tests/qemuxml2argvdata/q35-controllers-pciopts.args
create mode 100644 tests/qemuxml2argvdata/q35-controllers-pciopts.xml
create mode 100644 tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
create mode 100644 tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
create mode 100644 tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
--
2.14.3
10:08 a.m.
New subject: [libvirt] [PATCH REBASE 1/2] tests: Add some tests for PCI controller options
The input configurations set all existing options for all PCI
controllers, to see what ends up showing up in the output.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
.../i440fx-controllers-pciopts.args | 24 +++++++
.../i440fx-controllers-pciopts.xml | 36 ++++++++++
.../pseries-controllers-pciopts.args | 22 +++++++
.../pseries-controllers-pciopts.xml | 35 ++++++++++
.../qemuxml2argvdata/q35-controllers-pciopts.args | 28 ++++++++
tests/qemuxml2argvdata/q35-controllers-pciopts.xml | 60 +++++++++++++++++
tests/qemuxml2argvtest.c | 17 +++++
.../i440fx-controllers-pciopts.xml | 45 +++++++++++++
.../pseries-controllers-pciopts.xml | 43 ++++++++++++
.../qemuxml2xmloutdata/q35-controllers-pciopts.xml | 76 ++++++++++++++++++++++
tests/qemuxml2xmltest.c | 17 +++++
11 files changed, 403 insertions(+)
create mode 100644 tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
create mode 100644 tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml
create mode 100644 tests/qemuxml2argvdata/pseries-controllers-pciopts.args
create mode 100644 tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
create mode 100644 tests/qemuxml2argvdata/q35-controllers-pciopts.args
create mode 100644 tests/qemuxml2argvdata/q35-controllers-pciopts.xml
create mode 100644 tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
create mode 100644 tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
create mode 100644 tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
diff --git a/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
new file mode 100644
index 000000000..d85fae5c9
--- /dev/null
+++ b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
@@ -0,0 +1,24 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest \
+-S \
+-M pc \
+-m 1024 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-numa node,nodeid=0,cpus=0,mem=1024 \
+-uuid 496d7ea8-9739-544b-4ebd-ef08be936e8b \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-global i440FX-pcihost.pci-hole64-size=1024K \
+-device pci-bridge,chassis_nr=2,id=pci.1,bus=pci.0,addr=0x3 \
+-device pxb,bus_nr=3,id=pci.2,numa_node=0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml
b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml
new file mode 100644
index 000000000..861a66589
--- /dev/null
+++ b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml
@@ -0,0 +1,36 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ </os>
+ <cpu>
+ <numa>
+ <cell id='0' cpus='0' memory='1048576'
unit='KiB'/>
+ </numa>
+ </cpu>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='pci' index='0' model='pci-root'>
+ <!-- Only pcihole64 should be preserved -->
+ <target busNr='1' chassis='1' chassisNr='1'
index='0' port='1'/>
+ <pcihole64 unit='KiB'>1024</pcihole64>
+ </controller>
+ <controller type='pci' index='1' model='pci-bridge'>
+ <!-- Only chassisNr should be preserved -->
+ <target busNr='2' chassis='2' chassisNr='2'
index='2' port='2'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='2'
model='pci-expander-bus'>
+ <!-- Only busNr and numaNode should be preserved -->
+ <target busNr='3' chassis='3' chassisNr='3'
index='3' port='3'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='usb' index='0' model='none'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/pseries-controllers-pciopts.args
b/tests/qemuxml2argvdata/pseries-controllers-pciopts.args
new file mode 100644
index 000000000..5f1edfc83
--- /dev/null
+++ b/tests/qemuxml2argvdata/pseries-controllers-pciopts.args
@@ -0,0 +1,22 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-ppc64 \
+-name guest \
+-S \
+-M pseries \
+-m 1024 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-numa node,nodeid=0,cpus=0,mem=1024 \
+-uuid 496d7ea8-9739-544b-4ebd-ef08be936e8b \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-boot c \
+-device spapr-pci-host-bridge,index=1,id=pci.1,numa_node=0 \
+-device pci-bridge,chassis_nr=3,id=pci.2,bus=pci.0,addr=0x1
diff --git a/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
b/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
new file mode 100644
index 000000000..43353cba6
--- /dev/null
+++ b/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
@@ -0,0 +1,35 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='ppc64' machine='pseries'>hvm</type>
+ </os>
+ <cpu>
+ <numa>
+ <cell id='0' cpus='0' memory='1048576'
unit='KiB'/>
+ </numa>
+ </cpu>
+ <devices>
+ <emulator>/usr/bin/qemu-system-ppc64</emulator>
+ <controller type='pci' index='0' model='pci-root'>
+ <!-- Only targetIndex should be preserved -->
+ <target busNr='1' chassis='1' chassisNr='1'
index='0' port='1'/>
+ </controller>
+ <controller type='pci' index='1' model='pci-root'>
+ <!-- Only numaNode and targetIndex should be preserved -->
+ <target busNr='2' chassis='2' chassisNr='2'
index='1' port='2'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='2' model='pci-bridge'>
+ <!-- Only chassisNr should be preserved -->
+ <target busNr='3' chassis='3' chassisNr='3'
index='3' port='3'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='usb' index='0' model='none'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/q35-controllers-pciopts.args
b/tests/qemuxml2argvdata/q35-controllers-pciopts.args
new file mode 100644
index 000000000..44259f502
--- /dev/null
+++ b/tests/qemuxml2argvdata/q35-controllers-pciopts.args
@@ -0,0 +1,28 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest \
+-S \
+-M q35 \
+-m 1024 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-numa node,nodeid=0,cpus=0,mem=1024 \
+-uuid 496d7ea8-9739-544b-4ebd-ef08be936e8b \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-global q35-pcihost.pci-hole64-size=1024K \
+-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
+-device pci-bridge,chassis_nr=3,id=pci.2,bus=pci.1,addr=0x0 \
+-device pxb-pcie,bus_nr=4,id=pci.3,numa_node=0,bus=pcie.0,addr=0x2 \
+-device pcie-root-port,port=0x5,chassis=5,id=pci.4,bus=pcie.0,addr=0x3 \
+-device x3130-upstream,id=pci.5,bus=pci.4,addr=0x0 \
+-device xio3130-downstream,port=0x7,chassis=7,id=pci.6,bus=pci.5,addr=0x0
diff --git a/tests/qemuxml2argvdata/q35-controllers-pciopts.xml
b/tests/qemuxml2argvdata/q35-controllers-pciopts.xml
new file mode 100644
index 000000000..40db83615
--- /dev/null
+++ b/tests/qemuxml2argvdata/q35-controllers-pciopts.xml
@@ -0,0 +1,60 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='q35'>hvm</type>
+ </os>
+ <cpu>
+ <numa>
+ <cell id='0' cpus='0' memory='1048576'
unit='KiB'/>
+ </numa>
+ </cpu>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='pci' index='0' model='pcie-root'>
+ <!-- Only pcihole64 should be preserved -->
+ <target busNr='1' chassis='1' chassisNr='1'
index='0' port='1'/>
+ <pcihole64 unit='KiB'>1024</pcihole64>
+ </controller>
+ <controller type='pci' index='1'
model='dmi-to-pci-bridge'>
+ <!-- No option should be preserved -->
+ <target busNr='2' chassis='2' chassisNr='2'
index='2' port='2'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='2' model='pci-bridge'>
+ <!-- Only chassisNr should be preserved -->
+ <target busNr='3' chassis='3' chassisNr='3'
index='3' port='3'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='3'
model='pcie-expander-bus'>
+ <!-- Only busNr and numaNode should be preserved -->
+ <target busNr='4' chassis='4' chassisNr='4'
index='4' port='4'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='4'
model='pcie-root-port'>
+ <!-- Only chassis and port should be preserved -->
+ <target busNr='5' chassis='5' chassisNr='5'
index='5' port='5'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='5'
model='pcie-switch-upstream-port'>
+ <!-- No option should be preserved -->
+ <target busNr='6' chassis='6' chassisNr='6'
index='6' port='6'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='6'
model='pcie-switch-downstream-port'>
+ <!-- Only chassis and port should be preserved -->
+ <target busNr='7' chassis='7' chassisNr='7'
index='7' port='7'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='usb' index='0' model='none'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c8739909d..595b541d5 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2436,6 +2436,23 @@ mymain(void)
QEMU_CAPS_DEVICE_IOH3420,
QEMU_CAPS_DEVICE_PXB_PCIE);
+ DO_TEST("i440fx-controllers-pciopts",
+ QEMU_CAPS_I440FX_PCI_HOLE64_SIZE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_PXB);
+ DO_TEST("q35-controllers-pciopts",
+ QEMU_CAPS_Q35_PCI_HOLE64_SIZE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_PXB_PCIE,
+ QEMU_CAPS_DEVICE_PCIE_ROOT_PORT,
+ QEMU_CAPS_DEVICE_X3130_UPSTREAM,
+ QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM);
+ DO_TEST("pseries-controllers-pciopts",
+ QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
+ QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE);
+
DO_TEST("hostdev-scsi-lsi",
QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI,
QEMU_CAPS_DEVICE_SCSI_GENERIC);
diff --git a/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
b/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
new file mode 100644
index 000000000..d171d1370
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
@@ -0,0 +1,45 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <currentMemory unit='KiB'>1048576</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <cpu>
+ <numa>
+ <cell id='0' cpus='0' memory='1048576'
unit='KiB'/>
+ </numa>
+ </cpu>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='pci' index='0' model='pci-root'>
+ <target chassisNr='1' chassis='1' port='0x1'
busNr='1' index='0'/>
+ <pcihole64 unit='KiB'>1024</pcihole64>
+ </controller>
+ <controller type='pci' index='1' model='pci-bridge'>
+ <model name='pci-bridge'/>
+ <target chassisNr='2' chassis='2' port='0x2'
busNr='2' index='2'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </controller>
+ <controller type='pci' index='2'
model='pci-expander-bus'>
+ <model name='pxb'/>
+ <target chassisNr='3' chassis='3' port='0x3'
busNr='3' index='3'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
+ </controller>
+ <controller type='usb' index='0' model='none'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
b/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
new file mode 100644
index 000000000..bbe360e25
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
@@ -0,0 +1,43 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <currentMemory unit='KiB'>1048576</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='ppc64' machine='pseries'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <cpu>
+ <numa>
+ <cell id='0' cpus='0' memory='1048576'
unit='KiB'/>
+ </numa>
+ </cpu>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-ppc64</emulator>
+ <controller type='pci' index='0' model='pci-root'>
+ <model name='spapr-pci-host-bridge'/>
+ <target chassisNr='1' chassis='1' port='0x1'
busNr='1' index='0'/>
+ </controller>
+ <controller type='pci' index='1' model='pci-root'>
+ <model name='spapr-pci-host-bridge'/>
+ <target chassisNr='2' chassis='2' port='0x2'
busNr='2' index='1'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='2' model='pci-bridge'>
+ <model name='pci-bridge'/>
+ <target chassisNr='3' chassis='3' port='0x3'
busNr='3' index='3'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x0'/>
+ </controller>
+ <controller type='usb' index='0' model='none'/>
+ <memballoon model='none'/>
+ <panic model='pseries'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
b/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
new file mode 100644
index 000000000..5ef7aa564
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
@@ -0,0 +1,76 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <currentMemory unit='KiB'>1048576</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='q35'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <cpu>
+ <numa>
+ <cell id='0' cpus='0' memory='1048576'
unit='KiB'/>
+ </numa>
+ </cpu>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='pci' index='0' model='pcie-root'>
+ <target chassisNr='1' chassis='1' port='0x1'
busNr='1' index='0'/>
+ <pcihole64 unit='KiB'>1024</pcihole64>
+ </controller>
+ <controller type='pci' index='1'
model='dmi-to-pci-bridge'>
+ <model name='i82801b11-bridge'/>
+ <target chassisNr='2' chassis='2' port='0x2'
busNr='2' index='2'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x1e' function='0x0'/>
+ </controller>
+ <controller type='pci' index='2' model='pci-bridge'>
+ <model name='pci-bridge'/>
+ <target chassisNr='3' chassis='3' port='0x3'
busNr='3' index='3'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x01'
slot='0x00' function='0x0'/>
+ </controller>
+ <controller type='pci' index='3'
model='pcie-expander-bus'>
+ <model name='pxb-pcie'/>
+ <target chassisNr='4' chassis='4' port='0x4'
busNr='4' index='4'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x02' function='0x0'/>
+ </controller>
+ <controller type='pci' index='4'
model='pcie-root-port'>
+ <model name='pcie-root-port'/>
+ <target chassisNr='5' chassis='5' port='0x5'
busNr='5' index='5'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </controller>
+ <controller type='pci' index='5'
model='pcie-switch-upstream-port'>
+ <model name='x3130-upstream'/>
+ <target chassisNr='6' chassis='6' port='0x6'
busNr='6' index='6'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x04'
slot='0x00' function='0x0'/>
+ </controller>
+ <controller type='pci' index='6'
model='pcie-switch-downstream-port'>
+ <model name='xio3130-downstream'/>
+ <target chassisNr='7' chassis='7' port='0x7'
busNr='7' index='7'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x05'
slot='0x00' function='0x0'/>
+ </controller>
+ <controller type='usb' index='0' model='none'/>
+ <controller type='sata' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x1f' function='0x2'/>
+ </controller>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index d3544a1ef..48d774856 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1103,6 +1103,23 @@ mymain(void)
QEMU_CAPS_DEVICE_IOH3420,
QEMU_CAPS_HDA_DUPLEX);
+ DO_TEST("i440fx-controllers-pciopts",
+ QEMU_CAPS_I440FX_PCI_HOLE64_SIZE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_PXB);
+ DO_TEST("q35-controllers-pciopts",
+ QEMU_CAPS_Q35_PCI_HOLE64_SIZE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_PXB_PCIE,
+ QEMU_CAPS_DEVICE_PCIE_ROOT_PORT,
+ QEMU_CAPS_DEVICE_X3130_UPSTREAM,
+ QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM);
+ DO_TEST("pseries-controllers-pciopts",
+ QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
+ QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE);
+
DO_TEST("hostdev-scsi-vhost-scsi-ccw",
QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VHOST_SCSI,
QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_VIRTIO_CCW);
--
2.14.3
7:12 a.m.
New subject: [libvirt] [PATCH REBASE 1/2] tests: Add some tests for PCI controller options
On 02/05/2018 11:08 AM, Andrea Bolognani wrote:
The input configurations set all existing options for all PCI
controllers, to see what ends up showing up in the output.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
.../i440fx-controllers-pciopts.args | 24 +++++++
.../i440fx-controllers-pciopts.xml | 36 ++++++++++
.../pseries-controllers-pciopts.args | 22 +++++++
.../pseries-controllers-pciopts.xml | 35 ++++++++++
.../qemuxml2argvdata/q35-controllers-pciopts.args | 28 ++++++++
tests/qemuxml2argvdata/q35-controllers-pciopts.xml | 60 +++++++++++++++++
tests/qemuxml2argvtest.c | 17 +++++
.../i440fx-controllers-pciopts.xml | 45 +++++++++++++
.../pseries-controllers-pciopts.xml | 43 ++++++++++++
.../qemuxml2xmloutdata/q35-controllers-pciopts.xml | 76 ++++++++++++++++++++++
tests/qemuxml2xmltest.c | 17 +++++
11 files changed, 403 insertions(+)
create mode 100644 tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
create mode 100644 tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml
create mode 100644 tests/qemuxml2argvdata/pseries-controllers-pciopts.args
create mode 100644 tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
create mode 100644 tests/qemuxml2argvdata/q35-controllers-pciopts.args
create mode 100644 tests/qemuxml2argvdata/q35-controllers-pciopts.xml
create mode 100644 tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
create mode 100644 tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
create mode 100644 tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
Not quite sure I understand the need. The only capability not currently
used in some way is QEMU_CAPS_I440FX_PCI_HOLE64_SIZE.
I guess there's nothing wrong with adding them other than test bloat.
Still there's nothing to "force" someone that adds some new thing to
update one of the three if some new option/capability is added.
Ironically adding (for example) QEMU_CAPS_Q35_PCI_HOLE64_SIZE to
i440fx-* doesn't cause any failure. I didn't try other ones.
BTW: This is what led me to determine that the qemuxml2argv isn't being
built any more and a response to Dan's series.
I'm not opposed to this being added, but do you think we should add
something does the opposite check? That the wrong PCI adapter on the
wrong machine? The wrong option on the wrong adapter is a question for
the next patch though...
John
diff --git a/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
new file mode 100644
index 000000000..d85fae5c9
--- /dev/null
+++ b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
@@ -0,0 +1,24 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest \
+-S \
+-M pc \
+-m 1024 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-numa node,nodeid=0,cpus=0,mem=1024 \
+-uuid 496d7ea8-9739-544b-4ebd-ef08be936e8b \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-global i440FX-pcihost.pci-hole64-size=1024K \
+-device pci-bridge,chassis_nr=2,id=pci.1,bus=pci.0,addr=0x3 \
+-device pxb,bus_nr=3,id=pci.2,numa_node=0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml
b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml
new file mode 100644
index 000000000..861a66589
--- /dev/null
+++ b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml
@@ -0,0 +1,36 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ </os>
+ <cpu>
+ <numa>
+ <cell id='0' cpus='0' memory='1048576'
unit='KiB'/>
+ </numa>
+ </cpu>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='pci' index='0' model='pci-root'>
+ <!-- Only pcihole64 should be preserved -->
+ <target busNr='1' chassis='1' chassisNr='1'
index='0' port='1'/>
+ <pcihole64 unit='KiB'>1024</pcihole64>
+ </controller>
+ <controller type='pci' index='1' model='pci-bridge'>
+ <!-- Only chassisNr should be preserved -->
+ <target busNr='2' chassis='2' chassisNr='2'
index='2' port='2'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='2'
model='pci-expander-bus'>
+ <!-- Only busNr and numaNode should be preserved -->
+ <target busNr='3' chassis='3' chassisNr='3'
index='3' port='3'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='usb' index='0' model='none'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/pseries-controllers-pciopts.args
b/tests/qemuxml2argvdata/pseries-controllers-pciopts.args
new file mode 100644
index 000000000..5f1edfc83
--- /dev/null
+++ b/tests/qemuxml2argvdata/pseries-controllers-pciopts.args
@@ -0,0 +1,22 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-ppc64 \
+-name guest \
+-S \
+-M pseries \
+-m 1024 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-numa node,nodeid=0,cpus=0,mem=1024 \
+-uuid 496d7ea8-9739-544b-4ebd-ef08be936e8b \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-boot c \
+-device spapr-pci-host-bridge,index=1,id=pci.1,numa_node=0 \
+-device pci-bridge,chassis_nr=3,id=pci.2,bus=pci.0,addr=0x1
diff --git a/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
b/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
new file mode 100644
index 000000000..43353cba6
--- /dev/null
+++ b/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
@@ -0,0 +1,35 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='ppc64' machine='pseries'>hvm</type>
+ </os>
+ <cpu>
+ <numa>
+ <cell id='0' cpus='0' memory='1048576'
unit='KiB'/>
+ </numa>
+ </cpu>
+ <devices>
+ <emulator>/usr/bin/qemu-system-ppc64</emulator>
+ <controller type='pci' index='0' model='pci-root'>
+ <!-- Only targetIndex should be preserved -->
+ <target busNr='1' chassis='1' chassisNr='1'
index='0' port='1'/>
+ </controller>
+ <controller type='pci' index='1' model='pci-root'>
+ <!-- Only numaNode and targetIndex should be preserved -->
+ <target busNr='2' chassis='2' chassisNr='2'
index='1' port='2'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='2' model='pci-bridge'>
+ <!-- Only chassisNr should be preserved -->
+ <target busNr='3' chassis='3' chassisNr='3'
index='3' port='3'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='usb' index='0' model='none'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/q35-controllers-pciopts.args
b/tests/qemuxml2argvdata/q35-controllers-pciopts.args
new file mode 100644
index 000000000..44259f502
--- /dev/null
+++ b/tests/qemuxml2argvdata/q35-controllers-pciopts.args
@@ -0,0 +1,28 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest \
+-S \
+-M q35 \
+-m 1024 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-numa node,nodeid=0,cpus=0,mem=1024 \
+-uuid 496d7ea8-9739-544b-4ebd-ef08be936e8b \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-global q35-pcihost.pci-hole64-size=1024K \
+-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
+-device pci-bridge,chassis_nr=3,id=pci.2,bus=pci.1,addr=0x0 \
+-device pxb-pcie,bus_nr=4,id=pci.3,numa_node=0,bus=pcie.0,addr=0x2 \
+-device pcie-root-port,port=0x5,chassis=5,id=pci.4,bus=pcie.0,addr=0x3 \
+-device x3130-upstream,id=pci.5,bus=pci.4,addr=0x0 \
+-device xio3130-downstream,port=0x7,chassis=7,id=pci.6,bus=pci.5,addr=0x0
diff --git a/tests/qemuxml2argvdata/q35-controllers-pciopts.xml
b/tests/qemuxml2argvdata/q35-controllers-pciopts.xml
new file mode 100644
index 000000000..40db83615
--- /dev/null
+++ b/tests/qemuxml2argvdata/q35-controllers-pciopts.xml
@@ -0,0 +1,60 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='q35'>hvm</type>
+ </os>
+ <cpu>
+ <numa>
+ <cell id='0' cpus='0' memory='1048576'
unit='KiB'/>
+ </numa>
+ </cpu>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='pci' index='0' model='pcie-root'>
+ <!-- Only pcihole64 should be preserved -->
+ <target busNr='1' chassis='1' chassisNr='1'
index='0' port='1'/>
+ <pcihole64 unit='KiB'>1024</pcihole64>
+ </controller>
+ <controller type='pci' index='1'
model='dmi-to-pci-bridge'>
+ <!-- No option should be preserved -->
+ <target busNr='2' chassis='2' chassisNr='2'
index='2' port='2'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='2' model='pci-bridge'>
+ <!-- Only chassisNr should be preserved -->
+ <target busNr='3' chassis='3' chassisNr='3'
index='3' port='3'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='3'
model='pcie-expander-bus'>
+ <!-- Only busNr and numaNode should be preserved -->
+ <target busNr='4' chassis='4' chassisNr='4'
index='4' port='4'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='4'
model='pcie-root-port'>
+ <!-- Only chassis and port should be preserved -->
+ <target busNr='5' chassis='5' chassisNr='5'
index='5' port='5'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='5'
model='pcie-switch-upstream-port'>
+ <!-- No option should be preserved -->
+ <target busNr='6' chassis='6' chassisNr='6'
index='6' port='6'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='6'
model='pcie-switch-downstream-port'>
+ <!-- Only chassis and port should be preserved -->
+ <target busNr='7' chassis='7' chassisNr='7'
index='7' port='7'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='usb' index='0' model='none'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c8739909d..595b541d5 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2436,6 +2436,23 @@ mymain(void)
QEMU_CAPS_DEVICE_IOH3420,
QEMU_CAPS_DEVICE_PXB_PCIE);
+ DO_TEST("i440fx-controllers-pciopts",
+ QEMU_CAPS_I440FX_PCI_HOLE64_SIZE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_PXB);
+ DO_TEST("q35-controllers-pciopts",
+ QEMU_CAPS_Q35_PCI_HOLE64_SIZE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_PXB_PCIE,
+ QEMU_CAPS_DEVICE_PCIE_ROOT_PORT,
+ QEMU_CAPS_DEVICE_X3130_UPSTREAM,
+ QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM);
+ DO_TEST("pseries-controllers-pciopts",
+ QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
+ QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE);
+
DO_TEST("hostdev-scsi-lsi",
QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI,
QEMU_CAPS_DEVICE_SCSI_GENERIC);
diff --git a/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
b/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
new file mode 100644
index 000000000..d171d1370
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
@@ -0,0 +1,45 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <currentMemory unit='KiB'>1048576</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <cpu>
+ <numa>
+ <cell id='0' cpus='0' memory='1048576'
unit='KiB'/>
+ </numa>
+ </cpu>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='pci' index='0' model='pci-root'>
+ <target chassisNr='1' chassis='1' port='0x1'
busNr='1' index='0'/>
+ <pcihole64 unit='KiB'>1024</pcihole64>
+ </controller>
+ <controller type='pci' index='1' model='pci-bridge'>
+ <model name='pci-bridge'/>
+ <target chassisNr='2' chassis='2' port='0x2'
busNr='2' index='2'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </controller>
+ <controller type='pci' index='2'
model='pci-expander-bus'>
+ <model name='pxb'/>
+ <target chassisNr='3' chassis='3' port='0x3'
busNr='3' index='3'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
+ </controller>
+ <controller type='usb' index='0' model='none'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
b/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
new file mode 100644
index 000000000..bbe360e25
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
@@ -0,0 +1,43 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <currentMemory unit='KiB'>1048576</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='ppc64' machine='pseries'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <cpu>
+ <numa>
+ <cell id='0' cpus='0' memory='1048576'
unit='KiB'/>
+ </numa>
+ </cpu>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-ppc64</emulator>
+ <controller type='pci' index='0' model='pci-root'>
+ <model name='spapr-pci-host-bridge'/>
+ <target chassisNr='1' chassis='1' port='0x1'
busNr='1' index='0'/>
+ </controller>
+ <controller type='pci' index='1' model='pci-root'>
+ <model name='spapr-pci-host-bridge'/>
+ <target chassisNr='2' chassis='2' port='0x2'
busNr='2' index='1'>
+ <node>0</node>
+ </target>
+ </controller>
+ <controller type='pci' index='2' model='pci-bridge'>
+ <model name='pci-bridge'/>
+ <target chassisNr='3' chassis='3' port='0x3'
busNr='3' index='3'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x0'/>
+ </controller>
+ <controller type='usb' index='0' model='none'/>
+ <memballoon model='none'/>
+ <panic model='pseries'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
b/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
new file mode 100644
index 000000000..5ef7aa564
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
@@ -0,0 +1,76 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <currentMemory unit='KiB'>1048576</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='q35'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <cpu>
+ <numa>
+ <cell id='0' cpus='0' memory='1048576'
unit='KiB'/>
+ </numa>
+ </cpu>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='pci' index='0' model='pcie-root'>
+ <target chassisNr='1' chassis='1' port='0x1'
busNr='1' index='0'/>
+ <pcihole64 unit='KiB'>1024</pcihole64>
+ </controller>
+ <controller type='pci' index='1'
model='dmi-to-pci-bridge'>
+ <model name='i82801b11-bridge'/>
+ <target chassisNr='2' chassis='2' port='0x2'
busNr='2' index='2'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x1e' function='0x0'/>
+ </controller>
+ <controller type='pci' index='2' model='pci-bridge'>
+ <model name='pci-bridge'/>
+ <target chassisNr='3' chassis='3' port='0x3'
busNr='3' index='3'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x01'
slot='0x00' function='0x0'/>
+ </controller>
+ <controller type='pci' index='3'
model='pcie-expander-bus'>
+ <model name='pxb-pcie'/>
+ <target chassisNr='4' chassis='4' port='0x4'
busNr='4' index='4'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x02' function='0x0'/>
+ </controller>
+ <controller type='pci' index='4'
model='pcie-root-port'>
+ <model name='pcie-root-port'/>
+ <target chassisNr='5' chassis='5' port='0x5'
busNr='5' index='5'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </controller>
+ <controller type='pci' index='5'
model='pcie-switch-upstream-port'>
+ <model name='x3130-upstream'/>
+ <target chassisNr='6' chassis='6' port='0x6'
busNr='6' index='6'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x04'
slot='0x00' function='0x0'/>
+ </controller>
+ <controller type='pci' index='6'
model='pcie-switch-downstream-port'>
+ <model name='xio3130-downstream'/>
+ <target chassisNr='7' chassis='7' port='0x7'
busNr='7' index='7'>
+ <node>0</node>
+ </target>
+ <address type='pci' domain='0x0000' bus='0x05'
slot='0x00' function='0x0'/>
+ </controller>
+ <controller type='usb' index='0' model='none'/>
+ <controller type='sata' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x1f' function='0x2'/>
+ </controller>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index d3544a1ef..48d774856 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1103,6 +1103,23 @@ mymain(void)
QEMU_CAPS_DEVICE_IOH3420,
QEMU_CAPS_HDA_DUPLEX);
+ DO_TEST("i440fx-controllers-pciopts",
+ QEMU_CAPS_I440FX_PCI_HOLE64_SIZE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_PXB);
+ DO_TEST("q35-controllers-pciopts",
+ QEMU_CAPS_Q35_PCI_HOLE64_SIZE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_PXB_PCIE,
+ QEMU_CAPS_DEVICE_PCIE_ROOT_PORT,
+ QEMU_CAPS_DEVICE_X3130_UPSTREAM,
+ QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM);
+ DO_TEST("pseries-controllers-pciopts",
+ QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
+ QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE);
+
DO_TEST("hostdev-scsi-vhost-scsi-ccw",
QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VHOST_SCSI,
QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_VIRTIO_CCW);
3:32 a.m.
New subject: [libvirt] [PATCH REBASE 1/2] tests: Add some tests for PCI controller options
On Sun, 2018-02-11 at 08:12 -0500, John Ferlan wrote:
On 02/05/2018 11:08 AM, Andrea Bolognani wrote:
> The input configurations set all existing options for all PCI
> controllers, to see what ends up showing up in the output.
Not quite sure I understand the need. The only capability not currently
used in some way is QEMU_CAPS_I440FX_PCI_HOLE64_SIZE.
The idea is that existing tests only cover valid PCI controller
options being handled correctly, not what happens when you specify
an option which is not relevant to the PCI controller at hand.
I guess there's nothing wrong with adding them other than test
bloat.
Still there's nothing to "force" someone that adds some new thing to
update one of the three if some new option/capability is added.
That's correct. Can you think of a way to make sure that happens?
Ironically adding (for example) QEMU_CAPS_Q35_PCI_HOLE64_SIZE to
i440fx-* doesn't cause any failure. I didn't try other ones.
Of course it wouldn't: capabilities are a property of the QEMU
binary, and the same QEMU binary is used to run both i440fx and
q35 guests. Even for something entirely outlandish like adding a
q35-specific capability to a pSeries test you shouldn't see any
failure unless you actually attempt to use the QEMU feature the
capability is tied to.
I'm not opposed to this being added, but do you think we should
add
something does the opposite check? That the wrong PCI adapter on the
wrong machine? The wrong option on the wrong adapter is a question for
the next patch though...
We should already have at least *some* coverage for that, eg.
pseries-serial-invalid-machine and similar. I'm certainly not
volunteering to go over all controller and device and machine
types and write tests for all combinations :)
--
Andrea Bolognani / Red Hat / Virtualization
4:29 p.m.
New subject: [libvirt] [PATCH REBASE 1/2] tests: Add some tests for PCI controller options
On 02/12/2018 04:32 AM, Andrea Bolognani wrote:
On Sun, 2018-02-11 at 08:12 -0500, John Ferlan wrote:
> On 02/05/2018 11:08 AM, Andrea Bolognani wrote:
>> The input configurations set all existing options for all PCI
>> controllers, to see what ends up showing up in the output.
>
> Not quite sure I understand the need. The only capability not currently
> used in some way is QEMU_CAPS_I440FX_PCI_HOLE64_SIZE.
The idea is that existing tests only cover valid PCI controller
options being handled correctly, not what happens when you specify
an option which is not relevant to the PCI controller at hand.
Well I understood you're going for a put the stake in the ground in
order to supply that valid list of options for the pci controllers on
the related machines.
> I guess there's nothing wrong with adding them other than
test bloat.
> Still there's nothing to "force" someone that adds some new thing to
> update one of the three if some new option/capability is added.
That's correct. Can you think of a way to make sure that happens?
and of course that got me to wondering if it was possible in any way to
have some way that the "i440fx" xml2{xml|argv} to know that it's
"covering" all it's known and supported relative options. Secondarily
to know when someone adds something to a machine that won't support it
before they get to the point of actually running and things falling over
dead because of the wrong configuration provided.
Nothing sprang quickly to mind to try other than adding comments which
we all know are ignored anyway ;-0...
I'm not against this patch going in, but if someone has agita over a
small about of test bloat to list everything "currently" possible for a
machine type then they should speak up!
> Ironically adding (for example) QEMU_CAPS_Q35_PCI_HOLE64_SIZE to
> i440fx-* doesn't cause any failure. I didn't try other ones.
Of course it wouldn't: capabilities are a property of the QEMU
binary, and the same QEMU binary is used to run both i440fx and
q35 guests. Even for something entirely outlandish like adding a
q35-specific capability to a pSeries test you shouldn't see any
failure unless you actually attempt to use the QEMU feature the
capability is tied to.
> I'm not opposed to this being added, but do you think we should add
> something does the opposite check? That the wrong PCI adapter on the
> wrong machine? The wrong option on the wrong adapter is a question for
> the next patch though...
We should already have at least *some* coverage for that, eg.
pseries-serial-invalid-machine and similar. I'm certainly not
volunteering to go over all controller and device and machine
types and write tests for all combinations :)
We have something - good...
John
10:08 a.m.
New subject: [libvirt] [PATCH REBASE 2/2] qemu: Clean up PCI controller options
Most of the options are only applicable to one or two controller
types, so they should be filtered out everywhere else.
This will reduce user confusion and, in at least one corner case,
prevent guests from disappearing on daemon restart.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1483816
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 117 +++++++++++++++++++++
.../pseries-controllers-pciopts.xml | 1 +
.../i440fx-controllers-pciopts.xml | 7 +-
tests/qemuxml2xmloutdata/pcie-expander-bus.xml | 4 +-
.../pseries-controllers-pciopts.xml | 8 +-
.../qemuxml2xmloutdata/q35-controllers-pciopts.xml | 21 +---
6 files changed, 128 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index df433c2f0..867f805bd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4717,6 +4717,121 @@ qemuDomainShmemDefPostParse(virDomainShmemDefPtr shm)
#define QEMU_USB_XHCI_MAXPORTS 15
+/**
+ * qemuDomainPCIControllerCleanupOpts:
+ * @cont: controller
+ *
+ * Clean up PCI options for @cont so that only options applicable to
+ * the controller model will actually be set.
+ */
+static void
+qemuDomainPCIControllerCleanupOpts(const virDomainDef *def,
+ virDomainControllerDefPtr cont)
+{
+ if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI)
+ return;
+
+ /* pcihole64 and targetIndex */
+ switch ((virDomainControllerModelPCI) cont->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ /* These controllers support all options; however,
+ * targetIndex is only supported for pSeries guests and
+ * pcihole64 is only supported on x86 */
+ if (!qemuDomainIsPSeries(def))
+ cont->opts.pciopts.targetIndex = -1;
+ if (!ARCH_IS_X86(def->os.arch)) {
+ cont->opts.pciopts.pcihole64 = false;
+ cont->opts.pciopts.pcihole64size = 0;
+ }
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+ /* These controllers don't support any option */
+ cont->opts.pciopts.pcihole64 = false;
+ cont->opts.pciopts.pcihole64size = 0;
+ ATTRIBUTE_FALLTHROUGH;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ /* These controllers support all options except targetIndex */
+ cont->opts.pciopts.targetIndex = -1;
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
+ /* busNr and numaNode */
+ switch ((virDomainControllerModelPCI) cont->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+ /* These controllers support all options */
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ /* These controllers don't support any option */
+ cont->opts.pciopts.numaNode = -1;
+ ATTRIBUTE_FALLTHROUGH;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ /* These controllers support all options except busNr; however,
+ * numaNode is only supported for pSeries guests */
+ cont->opts.pciopts.busNr = -1;
+ if (!qemuDomainIsPSeries(def))
+ cont->opts.pciopts.numaNode = -1;
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
+ /* chassis and port */
+ switch ((virDomainControllerModelPCI) cont->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ /* These controllers support all options */
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+ /* These controllers don't support any option */
+ cont->opts.pciopts.chassis = -1;
+ cont->opts.pciopts.port = -1;
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
+ /* chassisNr */
+ switch ((virDomainControllerModelPCI) cont->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ /* These controllers support all options */
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+ /* These controllers don't support any option */
+ cont->opts.pciopts.chassisNr = -1;
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+}
+
+
static int
qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
const virDomainDef *def,
@@ -4799,6 +4914,8 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
+ qemuDomainPCIControllerCleanupOpts(def, cont);
+
/* pSeries guests can have multiple pci-root controllers,
* but other machine types only support a single one */
if (!qemuDomainIsPSeries(def) &&
diff --git a/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
b/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
index 43353cba6..a3237ff2c 100644
--- a/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
+++ b/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
@@ -16,6 +16,7 @@
<controller type='pci' index='0' model='pci-root'>
<!-- Only targetIndex should be preserved -->
<target busNr='1' chassis='1' chassisNr='1'
index='0' port='1'/>
+ <pcihole64 unit='KiB'>1024</pcihole64>
</controller>
<controller type='pci' index='1' model='pci-root'>
<!-- Only numaNode and targetIndex should be preserved -->
diff --git a/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
b/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
index d171d1370..d1868fdc2 100644
--- a/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
+++ b/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
@@ -20,19 +20,16 @@
<devices>
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<controller type='pci' index='0' model='pci-root'>
- <target chassisNr='1' chassis='1' port='0x1'
busNr='1' index='0'/>
<pcihole64 unit='KiB'>1024</pcihole64>
</controller>
<controller type='pci' index='1' model='pci-bridge'>
<model name='pci-bridge'/>
- <target chassisNr='2' chassis='2' port='0x2'
busNr='2' index='2'>
- <node>0</node>
- </target>
+ <target chassisNr='2'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
</controller>
<controller type='pci' index='2'
model='pci-expander-bus'>
<model name='pxb'/>
- <target chassisNr='3' chassis='3' port='0x3'
busNr='3' index='3'>
+ <target busNr='3'>
<node>0</node>
</target>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
diff --git a/tests/qemuxml2xmloutdata/pcie-expander-bus.xml
b/tests/qemuxml2xmloutdata/pcie-expander-bus.xml
index aaac423ca..b6498fd2e 100644
--- a/tests/qemuxml2xmloutdata/pcie-expander-bus.xml
+++ b/tests/qemuxml2xmloutdata/pcie-expander-bus.xml
@@ -36,9 +36,7 @@
</controller>
<controller type='pci' index='2'
model='pcie-root-port'>
<model name='ioh3420'/>
- <target chassis='2' port='0x0' busNr='220'>
- <node>1</node>
- </target>
+ <target chassis='2' port='0x0'/>
<address type='pci' domain='0x0000' bus='0x01'
slot='0x00' function='0x0'/>
</controller>
<controller type='pci' index='3'
model='pcie-switch-upstream-port'>
diff --git a/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
b/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
index bbe360e25..b5d646765 100644
--- a/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
+++ b/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
@@ -21,19 +21,17 @@
<emulator>/usr/bin/qemu-system-ppc64</emulator>
<controller type='pci' index='0' model='pci-root'>
<model name='spapr-pci-host-bridge'/>
- <target chassisNr='1' chassis='1' port='0x1'
busNr='1' index='0'/>
+ <target index='0'/>
</controller>
<controller type='pci' index='1' model='pci-root'>
<model name='spapr-pci-host-bridge'/>
- <target chassisNr='2' chassis='2' port='0x2'
busNr='2' index='1'>
+ <target index='1'>
<node>0</node>
</target>
</controller>
<controller type='pci' index='2' model='pci-bridge'>
<model name='pci-bridge'/>
- <target chassisNr='3' chassis='3' port='0x3'
busNr='3' index='3'>
- <node>0</node>
- </target>
+ <target chassisNr='3'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x0'/>
</controller>
<controller type='usb' index='0' model='none'/>
diff --git a/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
b/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
index 5ef7aa564..d8a014853 100644
--- a/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
+++ b/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
@@ -20,49 +20,36 @@
<devices>
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<controller type='pci' index='0' model='pcie-root'>
- <target chassisNr='1' chassis='1' port='0x1'
busNr='1' index='0'/>
<pcihole64 unit='KiB'>1024</pcihole64>
</controller>
<controller type='pci' index='1'
model='dmi-to-pci-bridge'>
<model name='i82801b11-bridge'/>
- <target chassisNr='2' chassis='2' port='0x2'
busNr='2' index='2'>
- <node>0</node>
- </target>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x1e' function='0x0'/>
</controller>
<controller type='pci' index='2' model='pci-bridge'>
<model name='pci-bridge'/>
- <target chassisNr='3' chassis='3' port='0x3'
busNr='3' index='3'>
- <node>0</node>
- </target>
+ <target chassisNr='3'/>
<address type='pci' domain='0x0000' bus='0x01'
slot='0x00' function='0x0'/>
</controller>
<controller type='pci' index='3'
model='pcie-expander-bus'>
<model name='pxb-pcie'/>
- <target chassisNr='4' chassis='4' port='0x4'
busNr='4' index='4'>
+ <target busNr='4'>
<node>0</node>
</target>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x02' function='0x0'/>
</controller>
<controller type='pci' index='4'
model='pcie-root-port'>
<model name='pcie-root-port'/>
- <target chassisNr='5' chassis='5' port='0x5'
busNr='5' index='5'>
- <node>0</node>
- </target>
+ <target chassis='5' port='0x5'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
</controller>
<controller type='pci' index='5'
model='pcie-switch-upstream-port'>
<model name='x3130-upstream'/>
- <target chassisNr='6' chassis='6' port='0x6'
busNr='6' index='6'>
- <node>0</node>
- </target>
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00' function='0x0'/>
</controller>
<controller type='pci' index='6'
model='pcie-switch-downstream-port'>
<model name='xio3130-downstream'/>
- <target chassisNr='7' chassis='7' port='0x7'
busNr='7' index='7'>
- <node>0</node>
- </target>
+ <target chassis='7' port='0x7'/>
<address type='pci' domain='0x0000' bus='0x05'
slot='0x00' function='0x0'/>
</controller>
<controller type='usb' index='0' model='none'/>
--
2.14.3
7:22 a.m.
New subject: [libvirt] [PATCH REBASE 2/2] qemu: Clean up PCI controller options
On 02/05/2018 11:08 AM, Andrea Bolognani wrote:
Most of the options are only applicable to one or two controller
types, so they should be filtered out everywhere else.
This will reduce user confusion and, in at least one corner case,
prevent guests from disappearing on daemon restart.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1483816
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 117 +++++++++++++++++++++
.../pseries-controllers-pciopts.xml | 1 +
.../i440fx-controllers-pciopts.xml | 7 +-
tests/qemuxml2xmloutdata/pcie-expander-bus.xml | 4 +-
.../pseries-controllers-pciopts.xml | 8 +-
.../qemuxml2xmloutdata/q35-controllers-pciopts.xml | 21 +---
6 files changed, 128 insertions(+), 30 deletions(-)
So, if I'm reading things correctly - rather than fail because someone
set an option on a controller (and sometimes for a machine) for which
it's not supported, the choice is to ignore the value and enforce the
default.
This does seem to go against what's been traditionally done (at least my
recollection of it) for other XML options being wrongly applied to some
other device.
Even the bz is a big vague:
Expected results:
Schema for the 'target' field in <controller model='pci-bridge'>
should
not accept 'chassis' and 'port' parameters for 'q35' machine type
But I'd read that as some sort of failure is expected rather than
acceptance and quietly changing.
Curious what other thoughts on this exist? At the very least be a bit
more pointed in the commit message rather than just saying "filtered" it
should be pointed out that if one is set, it's going to be cleared.
I didn't see any of these in the ABI Stability tests so that's good!
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index df433c2f0..867f805bd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4717,6 +4717,121 @@ qemuDomainShmemDefPostParse(virDomainShmemDefPtr shm)
#define QEMU_USB_XHCI_MAXPORTS 15
+/**
+ * qemuDomainPCIControllerCleanupOpts:
+ * @cont: controller
+ *
+ * Clean up PCI options for @cont so that only options applicable to
+ * the controller model will actually be set.
+ */
+static void
+qemuDomainPCIControllerCleanupOpts(const virDomainDef *def,
+ virDomainControllerDefPtr cont)
+{
+ if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI)
+ return;
This is redundant with [1]
+
+ /* pcihole64 and targetIndex */
+ switch ((virDomainControllerModelPCI) cont->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ /* These controllers support all options; however,
+ * targetIndex is only supported for pSeries guests and
+ * pcihole64 is only supported on x86 */
+ if (!qemuDomainIsPSeries(def))
+ cont->opts.pciopts.targetIndex = -1;
+ if (!ARCH_IS_X86(def->os.arch)) {
+ cont->opts.pciopts.pcihole64 = false;
+ cont->opts.pciopts.pcihole64size = 0;
+ }
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+ /* These controllers don't support any option */
+ cont->opts.pciopts.pcihole64 = false;
+ cont->opts.pciopts.pcihole64size = 0;
+ ATTRIBUTE_FALLTHROUGH;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ /* These controllers support all options except targetIndex */
+ cont->opts.pciopts.targetIndex = -1;
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
+ /* busNr and numaNode */
+ switch ((virDomainControllerModelPCI) cont->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+ /* These controllers support all options */
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ /* These controllers don't support any option */
+ cont->opts.pciopts.numaNode = -1;
+ ATTRIBUTE_FALLTHROUGH;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ /* These controllers support all options except busNr; however,
+ * numaNode is only supported for pSeries guests */
+ cont->opts.pciopts.busNr = -1;
+ if (!qemuDomainIsPSeries(def))
+ cont->opts.pciopts.numaNode = -1;
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
+ /* chassis and port */
+ switch ((virDomainControllerModelPCI) cont->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ /* These controllers support all options */
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+ /* These controllers don't support any option */
+ cont->opts.pciopts.chassis = -1;
+ cont->opts.pciopts.port = -1;
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
+ /* chassisNr */
+ switch ((virDomainControllerModelPCI) cont->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ /* These controllers support all options */
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+ /* These controllers don't support any option */
+ cont->opts.pciopts.chassisNr = -1;
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+}
+
+
static int
qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
const virDomainDef *def,
@@ -4799,6 +4914,8 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
[1] only called when type is PCI...
John
+ qemuDomainPCIControllerCleanupOpts(def, cont);
+
/* pSeries guests can have multiple pci-root controllers,
* but other machine types only support a single one */
if (!qemuDomainIsPSeries(def) &&
diff --git a/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
b/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
index 43353cba6..a3237ff2c 100644
--- a/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
+++ b/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
@@ -16,6 +16,7 @@
<controller type='pci' index='0' model='pci-root'>
<!-- Only targetIndex should be preserved -->
<target busNr='1' chassis='1' chassisNr='1'
index='0' port='1'/>
+ <pcihole64 unit='KiB'>1024</pcihole64>
</controller>
<controller type='pci' index='1' model='pci-root'>
<!-- Only numaNode and targetIndex should be preserved -->
diff --git a/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
b/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
index d171d1370..d1868fdc2 100644
--- a/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
+++ b/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
@@ -20,19 +20,16 @@
<devices>
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<controller type='pci' index='0' model='pci-root'>
- <target chassisNr='1' chassis='1' port='0x1'
busNr='1' index='0'/>
<pcihole64 unit='KiB'>1024</pcihole64>
</controller>
<controller type='pci' index='1' model='pci-bridge'>
<model name='pci-bridge'/>
- <target chassisNr='2' chassis='2' port='0x2'
busNr='2' index='2'>
- <node>0</node>
- </target>
+ <target chassisNr='2'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
</controller>
<controller type='pci' index='2'
model='pci-expander-bus'>
<model name='pxb'/>
- <target chassisNr='3' chassis='3' port='0x3'
busNr='3' index='3'>
+ <target busNr='3'>
<node>0</node>
</target>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
diff --git a/tests/qemuxml2xmloutdata/pcie-expander-bus.xml
b/tests/qemuxml2xmloutdata/pcie-expander-bus.xml
index aaac423ca..b6498fd2e 100644
--- a/tests/qemuxml2xmloutdata/pcie-expander-bus.xml
+++ b/tests/qemuxml2xmloutdata/pcie-expander-bus.xml
@@ -36,9 +36,7 @@
</controller>
<controller type='pci' index='2'
model='pcie-root-port'>
<model name='ioh3420'/>
- <target chassis='2' port='0x0' busNr='220'>
- <node>1</node>
- </target>
+ <target chassis='2' port='0x0'/>
<address type='pci' domain='0x0000' bus='0x01'
slot='0x00' function='0x0'/>
</controller>
<controller type='pci' index='3'
model='pcie-switch-upstream-port'>
diff --git a/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
b/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
index bbe360e25..b5d646765 100644
--- a/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
+++ b/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
@@ -21,19 +21,17 @@
<emulator>/usr/bin/qemu-system-ppc64</emulator>
<controller type='pci' index='0' model='pci-root'>
<model name='spapr-pci-host-bridge'/>
- <target chassisNr='1' chassis='1' port='0x1'
busNr='1' index='0'/>
+ <target index='0'/>
</controller>
<controller type='pci' index='1' model='pci-root'>
<model name='spapr-pci-host-bridge'/>
- <target chassisNr='2' chassis='2' port='0x2'
busNr='2' index='1'>
+ <target index='1'>
<node>0</node>
</target>
</controller>
<controller type='pci' index='2' model='pci-bridge'>
<model name='pci-bridge'/>
- <target chassisNr='3' chassis='3' port='0x3'
busNr='3' index='3'>
- <node>0</node>
- </target>
+ <target chassisNr='3'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x0'/>
</controller>
<controller type='usb' index='0' model='none'/>
diff --git a/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
b/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
index 5ef7aa564..d8a014853 100644
--- a/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
+++ b/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
@@ -20,49 +20,36 @@
<devices>
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<controller type='pci' index='0' model='pcie-root'>
- <target chassisNr='1' chassis='1' port='0x1'
busNr='1' index='0'/>
<pcihole64 unit='KiB'>1024</pcihole64>
</controller>
<controller type='pci' index='1'
model='dmi-to-pci-bridge'>
<model name='i82801b11-bridge'/>
- <target chassisNr='2' chassis='2' port='0x2'
busNr='2' index='2'>
- <node>0</node>
- </target>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x1e' function='0x0'/>
</controller>
<controller type='pci' index='2' model='pci-bridge'>
<model name='pci-bridge'/>
- <target chassisNr='3' chassis='3' port='0x3'
busNr='3' index='3'>
- <node>0</node>
- </target>
+ <target chassisNr='3'/>
<address type='pci' domain='0x0000' bus='0x01'
slot='0x00' function='0x0'/>
</controller>
<controller type='pci' index='3'
model='pcie-expander-bus'>
<model name='pxb-pcie'/>
- <target chassisNr='4' chassis='4' port='0x4'
busNr='4' index='4'>
+ <target busNr='4'>
<node>0</node>
</target>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x02' function='0x0'/>
</controller>
<controller type='pci' index='4'
model='pcie-root-port'>
<model name='pcie-root-port'/>
- <target chassisNr='5' chassis='5' port='0x5'
busNr='5' index='5'>
- <node>0</node>
- </target>
+ <target chassis='5' port='0x5'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
</controller>
<controller type='pci' index='5'
model='pcie-switch-upstream-port'>
<model name='x3130-upstream'/>
- <target chassisNr='6' chassis='6' port='0x6'
busNr='6' index='6'>
- <node>0</node>
- </target>
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00' function='0x0'/>
</controller>
<controller type='pci' index='6'
model='pcie-switch-downstream-port'>
<model name='xio3130-downstream'/>
- <target chassisNr='7' chassis='7' port='0x7'
busNr='7' index='7'>
- <node>0</node>
- </target>
+ <target chassis='7' port='0x7'/>
<address type='pci' domain='0x0000' bus='0x05'
slot='0x00' function='0x0'/>
</controller>
<controller type='usb' index='0' model='none'/>
3:50 a.m.
New subject: [libvirt] [PATCH REBASE 2/2] qemu: Clean up PCI controller options
On Sun, 2018-02-11 at 08:22 -0500, John Ferlan wrote:
On 02/05/2018 11:08 AM, Andrea Bolognani wrote:
> Most of the options are only applicable to one or two controller
> types, so they should be filtered out everywhere else.
>
> This will reduce user confusion and, in at least one corner case,
> prevent guests from disappearing on daemon restart.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1483816
So, if I'm reading things correctly - rather than fail because someone
set an option on a controller (and sometimes for a machine) for which
it's not supported, the choice is to ignore the value and enforce the
default.
This does seem to go against what's been traditionally done (at least my
recollection of it) for other XML options being wrongly applied to some
other device.
Even the bz is a big vague:
Expected results:
Schema for the 'target' field in <controller model='pci-bridge'>
should
not accept 'chassis' and 'port' parameters for 'q35' machine
type
But I'd read that as some sort of failure is expected rather than
acceptance and quietly changing.
Yeah, I'm not sure why I implemented it that way myself. I'll
move everything to a Validate() sub-callback and error out if any
unexpected option is set for a controller.
That will make the test cases added in 1/2 slightly less useful,
though, as it will error out on the first such option instead of
showing that it's resetting all of them.
> +static void
> +qemuDomainPCIControllerCleanupOpts(const virDomainDef *def,
> + virDomainControllerDefPtr cont)
> +{
> + if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI)
> + return;
This is redundant with [1]
> @@ -4799,6 +4914,8 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr
cont,
>
> case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
[1] only called when type is PCI...
I prefer not embedding in a function knowledge about its callers,
because callers change all the time. In this specific case, I guess
the name is explicit enough that nobody would try calling it on a
USB controller, so we can probably skip the check.
--
Andrea Bolognani / Red Hat / Virtualization
2475
days inactive
2482
days old
7 comments
2 participants
participants (2)
-
Andrea Bolognani -
John Ferlan