10:27 a.m.
Changes from [v1]:
* error out instead of silently accept invalid options;
* shave quite a lot of yaks.
[v1] https://www.redhat.com/archives/libvir-list/2018-February/msg00244.html
Andrea Bolognani (15):
tests: Add some tests for PCI controller options
qemu: Move 'done' label in qemuBuildControllerDevStr()
qemu: Move skip for implicit PHB of pSeries guests
qemu: Restrict scope in qemuBuildControllerDevStr()
qemu: Simplify modelName stringification
qemu: Defer capability check to command line generation time
qemu: Clear out qemuDomainDeviceDefValidateControllerPCI()
qemu: Validate PCI controllers (index)
qemu: Validate PCI controllers (modelName)
qemu: Validate PCI controllers (targetIndex)
qemu: Validate PCI controllers (pcihole64)
qemu: Validate PCI controllers (busNr)
qemu: Validate PCI controllers (numaNode)
qemu: Validate PCI controllers (chassisNr)
qemu: Validate PCI controllers (chassis and port)
src/qemu/qemu_command.c | 80 +++-
src/qemu/qemu_domain.c | 425 +++++++++++++--------
.../i440fx-controllers-pciopts.xml | 36 ++
tests/qemuxml2argvdata/pcie-expander-bus.xml | 3 -
.../pseries-controllers-pciopts.xml | 35 ++
tests/qemuxml2argvdata/q35-controllers-pciopts.xml | 60 +++
tests/qemuxml2argvtest.c | 4 +
tests/qemuxml2xmloutdata/pcie-expander-bus.xml | 4 +-
8 files changed, 475 insertions(+), 172 deletions(-)
create mode 100644 tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml
create mode 100644 tests/qemuxml2argvdata/pseries-controllers-pciopts.xml
create mode 100644 tests/qemuxml2argvdata/q35-controllers-pciopts.xml
--
2.14.3
10:27 a.m.
New subject: [libvirt] [PATCH v2 01/15] 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 ade18c055..4154663da 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2435,6 +2435,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 0eb9e6c77..7b8a16078 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1102,6 +1102,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
10:27 a.m.
New subject: [libvirt] [PATCH v2 02/15] qemu: Move 'done' label in qemuBuildControllerDevStr()
Even when we skip part of the processing, we still want error
checking on the buffer.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 55e573b61..1ab5b0818 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2801,10 +2801,10 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
if (qemuBuildDeviceAddressStr(&buf, domainDef, &def->info, qemuCaps) <
0)
goto error;
+ done:
if (virBufferCheckError(&buf) < 0)
goto error;
- done:
*devstr = virBufferContentAndReset(&buf);
return 0;
--
2.14.3
8:55 a.m.
New subject: [libvirt] [PATCH v2 02/15] qemu: Move 'done' label in qemuBuildControllerDevStr()
On Fri, Feb 16, 2018 at 17:27:59 +0100, Andrea Bolognani wrote:
Even when we skip part of the processing, we still want error
checking on the buffer.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK
10:28 a.m.
New subject: [libvirt] [PATCH v2 03/15] qemu: Move skip for implicit PHB of pSeries guests
Performing the skip earlier will help us making the function
nicer later on. We also make the condition for the skip a bit
more precise, though that'a more for self-documenting purposes
and doesn't change anything in practice.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1ab5b0818..5e4dfcf75 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2732,6 +2732,14 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST)
modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
+ /* Skip the implicit PHB for pSeries guests */
+ if (def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT &&
+ pciopts->modelName ==
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE &&
+ pciopts->targetIndex == 0 &&
+ qemuDomainIsPSeries(domainDef)) {
+ goto done;
+ }
+
switch ((virDomainControllerModelPCI) def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
virBufferAsprintf(&buf, "%s,chassis_nr=%d,id=%s",
@@ -2759,10 +2767,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
pciopts->chassis, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
- /* Skip the implicit one */
- if (pciopts->targetIndex == 0)
- goto done;
-
virBufferAsprintf(&buf, "%s,index=%d,id=%s",
modelName, pciopts->targetIndex,
def->info.alias);
--
2.14.3
8:58 a.m.
New subject: [libvirt] [PATCH v2 03/15] qemu: Move skip for implicit PHB of pSeries guests
On Fri, Feb 16, 2018 at 17:28:00 +0100, Andrea Bolognani wrote:
Performing the skip earlier will help us making the function
nicer later on. We also make the condition for the skip a bit
more precise, though that'a more for self-documenting purposes
and doesn't change anything in practice.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1ab5b0818..5e4dfcf75 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2732,6 +2732,14 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST)
modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
+ /* Skip the implicit PHB for pSeries guests */
+ if (def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT &&
+ pciopts->modelName ==
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE &&
+ pciopts->targetIndex == 0 &&
+ qemuDomainIsPSeries(domainDef)) {
I'm not sure about the last line. Shouldn't that alredy be validated? At
least in the case when the PHB model should not be present on
non-pseries qemus?
ACK if you agree and drop the last line.
9:12 a.m.
New subject: [libvirt] [PATCH v2 03/15] qemu: Move skip for implicit PHB of pSeries guests
On Mon, 2018-02-19 at 15:58 +0100, Peter Krempa wrote:
On Fri, Feb 16, 2018 at 17:28:00 +0100, Andrea Bolognani wrote:
> Performing the skip earlier will help us making the function
> nicer later on. We also make the condition for the skip a bit
> more precise, though that'a more for self-documenting purposes
> and doesn't change anything in practice.
>
> Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 1ab5b0818..5e4dfcf75 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2732,6 +2732,14 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
> def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST)
> modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
>
> + /* Skip the implicit PHB for pSeries guests */
> + if (def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT &&
> + pciopts->modelName ==
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE &&
> + pciopts->targetIndex == 0 &&
> + qemuDomainIsPSeries(domainDef)) {
I'm not sure about the last line. Shouldn't that alredy be validated? At
least in the case when the PHB model should not be present on
non-pseries qemus?
ACK if you agree and drop the last line.
Strictly speaking, both the check on modelName and that on the
machine type are redundant, because we make sure targetIndex is only
set for PHBs and thus only for pSeries guests.
That said, checking again doesn't hurt and makes the reason for
skipping more obvious. It might also help catch bugs introduced in
the validate callback that would result in a controller that was
supposed to be skipped showing up in the output for some test case.
--
Andrea Bolognani / Red Hat / Virtualization
10:28 a.m.
New subject: [libvirt] [PATCH v2 04/15] qemu: Restrict scope in qemuBuildControllerDevStr()
Some variables are only used for PCI controllers, and we're going
to add more soon. We can declare them in the 'case' scope rather
than in the function scope to make it a bit nicer.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5e4dfcf75..2291bf5da 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2625,8 +2625,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
int address_type = def->info.type;
- const virDomainPCIControllerOpts *pciopts;
- const char *modelName = NULL;
*devstr = NULL;
@@ -2726,7 +2724,10 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
break;
- case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
+ case VIR_DOMAIN_CONTROLLER_TYPE_PCI: {
+ const virDomainPCIControllerOpts *pciopts;
+ const char *modelName = NULL;
+
pciopts = &def->opts.pciopts;
if (def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT &&
def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST)
@@ -2781,6 +2782,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
goto error;
}
break;
+ }
case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
--
2.14.3
9 a.m.
New subject: [libvirt] [PATCH v2 04/15] qemu: Restrict scope in qemuBuildControllerDevStr()
On Fri, Feb 16, 2018 at 17:28:01 +0100, Andrea Bolognani wrote:
Some variables are only used for PCI controllers, and we're
going
to add more soon. We can declare them in the 'case' scope rather
than in the function scope to make it a bit nicer.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5e4dfcf75..2291bf5da 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2625,8 +2625,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
int address_type = def->info.type;
- const virDomainPCIControllerOpts *pciopts;
- const char *modelName = NULL;
*devstr = NULL;
Since the next patch changes both lines again I suggest you squash it
into the next patch.
9:28 a.m.
New subject: [libvirt] [PATCH v2 04/15] qemu: Restrict scope in qemuBuildControllerDevStr()
On Mon, 2018-02-19 at 16:00 +0100, Peter Krempa wrote:
> @@ -2625,8 +2625,6 @@ qemuBuildControllerDevStr(const
virDomainDef *domainDef,
> {
> virBuffer buf = VIR_BUFFER_INITIALIZER;
> int address_type = def->info.type;
> - const virDomainPCIControllerOpts *pciopts;
> - const char *modelName = NULL;
>
> *devstr = NULL;
Since the next patch changes both lines again I suggest you squash it
into the next patch.
They're semantically independent changes so I thought it would be
better to perform them separately, but I can squash them together
if that's your preference.
--
Andrea Bolognani / Red Hat / Virtualization
10:28 a.m.
New subject: [libvirt] [PATCH v2 05/15] qemu: Simplify modelName stringification
There's no need to perform checks before conversion, we can
just go try and check the results later on.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2291bf5da..a44a1b2d2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2725,13 +2725,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
break;
case VIR_DOMAIN_CONTROLLER_TYPE_PCI: {
- const virDomainPCIControllerOpts *pciopts;
- const char *modelName = NULL;
-
- pciopts = &def->opts.pciopts;
- if (def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT &&
- def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST)
- modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
+ const virDomainPCIControllerOpts *pciopts = &def->opts.pciopts;
+ const char *modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
/* Skip the implicit PHB for pSeries guests */
if (def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT &&
@@ -2741,6 +2736,13 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
goto done;
}
+ if (!modelName) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown virDomainControllerPCIModelName value:
%d"),
+ pciopts->modelName);
+ return -1;
+ }
+
switch ((virDomainControllerModelPCI) def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
virBufferAsprintf(&buf, "%s,chassis_nr=%d,id=%s",
--
2.14.3
9:06 a.m.
New subject: [libvirt] [PATCH v2 05/15] qemu: Simplify modelName stringification
On Fri, Feb 16, 2018 at 17:28:02 +0100, Andrea Bolognani wrote:
There's no need to perform checks before conversion, we can
just go try and check the results later on.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2291bf5da..a44a1b2d2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2725,13 +2725,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
break;
case VIR_DOMAIN_CONTROLLER_TYPE_PCI: {
- const virDomainPCIControllerOpts *pciopts;
- const char *modelName = NULL;
-
- pciopts = &def->opts.pciopts;
- if (def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT &&
- def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST)
- modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
+ const virDomainPCIControllerOpts *pciopts = &def->opts.pciopts;
+ const char *modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
This is not equivalent. virDomainControllerPCIModelName implementation
actually defines a string for VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT so
it will not return NULL and the commit message does not explain why/if
it is actually okay.
/* Skip the implicit PHB for pSeries guests */
if (def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT &&
@@ -2741,6 +2736,13 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
goto done;
}
+ if (!modelName) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown virDomainControllerPCIModelName value:
%d"),
I think this was under discussion somewhere in this series.
+ pciopts->modelName);
+ return -1;
+ }
+
switch ((virDomainControllerModelPCI) def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
virBufferAsprintf(&buf, "%s,chassis_nr=%d,id=%s",
--
2.14.3
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
9:26 a.m.
New subject: [libvirt] [PATCH v2 05/15] qemu: Simplify modelName stringification
On Mon, 2018-02-19 at 16:06 +0100, Peter Krempa wrote:
> case VIR_DOMAIN_CONTROLLER_TYPE_PCI: {
> - const virDomainPCIControllerOpts *pciopts;
> - const char *modelName = NULL;
> -
> - pciopts = &def->opts.pciopts;
> - if (def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT &&
> - def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST)
> - modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
> + const virDomainPCIControllerOpts *pciopts = &def->opts.pciopts;
> + const char *modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
This is not equivalent. virDomainControllerPCIModelName implementation
actually defines a string for VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT so
it will not return NULL and the commit message does not explain why/if
it is actually okay.
The result doesn't change because the PCIE_ROOT case in the switch
statement immediately following the conversion does nothing but
error out, which makes whether or not modelName is NULL irrelevant.
I can add the explanation above to the commit message if you want.
--
Andrea Bolognani / Red Hat / Virtualization
6:52 a.m.
New subject: [libvirt] [PATCH v2 05/15] qemu: Simplify modelName stringification
On Mon, Feb 19, 2018 at 16:26:35 +0100, Andrea Bolognani wrote:
On Mon, 2018-02-19 at 16:06 +0100, Peter Krempa wrote:
> > case VIR_DOMAIN_CONTROLLER_TYPE_PCI: {
> > - const virDomainPCIControllerOpts *pciopts;
> > - const char *modelName = NULL;
> > -
> > - pciopts = &def->opts.pciopts;
> > - if (def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT &&
> > - def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST)
> > - modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
> > + const virDomainPCIControllerOpts *pciopts =
&def->opts.pciopts;
> > + const char *modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
>
> This is not equivalent. virDomainControllerPCIModelName implementation
> actually defines a string for VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT so
> it will not return NULL and the commit message does not explain why/if
> it is actually okay.
The result doesn't change because the PCIE_ROOT case in the switch
statement immediately following the conversion does nothing but
error out, which makes whether or not modelName is NULL irrelevant.
I mistakenly looked into the case for PCI_ROOT, so that mislead me. I
think it's okay in this case even without a comment. Thanks for the
explanation.
10:28 a.m.
New subject: [libvirt] [PATCH v2 06/15] qemu: Defer capability check to command line generation time
Validate time is a bit too early to check whether the required
capabilities are available, since the QEMU binary might have
been updated or replaced by the time we are asked to run the
guest.
Move capability checks (back) to qemuBuildControllerDevStr()
and get rid of a lot of redundancy in the process.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 50 +++++++++++++++++++++++++++++++
src/qemu/qemu_domain.c | 80 ++-----------------------------------------------
2 files changed, 52 insertions(+), 78 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a44a1b2d2..a957132bd 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2595,6 +2595,35 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def,
return 0;
}
+static int
+virDomainControllerPCIModelNameToQEMUCaps(int modelName)
+{
+ switch ((virDomainControllerPCIModelName) modelName) {
+ case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE:
+ return QEMU_CAPS_DEVICE_PCI_BRIDGE;
+ case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE:
+ return QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE;
+ case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420:
+ return QEMU_CAPS_DEVICE_IOH3420;
+ case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM:
+ return QEMU_CAPS_DEVICE_X3130_UPSTREAM;
+ case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM:
+ return QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM;
+ case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB:
+ return QEMU_CAPS_DEVICE_PXB;
+ case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE:
+ return QEMU_CAPS_DEVICE_PXB_PCIE;
+ case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT:
+ return QEMU_CAPS_DEVICE_PCIE_ROOT_PORT;
+ case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE:
+ return QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE;
+ case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE:
+ case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST:
+ break;
+ }
+ return -1;
+}
+
/**
* qemuBuildControllerDevStr:
@@ -2727,6 +2756,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
case VIR_DOMAIN_CONTROLLER_TYPE_PCI: {
const virDomainPCIControllerOpts *pciopts = &def->opts.pciopts;
const char *modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
+ int cap = virDomainControllerPCIModelNameToQEMUCaps(pciopts->modelName);
/* Skip the implicit PHB for pSeries guests */
if (def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT &&
@@ -2742,6 +2772,18 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
pciopts->modelName);
return -1;
}
+ if (cap < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown QEMU device for '%s'
controller"),
+ modelName);
+ return -1;
+ }
+ if (!virQEMUCapsGet(qemuCaps, cap)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("The '%s' device is not supported by this QEMU
binary"),
+ modelName);
+ return -1;
+ }
switch ((virDomainControllerModelPCI) def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
@@ -2770,6 +2812,14 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
pciopts->chassis, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ if (pciopts->numaNode != -1 &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the spapr-pci-host-bridge controller doesn't
"
+ "support numa_node in this QEMU binary"));
+ return -1;
+ }
+
virBufferAsprintf(&buf, "%s,index=%d,id=%s",
modelName, pciopts->targetIndex,
def->info.alias);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2bc0259ea..2fbae695a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4244,8 +4244,7 @@ qemuDomainDeviceDefValidateControllerSCSI(const
virDomainControllerDef *controll
static int
qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller,
- const virDomainDef *def,
- virQEMUCapsPtr qemuCaps)
+ const virDomainDef *def)
{
virDomainControllerModelPCI model = controller->model;
const virDomainPCIControllerOpts *pciopts;
@@ -4322,13 +4321,6 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pci-bridge controller is not supported "
- "in this QEMU binary"));
- return -1;
- }
-
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
@@ -4346,13 +4338,6 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pxb controller is not supported in this "
- "QEMU binary"));
- return -1;
- }
-
break;
case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
@@ -4364,13 +4349,6 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the dmi-to-pci-bridge (i82801b11-bridge) "
- "controller is not supported in this QEMU
binary"));
- return -1;
- }
-
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
@@ -4389,22 +4367,6 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
- if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420)
&&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pcie-root-port (ioh3420) controller "
- "is not supported in this QEMU binary"));
- return -1;
- }
-
- if ((pciopts->modelName ==
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT) &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pcie-root-port (pcie-root-port) controller "
- "is not supported in this QEMU binary"));
- return -1;
- }
-
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
@@ -4416,13 +4378,6 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pcie-switch-upstream-port (x3130-upstream) "
- "controller is not supported in this QEMU
binary"));
- return -1;
- }
-
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
@@ -4441,14 +4396,6 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("The pcie-switch-downstream-port "
- "(xio3130-downstream) controller is not "
- "supported in this QEMU binary"));
- return -1;
- }
-
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
@@ -4466,13 +4413,6 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB_PCIE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pxb-pcie controller is not supported "
- "in this QEMU binary"));
- return -1;
- }
-
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
@@ -4494,21 +4434,6 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the spapr-pci-host-bridge controller is not "
- "supported in this QEMU binary"));
- return -1;
- }
-
- if (pciopts->numaNode != -1 &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the spapr-pci-host-bridge controller doesn't
"
- "support numa_node in this QEMU binary"));
- return -1;
- }
-
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
@@ -4566,8 +4491,7 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef
*controller,
break;
case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
- ret = qemuDomainDeviceDefValidateControllerPCI(controller, def,
- qemuCaps);
+ ret = qemuDomainDeviceDefValidateControllerPCI(controller, def);
break;
case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
--
2.14.3
12:24 a.m.
New subject: [libvirt] [PATCH v2 06/15] qemu: Defer capability check to command line generation time
On Fri, Feb 16, 2018 at 17:28:03 +0100, Andrea Bolognani wrote:
Validate time is a bit too early to check whether the required
capabilities are available, since the QEMU binary might have
been updated or replaced by the time we are asked to run the
guest.
So are you having problem with the fact that the definition will be
rejected right away and not just when you try to start it?
Validate is re-run when starting the VM so a downgrade is handled
properly.
3:29 a.m.
New subject: [libvirt] [PATCH v2 06/15] qemu: Defer capability check to command line generation time
On Mon, 2018-02-19 at 07:24 +0100, Peter Krempa wrote:
On Fri, Feb 16, 2018 at 17:28:03 +0100, Andrea Bolognani wrote:
> Validate time is a bit too early to check whether the required
> capabilities are available, since the QEMU binary might have
> been updated or replaced by the time we are asked to run the
> guest.
So are you having problem with the fact that the definition will be
rejected right away and not just when you try to start it?
Validate is re-run when starting the VM so a downgrade is handled
properly.
Right, but isn't checking for QEMU capabilities at validate time
unreasonably strict? A guest which uses eg. an invalid combination
of machine type and architecture will never become valid at a later
point, but a guest should not be considered invalid just because
the QEMU binary you happened to have installed at the time you
defined it lacked some features - the guest itself is perfectly
valid, it just can't be run :)
--
Andrea Bolognani / Red Hat / Virtualization
3:38 a.m.
New subject: [libvirt] [PATCH v2 06/15] qemu: Defer capability check to command line generation time
On Mon, Feb 19, 2018 at 10:29:18 +0100, Andrea Bolognani wrote:
On Mon, 2018-02-19 at 07:24 +0100, Peter Krempa wrote:
> On Fri, Feb 16, 2018 at 17:28:03 +0100, Andrea Bolognani wrote:
> > Validate time is a bit too early to check whether the required
> > capabilities are available, since the QEMU binary might have
> > been updated or replaced by the time we are asked to run the
> > guest.
>
> So are you having problem with the fact that the definition will be
> rejected right away and not just when you try to start it?
>
> Validate is re-run when starting the VM so a downgrade is handled
> properly.
Right, but isn't checking for QEMU capabilities at validate time
unreasonably strict?
Well, the only difference is that you are unable to 'define' such XML. I
don't think it's strict since you would not be able to run it anyways.
A guest which uses eg. an invalid combination
of machine type and architecture will never become valid at a later
point, but a guest should not be considered invalid just because
the QEMU binary you happened to have installed at the time you
defined it lacked some features - the guest itself is perfectly
valid, it just can't be run :)
I think the term 'guest' here is misleading. The configuration is
invalid and thus you can't really create a guest out of it.
This is actually a great example, as you actually are not able to define
a XML which would fail to be run anyways.
The specific reason for the validation callback is to not make the
definition vanish in such case.
In general, I think that the command line generator is the wrong place
for such checks since they are scattered around and you can't really
figure out whether you can start such config until you've done a lot of
preparation steps.
6:57 a.m.
New subject: [libvirt] [PATCH v2 06/15] qemu: Defer capability check to command line generation time
On 02/19/2018 04:29 AM, Andrea Bolognani wrote:
On Mon, 2018-02-19 at 07:24 +0100, Peter Krempa wrote:
> On Fri, Feb 16, 2018 at 17:28:03 +0100, Andrea Bolognani wrote:
>> Validate time is a bit too early to check whether the required
>> capabilities are available, since the QEMU binary might have
>> been updated or replaced by the time we are asked to run the
>> guest.
>
> So are you having problem with the fact that the definition will be
> rejected right away and not just when you try to start it?
>
> Validate is re-run when starting the VM so a downgrade is handled
> properly.
Right, but isn't checking for QEMU capabilities at validate time
unreasonably strict? A guest which uses eg. an invalid combination
of machine type and architecture will never become valid at a later
point, but a guest should not be considered invalid just because
the QEMU binary you happened to have installed at the time you
defined it lacked some features - the guest itself is perfectly
valid, it just can't be run :)
IIRC - the decision processing I used for moving the capability checking
into Validation for controller options (PCI, SCSI, USB, SATA) was
because at least qemuDomainDefValidateMemoryHotplug already had failure
paths when capabilities weren't set. Undoing PCI would perhaps mean
quite a few more places that need adjustment to get that bald yak.
As for the question of "unreasonably strict" at Validation time - isn't
a purpose of Validation to ensure certain combinations that were defined
would work together properly? It just so happens that using certain
combinations of options with a specific version of a binary could be
that type of check. Although I do see your point. Personally I'd rather
get something "right" when defining it rather than find out later - it's
one of those - well why didn't you tell me that before type gripes.
John
7:04 a.m.
New subject: [libvirt] [PATCH v2 06/15] qemu: Defer capability check to command line generation time
On Mon, Feb 19, 2018 at 10:29:18AM +0100, Andrea Bolognani wrote:
On Mon, 2018-02-19 at 07:24 +0100, Peter Krempa wrote:
> On Fri, Feb 16, 2018 at 17:28:03 +0100, Andrea Bolognani wrote:
> > Validate time is a bit too early to check whether the required
> > capabilities are available, since the QEMU binary might have
> > been updated or replaced by the time we are asked to run the
> > guest.
>
> So are you having problem with the fact that the definition will be
> rejected right away and not just when you try to start it?
>
> Validate is re-run when starting the VM so a downgrade is handled
> properly.
Right, but isn't checking for QEMU capabilities at validate time
unreasonably strict? A guest which uses eg. an invalid combination
of machine type and architecture will never become valid at a later
point, but a guest should not be considered invalid just because
the QEMU binary you happened to have installed at the time you
defined it lacked some features - the guest itself is perfectly
valid, it just can't be run :)
Given that we increasingly fill in alot of information in the XML at define
time, we already have a general expectation that the QEMU binary will be
present at define time. I think this is not unreasonable - we suggest apps
call virConnectGetCapabilities and/or virDomainGetCapabilities to understand
what is installed/available before creating an XML document to define. Those
APIs of course require binaries to be installed too. So I don't think we
should really put effort into coping with defining XML for a time when the
QEMU binaries aren't installed. Such a scenario is so unlikely to be hit
that any code trying to cope with that is going to be largely untested and
fragile, so it would be a disservice to pretend it'll be something worth
relying on.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
7:13 a.m.
New subject: [libvirt] [PATCH v2 06/15] qemu: Defer capability check to command line generation time
On Mon, Feb 19, 2018 at 13:04:08 +0000, Daniel Berrange wrote:
On Mon, Feb 19, 2018 at 10:29:18AM +0100, Andrea Bolognani wrote:
> On Mon, 2018-02-19 at 07:24 +0100, Peter Krempa wrote:
> > On Fri, Feb 16, 2018 at 17:28:03 +0100, Andrea Bolognani wrote:
> > > Validate time is a bit too early to check whether the required
> > > capabilities are available, since the QEMU binary might have
> > > been updated or replaced by the time we are asked to run the
> > > guest.
> >
> > So are you having problem with the fact that the definition will be
> > rejected right away and not just when you try to start it?
> >
> > Validate is re-run when starting the VM so a downgrade is handled
> > properly.
>
> Right, but isn't checking for QEMU capabilities at validate time
> unreasonably strict? A guest which uses eg. an invalid combination
> of machine type and architecture will never become valid at a later
> point, but a guest should not be considered invalid just because
> the QEMU binary you happened to have installed at the time you
> defined it lacked some features - the guest itself is perfectly
> valid, it just can't be run :)
Given that we increasingly fill in alot of information in the XML at define
time, we already have a general expectation that the QEMU binary will be
present at define time. I think this is not unreasonable - we suggest apps
call virConnectGetCapabilities and/or virDomainGetCapabilities to understand
what is installed/available before creating an XML document to define. Those
APIs of course require binaries to be installed too. So I don't think we
should really put effort into coping with defining XML for a time when the
QEMU binaries aren't installed. Such a scenario is so unlikely to be hit
that any code trying to cope with that is going to be largely untested and
fragile, so it would be a disservice to pretend it'll be something worth
relying on.
The only situation when we should not fail if QEMU is not installed and
you restart libvirtd. Making defined domains disappear is bad.
The good thing is that at that point all defaults should be already
filled in so it should not matter much whether capabilities are present.
Other than that, I agree. Checking stuff uprfront is usually better than
get to the situation when it fails to start.
7:21 a.m.
New subject: [libvirt] [PATCH v2 06/15] qemu: Defer capability check to command line generation time
On Mon, Feb 19, 2018 at 02:13:14PM +0100, Peter Krempa wrote:
On Mon, Feb 19, 2018 at 13:04:08 +0000, Daniel Berrange wrote:
> On Mon, Feb 19, 2018 at 10:29:18AM +0100, Andrea Bolognani wrote:
> > On Mon, 2018-02-19 at 07:24 +0100, Peter Krempa wrote:
> > > On Fri, Feb 16, 2018 at 17:28:03 +0100, Andrea Bolognani wrote:
> > > > Validate time is a bit too early to check whether the required
> > > > capabilities are available, since the QEMU binary might have
> > > > been updated or replaced by the time we are asked to run the
> > > > guest.
> > >
> > > So are you having problem with the fact that the definition will be
> > > rejected right away and not just when you try to start it?
> > >
> > > Validate is re-run when starting the VM so a downgrade is handled
> > > properly.
> >
> > Right, but isn't checking for QEMU capabilities at validate time
> > unreasonably strict? A guest which uses eg. an invalid combination
> > of machine type and architecture will never become valid at a later
> > point, but a guest should not be considered invalid just because
> > the QEMU binary you happened to have installed at the time you
> > defined it lacked some features - the guest itself is perfectly
> > valid, it just can't be run :)
>
> Given that we increasingly fill in alot of information in the XML at define
> time, we already have a general expectation that the QEMU binary will be
> present at define time. I think this is not unreasonable - we suggest apps
> call virConnectGetCapabilities and/or virDomainGetCapabilities to understand
> what is installed/available before creating an XML document to define. Those
> APIs of course require binaries to be installed too. So I don't think we
> should really put effort into coping with defining XML for a time when the
> QEMU binaries aren't installed. Such a scenario is so unlikely to be hit
> that any code trying to cope with that is going to be largely untested and
> fragile, so it would be a disservice to pretend it'll be something worth
> relying on.
The only situation when we should not fail if QEMU is not installed and
you restart libvirtd. Making defined domains disappear is bad.
A long time back we did have a patch series somewhere that tried to
keep domain's loaded, but mark them as "broken" if the QEMU we needed
was missing. Can't remember why we never went further with it now...
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
7:30 a.m.
New subject: [libvirt] [PATCH v2 06/15] qemu: Defer capability check to command line generation time
On Mon, Feb 19, 2018 at 13:21:35 +0000, Daniel Berrange wrote:
On Mon, Feb 19, 2018 at 02:13:14PM +0100, Peter Krempa wrote:
> On Mon, Feb 19, 2018 at 13:04:08 +0000, Daniel Berrange wrote:
> > On Mon, Feb 19, 2018 at 10:29:18AM +0100, Andrea Bolognani wrote:
> > > On Mon, 2018-02-19 at 07:24 +0100, Peter Krempa wrote:
> > > > On Fri, Feb 16, 2018 at 17:28:03 +0100, Andrea Bolognani wrote:
> > > > > Validate time is a bit too early to check whether the required
> > > > > capabilities are available, since the QEMU binary might have
> > > > > been updated or replaced by the time we are asked to run the
> > > > > guest.
> > > >
> > > > So are you having problem with the fact that the definition will be
> > > > rejected right away and not just when you try to start it?
> > > >
> > > > Validate is re-run when starting the VM so a downgrade is handled
> > > > properly.
> > >
> > > Right, but isn't checking for QEMU capabilities at validate time
> > > unreasonably strict? A guest which uses eg. an invalid combination
> > > of machine type and architecture will never become valid at a later
> > > point, but a guest should not be considered invalid just because
> > > the QEMU binary you happened to have installed at the time you
> > > defined it lacked some features - the guest itself is perfectly
> > > valid, it just can't be run :)
> >
> > Given that we increasingly fill in alot of information in the XML at define
> > time, we already have a general expectation that the QEMU binary will be
> > present at define time. I think this is not unreasonable - we suggest apps
> > call virConnectGetCapabilities and/or virDomainGetCapabilities to understand
> > what is installed/available before creating an XML document to define. Those
> > APIs of course require binaries to be installed too. So I don't think we
> > should really put effort into coping with defining XML for a time when the
> > QEMU binaries aren't installed. Such a scenario is so unlikely to be hit
> > that any code trying to cope with that is going to be largely untested and
> > fragile, so it would be a disservice to pretend it'll be something worth
> > relying on.
>
> The only situation when we should not fail if QEMU is not installed and
> you restart libvirtd. Making defined domains disappear is bad.
A long time back we did have a patch series somewhere that tried to
keep domain's loaded, but mark them as "broken" if the QEMU we needed
was missing. Can't remember why we never went further with it now...
Well in the meanwhile I've made qemuCaps in the PostParse callbacks
optional and it's re-run in such case on startup of the VM. The
validation callbacks are not run when parsing status/config XMLs so it
should mostly work right now properly. (at least it fixed my case when
my binaries failed to exec due to broken deps)
8:45 a.m.
New subject: [libvirt] [PATCH v2 06/15] qemu: Defer capability check to command line generation time
On Mon, 2018-02-19 at 14:30 +0100, Peter Krempa wrote:
On Mon, Feb 19, 2018 at 13:21:35 +0000, Daniel Berrange wrote:
> On Mon, Feb 19, 2018 at 02:13:14PM +0100, Peter Krempa wrote:
> > The only situation when we should not fail if QEMU is not installed and
> > you restart libvirtd. Making defined domains disappear is bad.
I think we can all agree on that :)
> A long time back we did have a patch series somewhere that tried
to
> keep domain's loaded, but mark them as "broken" if the QEMU we needed
> was missing. Can't remember why we never went further with it now...
Well in the meanwhile I've made qemuCaps in the PostParse callbacks
optional and it's re-run in such case on startup of the VM. The
validation callbacks are not run when parsing status/config XMLs so it
should mostly work right now properly. (at least it fixed my case when
my binaries failed to exec due to broken deps)
Okay, I'll leave capabilities checks in the validate callback.
--
Andrea Bolognani / Red Hat / Virtualization
10:28 a.m.
New subject: [libvirt] [PATCH v2 07/15] qemu: Clear out qemuDomainDeviceDefValidateControllerPCI()
We will rewrite pretty much every single line of this function
over the course of the next several commits, and starting from
a clean slate rather than replacing it bit by bit makes the
resulting diffs unmeasurably easier to read and understand,
and you need fewer of them to boot. Trust me, I tried the other
approach first :)
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 199 +------------------------------------------------
1 file changed, 2 insertions(+), 197 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2fbae695a..9dc3d5597 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4243,204 +4243,9 @@ qemuDomainDeviceDefValidateControllerSCSI(const
virDomainControllerDef *controll
static int
-qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller,
- const virDomainDef *def)
+qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller
ATTRIBUTE_UNUSED,
+ const virDomainDef *def ATTRIBUTE_UNUSED)
{
- virDomainControllerModelPCI model = controller->model;
- const virDomainPCIControllerOpts *pciopts;
- const char *modelName = NULL;
-
- /* skip pcie-root */
- if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
- return 0;
-
- /* Skip pci-root, except for pSeries guests (which actually
- * support more than one PCI Host Bridge per guest) */
- if (!qemuDomainIsPSeries(def) &&
- controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
- return 0;
-
- /* First pass - just check the controller index for the model's
- * that we care to check... */
- switch (model) {
- 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:
- if (controller->idx == 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("index for pci controllers of model '%s' must
be > 0"),
- virDomainControllerModelPCITypeToString(model));
- return -1;
- }
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
- break;
- }
-
- pciopts = &controller->opts.pciopts;
- if (controller->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT &&
- controller->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST) {
- if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("autogenerated %s options not set"),
-
virDomainControllerModelPCITypeToString(controller->model));
- return -1;
- }
-
- modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
- if (!modelName) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown %s modelName value %d"),
-
virDomainControllerModelPCITypeToString(controller->model),
- pciopts->modelName);
- return -1;
- }
- }
-
- /* Second pass - now the model specific checks */
- switch (model) {
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
- if (pciopts->chassisNr == -1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pci-bridge options not set"));
- return -1;
- }
-
- if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' is not valid
"
- "for a pci-bridge"),
- modelName);
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
- if (pciopts->busNr == -1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pci-expander-bus options not
set"));
- return -1;
- }
-
- if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' is not valid
"
- "for a pci-expander-bus"),
- modelName);
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
- if (pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' is not valid
"
- "for a dmi-to-pci-bridge"),
- modelName);
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
- if (pciopts->chassis == -1 || pciopts->port == -1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pcie-root-port options not set"));
- return -1;
- }
-
- if ((pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420)
&&
- (pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' is not valid
"
- "for a pcie-root-port"),
- modelName);
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
- if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM)
{
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' is not valid
"
- "for a pcie-switch-upstream-port"),
- modelName);
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
- if (pciopts->chassis == -1 || pciopts->port == -1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pcie-switch-downstream-port "
- "options not set"));
- return -1;
- }
-
- if (pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' is not valid
"
- "for a pcie-switch-downstream-port"),
- modelName);
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
- if (pciopts->busNr == -1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pcie-expander-bus options not
set"));
- return -1;
- }
-
- if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' is not valid
"
- "for a pcie-expander-bus"),
- modelName);
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
- if (pciopts->targetIndex == -1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pci-root options not set"));
- return -1;
- }
-
- /* Skip the implicit one */
- if (pciopts->targetIndex == 0)
- return 0;
-
- if (pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' is not valid
"
- "for a pci-root"),
- modelName);
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
- break;
- }
-
return 0;
}
--
2.14.3
12:28 a.m.
New subject: [libvirt] [PATCH v2 07/15] qemu: Clear out qemuDomainDeviceDefValidateControllerPCI()
On Fri, Feb 16, 2018 at 17:28:04 +0100, Andrea Bolognani wrote:
We will rewrite pretty much every single line of this function
over the course of the next several commits, and starting from
a clean slate rather than replacing it bit by bit makes the
resulting diffs unmeasurably easier to read and understand,
and you need fewer of them to boot. Trust me, I tried the other
approach first :)
Will this remove any checks during the series? If yes, then you probably
should at first rename this function and add a almost-empty wrapper then
add new code to the wrapper and delete the renamed function at the end.
3:22 a.m.
New subject: [libvirt] [PATCH v2 07/15] qemu: Clear out qemuDomainDeviceDefValidateControllerPCI()
On Mon, 2018-02-19 at 07:28 +0100, Peter Krempa wrote:
On Fri, Feb 16, 2018 at 17:28:04 +0100, Andrea Bolognani wrote:
> We will rewrite pretty much every single line of this function
> over the course of the next several commits, and starting from
> a clean slate rather than replacing it bit by bit makes the
> resulting diffs unmeasurably easier to read and understand,
> and you need fewer of them to boot. Trust me, I tried the other
> approach first :)
Will this remove any checks during the series? If yes, then you probably
should at first rename this function and add a almost-empty wrapper then
add new code to the wrapper and delete the renamed function at the end.
No, if anything it *adds* a bunch of checks :)
Renaming the function won't work because then the compiler will
complain about it being unused. Unless you meant something like
/* Delete once done */
ValidateControllerPCIOld() {
/* Existing checks here */
}
ValidateControllerPCI() {
/* New checks here */
/* Delete once done */
ValidateControllerPCIOld();
}
which could actually do the trick.
--
Andrea Bolognani / Red Hat / Virtualization
3:40 a.m.
New subject: [libvirt] [PATCH v2 07/15] qemu: Clear out qemuDomainDeviceDefValidateControllerPCI()
On Mon, Feb 19, 2018 at 10:22:29 +0100, Andrea Bolognani wrote:
On Mon, 2018-02-19 at 07:28 +0100, Peter Krempa wrote:
> On Fri, Feb 16, 2018 at 17:28:04 +0100, Andrea Bolognani wrote:
> > We will rewrite pretty much every single line of this function
> > over the course of the next several commits, and starting from
> > a clean slate rather than replacing it bit by bit makes the
> > resulting diffs unmeasurably easier to read and understand,
> > and you need fewer of them to boot. Trust me, I tried the other
> > approach first :)
>
> Will this remove any checks during the series? If yes, then you probably
> should at first rename this function and add a almost-empty wrapper then
> add new code to the wrapper and delete the renamed function at the end.
No, if anything it *adds* a bunch of checks :)
Well, I meant that after applying this patch a bunch of checks will
vanish until you add them in the next patches, which I don't think we
should do.
Renaming the function won't work because then the compiler will
complain about it being unused. Unless you meant something like
/* Delete once done */
ValidateControllerPCIOld() {
/* Existing checks here */
}
ValidateControllerPCI() {
/* New checks here */
/* Delete once done */
ValidateControllerPCIOld();
}
which could actually do the trick.
I meant this flow obviously, so that the checks are kept until
fixed/reimplemented.
5:43 a.m.
New subject: [libvirt] [PATCH v2 07/15] qemu: Clear out qemuDomainDeviceDefValidateControllerPCI()
On Mon, 2018-02-19 at 10:40 +0100, Peter Krempa wrote:
> Renaming the function won't work because then the compiler
will
> complain about it being unused. Unless you meant something like
>
> /* Delete once done */
> ValidateControllerPCIOld() {
> /* Existing checks here */
> }
>
> ValidateControllerPCI() {
> /* New checks here */
>
> /* Delete once done */
> ValidateControllerPCIOld();
> }
>
> which could actually do the trick.
I meant this flow obviously, so that the checks are kept until
fixed/reimplemented.
It was not obvious to me when reading the first message :)
But I agree it's a better way to handle the situation, so I will
do it this way in v3.
--
Andrea Bolognani / Red Hat / Virtualization
10:28 a.m.
New subject: [libvirt] [PATCH v2 08/15] qemu: Validate PCI controllers (index)
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 54 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9dc3d5597..e4e67c585 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4243,9 +4243,61 @@ qemuDomainDeviceDefValidateControllerSCSI(const
virDomainControllerDef *controll
static int
-qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller
ATTRIBUTE_UNUSED,
- const virDomainDef *def ATTRIBUTE_UNUSED)
+qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller,
+ const virDomainDef *def)
{
+ const virDomainPCIControllerOpts *pciopts = &controller->opts.pciopts;
+ const char *model = virDomainControllerModelPCITypeToString(controller->model);
+ const char *modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
+
+ if (!model) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown virDomainControllerModelPCI value: %d"),
+ controller->model);
+ return -1;
+ }
+ if (!modelName) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown virDomainControllerPCIModelName value: %d"),
+ pciopts->modelName);
+ return -1;
+ }
+
+ /* index */
+ switch ((virDomainControllerModelPCI) controller->model) {
+ 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:
+ if (controller->idx == 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Index for '%s' controller must be >
0"),
+ model);
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ /* pci-root controllers for pSeries guests can have any index, but
+ * all other pci-root and pcie-root controller must have index 0 */
+ if (controller->idx != 0 &&
+ !(qemuDomainIsPSeries(def) &&
+ controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Index for '%s' controller must be 0"),
+ model);
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
return 0;
}
--
2.14.3
5:52 a.m.
New subject: [libvirt] [PATCH v2 08/15] qemu: Validate PCI controllers (index)
On Fri, Feb 16, 2018 at 05:28:05PM +0100, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 54 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9dc3d5597..e4e67c585 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4243,9 +4243,61 @@ qemuDomainDeviceDefValidateControllerSCSI(const
virDomainControllerDef *controll
static int
-qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller
ATTRIBUTE_UNUSED,
- const virDomainDef *def ATTRIBUTE_UNUSED)
+qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller,
+ const virDomainDef *def)
{
+ const virDomainPCIControllerOpts *pciopts = &controller->opts.pciopts;
+ const char *model = virDomainControllerModelPCITypeToString(controller->model);
+ const char *modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
+
+ if (!model) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown virDomainControllerModelPCI value: %d"),
Including type names in error messages is not too nice as a user facing
error - better to use plain english "Unexpected PCI controller model")
+ controller->model);
+ return -1;
+ }
+ if (!modelName) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown virDomainControllerPCIModelName value:
%d"),
+ pciopts->modelName);
Likewise here.
+ return -1;
+ }
+
+ /* index */
+ switch ((virDomainControllerModelPCI) controller->model) {
+ 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:
+ if (controller->idx == 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Index for '%s' controller must be >
0"),
+ model);
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ /* pci-root controllers for pSeries guests can have any index, but
+ * all other pci-root and pcie-root controller must have index 0 */
+ if (controller->idx != 0 &&
+ !(qemuDomainIsPSeries(def) &&
+ controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Index for '%s' controller must be 0"),
+ model);
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
Any time there is a _LAST element we should have an error report too
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unexpected controller model %d"), controller->model);
return -1;
It should also have an 'default:' next to the LAST. Both are things that
should never occur unless we've screwed up code elsewherein libvirt, but
we want to be robust about catching such screw ups. This is particularly
important for controller models, as we have many different controller
type specific enums all stored in the same struct field
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
6:05 a.m.
New subject: [libvirt] [PATCH v2 08/15] qemu: Validate PCI controllers (index)
On Mon, 2018-02-19 at 11:52 +0000, Daniel P. Berrangé wrote:
> +qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controller,
> + const virDomainDef *def)
> {
> + const virDomainPCIControllerOpts *pciopts = &controller->opts.pciopts;
> + const char *model =
virDomainControllerModelPCITypeToString(controller->model);
> + const char *modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
> +
> + if (!model) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unknown virDomainControllerModelPCI value:
%d"),
Including type names in error messages is not too nice as a user facing
error - better to use plain english "Unexpected PCI controller model")
But these are marked as internal errors, eg. They Should Never
Happen™, and if they ever do it's useful for developers to have
more detailed information. Users can't really do anything but
report the issue, after all.
> + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
> + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
> + /* pci-root controllers for pSeries guests can have any index, but
> + * all other pci-root and pcie-root controller must have index 0 */
> + if (controller->idx != 0 &&
> + !(qemuDomainIsPSeries(def) &&
> + controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Index for '%s' controller must be
0"),
> + model);
> + return -1;
> + }
> + break;
> +
> + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
Any time there is a _LAST element we should have an error report too
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unexpected controller model %d"), controller->model);
return -1;
It should also have an 'default:' next to the LAST. Both are things that
should never occur unless we've screwed up code elsewherein libvirt, but
we want to be robust about catching such screw ups. This is particularly
important for controller models, as we have many different controller
type specific enums all stored in the same struct field
Okay, I'll address that, but the point above about what error
message to show in such scenarios still applies IMHO.
--
Andrea Bolognani / Red Hat / Virtualization
10:28 a.m.
New subject: [libvirt] [PATCH v2 09/15] qemu: Validate PCI controllers (modelName)
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 110 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e4e67c585..2f76464df 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4298,6 +4298,116 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
break;
}
+ /* modelName */
+ switch ((virDomainControllerModelPCI) controller->model) {
+ 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:
+ if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Option '%s' not set for '%s'
controller"),
+ "modelName", model);
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+ switch ((virDomainControllerModelPCI) controller->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE
&&
+ pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Model name '%s' is not valid for '%s'
controller"),
+ modelName, "pci-root");
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Model name '%s' is not valid for '%s'
controller"),
+ modelName, "pci-bridge");
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ if (pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Model name '%s' is not valid for '%s'
controller"),
+ modelName, "dmi-to-pci-bridge");
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420
&&
+ pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT)
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Model name '%s' is not valid for '%s'
controller"),
+ modelName, "pcie-root-port");
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM)
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Model name '%s' is not valid for '%s'
controller"),
+ modelName, "pcie-switch-upstream-port");
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ if (pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Model name '%s' is not valid for '%s'
controller"),
+ modelName, "pci-switch-downstream-port");
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+ if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Model name '%s' is not valid for '%s'
controller"),
+ modelName, "pci-expander-bus");
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+ if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Model name '%s' is not valid for '%s'
controller"),
+ modelName, "pcie-expander-bus");
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Model name '%s' is not valid for '%s'
controller"),
+ modelName, "pcie-root");
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
return 0;
}
--
2.14.3
10:28 a.m.
New subject: [libvirt] [PATCH v2 10/15] qemu: Validate PCI controllers (targetIndex)
Some test cases that were specifically set up to exploit the
lack of checks on PCI controller options start failing with
these changes, so they are marked as such.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 43 ++++++++++++
.../i440fx-controllers-pciopts.args | 24 -------
.../pseries-controllers-pciopts.args | 22 -------
.../qemuxml2argvdata/q35-controllers-pciopts.args | 28 --------
tests/qemuxml2argvtest.c | 19 +-----
.../i440fx-controllers-pciopts.xml | 45 -------------
.../pseries-controllers-pciopts.xml | 43 ------------
.../qemuxml2xmloutdata/q35-controllers-pciopts.xml | 76 ----------------------
tests/qemuxml2xmltest.c | 17 -----
9 files changed, 46 insertions(+), 271 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
delete mode 100644 tests/qemuxml2argvdata/pseries-controllers-pciopts.args
delete mode 100644 tests/qemuxml2argvdata/q35-controllers-pciopts.args
delete mode 100644 tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
delete mode 100644 tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
delete mode 100644 tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2f76464df..52c76b515 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4408,6 +4408,49 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
break;
}
+ /* targetIndex */
+ switch ((virDomainControllerModelPCI) controller->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ /* PHBs for pSeries guests are stored as pci-root controllers and
+ * must have been assigned a targetIndex */
+ if (pciopts->targetIndex == -1 &&
+ qemuDomainIsPSeries(def)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Option '%s' not set for '%s'
controller"),
+ "targetIndex", model);
+ return -1;
+ }
+ /* targetIndex only applies to pSeries guests, so for any other
+ * guest type it being present is an error */
+ if (pciopts->targetIndex != -1 &&
+ !qemuDomainIsPSeries(def)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Option '%s' is not valid for '%s'
controller"),
+ "targetIndex", model);
+ return -1;
+ }
+ 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:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ if (pciopts->targetIndex != -1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Option '%s' is not valid for '%s'
controller"),
+ "targetIndex", model);
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
return 0;
}
diff --git a/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
deleted file mode 100644
index d85fae5c9..000000000
--- a/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args
+++ /dev/null
@@ -1,24 +0,0 @@
-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/pseries-controllers-pciopts.args
b/tests/qemuxml2argvdata/pseries-controllers-pciopts.args
deleted file mode 100644
index 5f1edfc83..000000000
--- a/tests/qemuxml2argvdata/pseries-controllers-pciopts.args
+++ /dev/null
@@ -1,22 +0,0 @@
-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/q35-controllers-pciopts.args
b/tests/qemuxml2argvdata/q35-controllers-pciopts.args
deleted file mode 100644
index 44259f502..000000000
--- a/tests/qemuxml2argvdata/q35-controllers-pciopts.args
+++ /dev/null
@@ -1,28 +0,0 @@
-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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 4154663da..525042e95 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2435,22 +2435,9 @@ 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_PARSE_ERROR("i440fx-controllers-pciopts", NONE);
+ DO_TEST_PARSE_ERROR("q35-controllers-pciopts", NONE);
+ DO_TEST_PARSE_ERROR("pseries-controllers-pciopts", NONE);
DO_TEST("hostdev-scsi-lsi",
QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI,
diff --git a/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
b/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
deleted file mode 100644
index d171d1370..000000000
--- a/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml
+++ /dev/null
@@ -1,45 +0,0 @@
-<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
deleted file mode 100644
index bbe360e25..000000000
--- a/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml
+++ /dev/null
@@ -1,43 +0,0 @@
-<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
deleted file mode 100644
index 5ef7aa564..000000000
--- a/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml
+++ /dev/null
@@ -1,76 +0,0 @@
-<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 7b8a16078..0eb9e6c77 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1102,23 +1102,6 @@ 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
10:28 a.m.
New subject: [libvirt] [PATCH v2 11/15] qemu: Validate PCI controllers (pcihole64)
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 52c76b515..b4dd7a7e8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4451,6 +4451,43 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
break;
}
+ /* pcihole64 */
+ switch ((virDomainControllerModelPCI) controller->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ /* The pcihole64 option only applies to x86 guests */
+ if ((pciopts->pcihole64 ||
+ pciopts->pcihole64size != 0) &&
+ !ARCH_IS_X86(def->os.arch)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Option '%s' is not valid for '%s'
controller "
+ "on '%s' architecture or '%s' machine
type"),
+ "pcihole64", model,
+ virArchToString(def->os.arch), def->os.machine);
+ return -1;
+ }
+ 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:
+ if (pciopts->pcihole64 ||
+ pciopts->pcihole64size != 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Option '%s' is not valid for '%s'
controller"),
+ "pcihole64", model);
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
return 0;
}
--
2.14.3
10:28 a.m.
New subject: [libvirt] [PATCH v2 12/15] qemu: Validate PCI controllers (busNr)
This change catches an invalid use of the option in our
test suite.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 31 ++++++++++++++++++++++++++
tests/qemuxml2argvdata/pcie-expander-bus.xml | 2 +-
tests/qemuxml2xmloutdata/pcie-expander-bus.xml | 2 +-
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b4dd7a7e8..81f347ce4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4488,6 +4488,37 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
break;
}
+ /* busNr */
+ switch ((virDomainControllerModelPCI) controller->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+ if (pciopts->busNr == -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Option '%s' not set for '%s'
controller"),
+ "busNr", model);
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_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:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ if (pciopts->busNr != -1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Option '%s' is not valid for '%s'
controller"),
+ "busNr", model);
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
return 0;
}
diff --git a/tests/qemuxml2argvdata/pcie-expander-bus.xml
b/tests/qemuxml2argvdata/pcie-expander-bus.xml
index ac01c26cc..237430a1e 100644
--- a/tests/qemuxml2argvdata/pcie-expander-bus.xml
+++ b/tests/qemuxml2argvdata/pcie-expander-bus.xml
@@ -35,7 +35,7 @@
<address type='pci' bus='0x00' slot='4'/>
</controller>
<controller type='pci' index='2'
model='pcie-root-port'>
- <target busNr='220'>
+ <target>
<node>1</node>
</target>
<address type='pci' bus='0x01'/>
diff --git a/tests/qemuxml2xmloutdata/pcie-expander-bus.xml
b/tests/qemuxml2xmloutdata/pcie-expander-bus.xml
index aaac423ca..d5e741b80 100644
--- a/tests/qemuxml2xmloutdata/pcie-expander-bus.xml
+++ b/tests/qemuxml2xmloutdata/pcie-expander-bus.xml
@@ -36,7 +36,7 @@
</controller>
<controller type='pci' index='2'
model='pcie-root-port'>
<model name='ioh3420'/>
- <target chassis='2' port='0x0' busNr='220'>
+ <target chassis='2' port='0x0'>
<node>1</node>
</target>
<address type='pci' domain='0x0000' bus='0x01'
slot='0x00' function='0x0'/>
--
2.14.3
10:28 a.m.
New subject: [libvirt] [PATCH v2 13/15] qemu: Validate PCI controllers (numaNode)
This change catches an invalid use of the option in our
test suite.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 39 ++++++++++++++++++++++++++
tests/qemuxml2argvdata/pcie-expander-bus.xml | 3 --
tests/qemuxml2xmloutdata/pcie-expander-bus.xml | 4 +--
3 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 81f347ce4..965a8384c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4519,6 +4519,45 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
break;
}
+ /* numaNode */
+ switch ((virDomainControllerModelPCI) controller->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+ /* numaNode can be used for these controllers, but it's not set
+ * automatically so it can be missing */
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ /* The pci-root controller only support numaNode for pSeries guests */
+ if (pciopts->numaNode != -1 &&
+ !qemuDomainIsPSeries(def)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Option '%s' is not valid for '%s'
controller "
+ "on '%s' architecture or '%s' machine
type"),
+ "numaNode", model,
+ virArchToString(def->os.arch), def->os.machine);
+ return -1;
+ }
+ 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_PCIE_ROOT:
+ if (pciopts->numaNode != -1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Option '%s' is not valid for '%s'
controller"),
+ "numaNode", model);
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
return 0;
}
diff --git a/tests/qemuxml2argvdata/pcie-expander-bus.xml
b/tests/qemuxml2argvdata/pcie-expander-bus.xml
index 237430a1e..5c5d34d1e 100644
--- a/tests/qemuxml2argvdata/pcie-expander-bus.xml
+++ b/tests/qemuxml2argvdata/pcie-expander-bus.xml
@@ -35,9 +35,6 @@
<address type='pci' bus='0x00' slot='4'/>
</controller>
<controller type='pci' index='2'
model='pcie-root-port'>
- <target>
- <node>1</node>
- </target>
<address type='pci' bus='0x01'/>
</controller>
<controller type='pci' index='3'
model='pcie-switch-upstream-port'>
diff --git a/tests/qemuxml2xmloutdata/pcie-expander-bus.xml
b/tests/qemuxml2xmloutdata/pcie-expander-bus.xml
index d5e741b80..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'>
- <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'>
--
2.14.3
10:28 a.m.
New subject: [libvirt] [PATCH v2 14/15] qemu: Validate PCI controllers (chassisNr)
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 965a8384c..99573dee9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4558,6 +4558,37 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
break;
}
+ /* chassisNr */
+ switch ((virDomainControllerModelPCI) controller->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ if (pciopts->chassisNr == -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Option '%s' not set for '%s'
controller"),
+ "chassisNr", model);
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_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:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ if (pciopts->chassisNr != -1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Option '%s' is not valid for '%s'
controller"),
+ "chassisNr", model);
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
return 0;
}
--
2.14.3
10:28 a.m.
New subject: [libvirt] [PATCH v2 15/15] qemu: Validate PCI controllers (chassis and port)
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 99573dee9..df725ed74 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4589,6 +4589,49 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
break;
}
+ /* chassis and port */
+ switch ((virDomainControllerModelPCI) controller->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ if (pciopts->chassis == -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Option '%s' not set for '%s'
controller"),
+ "chassis", model);
+ return -1;
+ }
+ if (pciopts->port == -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Option '%s' not set for '%s'
controller"),
+ "port", model);
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_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:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ if (pciopts->chassis != -1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Option '%s' is not valid for '%s'
controller"),
+ "chassis", model);
+ return -1;
+ }
+ if (pciopts->port != -1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Option '%s' is not valid for '%s'
controller"),
+ "port", model);
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
return 0;
}
--
2.14.3
2467
days inactive
2471
days old
38 comments
4 participants
participants (4)
-
Andrea Bolognani -
Daniel P. Berrangé -
John Ferlan -
Peter Krempa