From e5aecc2f800e8e14edd561dc5af4f763f040d842 Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Tue, 19 Apr 2016 17:05:54 -0400 Subject: [PATCH] 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 --- src/qemu/qemu_domain.c | 47 ++++++++++++++----- .../qemuxml2argv-440fx-wrong-root.xml | 28 +++++++++++ .../qemuxml2argv-q35-wrong-root.xml | 27 +++++++++++ tests/qemuxml2argvtest.c | 9 ++++ 4 files changed, 99 insertions(+), 12 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-440fx-wrong-root.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0cddb86b51..3da0079770 100644 --- 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 index 0000000000..93fef08214 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-440fx-wrong-root.xml @@ -0,0 +1,28 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + A description of the test machine. + + A test of qemu's minimal configuration. + This test also tests the description and title elements. + + 219100 + 219100 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu-kvm + + + + + + + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml new file mode 100644 index 0000000000..836de52a0c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml @@ -0,0 +1,27 @@ + + q35-test + 11dbdcdd-4c3b-482b-8903-9bdb8c0a2774 + 2097152 + 2097152 + 2 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu-kvm + + + + + + + + + + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e41444d2a4..d1cfbecb9a 100644 --- 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, -- GitLab