conf: log error when incorrect PCI root controller is added to domain

libvirt may automatically add a pci-root or pcie-root controller to a
domain, depending on the arch/machinetype, and it hopefully always
makes the right decision about which to add (since in all cases these
controllers are an implicit part of the virtual machine).

But it's always possible that someone will create a config that
explicitly supplies the wrong type of PCI controller for the selected
machinetype. In the past that would lead to an error later when
libvirt was trying to assign addresses to other devices, for example:

  XML error: PCI bus is not compatible with the device at
  0000:00:02.0. Device requires a PCI Express slot, which is not
  provided by bus 0000:00

(that's the error message that appears if you replace the pcie-root
controller in a Q35 domain with a pci-root controller).

This patch adds a check at the same place that the implicit
controllers are added (to ensure that the same logic is used to check
which type of pci root is correct). If a pci controller with index='0'
is already present, we verify that it is of the model that we would
have otherwise added automatically; if not, an error is logged:

  The PCI controller with index='0' must be " model='pcie-root' for
  this machine type, " but model='pci-root' was found instead.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1004602

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0cddb86b51d72b8241026aa15fdd77f0aa9adfcb..3da00797706bd6f16b8083b5928264d45f65f68f 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1742,6 +1742,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
 {
     bool addDefaultUSB = true;
     int usbModel = -1; /* "default for machinetype" */
+    int pciRoot;       /* index within def->controllers */
     bool addImplicitSATA = false;
     bool addPCIRoot = false;
     bool addPCIeRoot = false;
@@ -1833,14 +1834,26 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
             def, VIR_DOMAIN_CONTROLLER_TYPE_SATA, 0, -1) < 0)
         goto cleanup;
 
+    pciRoot = virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0);
+
     /* NB: any machine that sets addPCIRoot to true must also return
      * true from the function qemuDomainSupportsPCI().
      */
-    if (addPCIRoot &&
-        virDomainDefMaybeAddController(
-            def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
-            VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
-        goto cleanup;
+    if (addPCIRoot) {
+        if (pciRoot >= 0) {
+            if (def->controllers[pciRoot]->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) {
+                virReportError(VIR_ERR_XML_ERROR,
+                               _("The PCI controller with index='0' must be "
+                                 "model='pci-root' for this machine type, "
+                                 "but model='%s' was found instead"),
+                               virDomainControllerModelPCITypeToString(def->controllers[pciRoot]->model));
+                goto cleanup;
+            }
+        } else if (!virDomainDefAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
+                                              VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)) {
+            goto cleanup;
+        }
+    }
 
     /* When a machine has a pcie-root, make sure that there is always
      * a dmi-to-pci-bridge controller added as bus 1, and a pci-bridge
@@ -1850,15 +1863,25 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
      * true from the function qemuDomainSupportsPCI().
      */
     if (addPCIeRoot) {
+        if (pciRoot >= 0) {
+            if (def->controllers[pciRoot]->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) {
+                virReportError(VIR_ERR_XML_ERROR,
+                               _("The PCI controller with index='0' must be "
+                                 "model='pcie-root' for this machine type, "
+                                 "but model='%s' was found instead"),
+                               virDomainControllerModelPCITypeToString(def->controllers[pciRoot]->model));
+                goto cleanup;
+            }
+        } else if (!virDomainDefAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
+                                             VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)) {
+            goto cleanup;
+        }
         if (virDomainDefMaybeAddController(
-                def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
-                VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) < 0 ||
-            virDomainDefMaybeAddController(
-                def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1,
-                VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE) < 0 ||
+               def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1,
+               VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE) < 0 ||
             virDomainDefMaybeAddController(
-                def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 2,
-                VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE) < 0) {
+               def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 2,
+               VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE) < 0) {
             goto cleanup;
         }
     }

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-440fx-wrong-root.xml b/tests/qemuxml2argvdata/qemuxml2argv-440fx-wrong-root.xml
new file mode 100644 (file)
index 0000000..93fef08
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-440fx-wrong-root.xml
@@ -0,0 +1,28 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <title>A description of the test machine.</title>
+  <description>
+      A test of qemu&apos;s minimal configuration.
+      This test also tests the description and title elements.
+  </description>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-kvm</emulator>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml
new file mode 100644 (file)
index 0000000..836de52
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml
@@ -0,0 +1,27 @@
+<domain type='qemu'>
+  <name>q35-test</name>
+  <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid>
+  <memory unit='KiB'>2097152</memory>
+  <currentMemory unit='KiB'>2097152</currentMemory>
+  <vcpu placement='static' cpuset='0-1'>2</vcpu>
+  <os>
+    <type arch='x86_64' machine='q35'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-kvm</emulator>
+    <controller type='pci' index='0' model='pci-root'/>
+    <controller type='pci' index='1' model='dmi-to-pci-bridge'>
+      <model name='i82801b11-bridge'/>
+    </controller>
+    <controller type='pci' index='2' model='pci-bridge'>
+      <model name='pci-bridge'/>
+      <target chassisNr='56'/>
+    </controller>
+    <memballoon model='none'/>
+  </devices>
+</domain>

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index e41444d2a470e9b0839df667e56b7903d308e0ad..d1cfbecb9addfd57618c7ebc70910edfefff7d97 100644 (file)
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1554,6 +1554,15 @@ mymain(void)
             QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
             QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
 
+    DO_TEST_PARSE_ERROR("q35-wrong-root",
+            QEMU_CAPS_DEVICE_PCI_BRIDGE,
+            QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+            QEMU_CAPS_ICH9_AHCI,
+            QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1,
+            QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
+            QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
+    DO_TEST_PARSE_ERROR("440fx-wrong-root", NONE);
+
     DO_TEST_FAILURE("pcie-root-port-too-many",
             QEMU_CAPS_DEVICE_PCI_BRIDGE,
             QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,