On 06/12/2017 10:08 PM, Laine Stump wrote:
On 06/02/2017 12:07 PM, Andrea Bolognani wrote:
pSeries guests will soon be allowed to have multiple PHBs (pci-root controllers), which of course means that all but one of them will have a non-zero index; hence, we'll need to relax the current check.
However, right now the check is performed in the conf module, which is generic rather than tied to the QEMU driver, and where we don't have information such as the guest machine type available.
To make this change of behavior possible down the line, we need to move the check from the XML parser to the driver. Unfortunately, this means duplicating code :(\
Maybe instead we should just eliminate this check (since the pSeries case shows that it's an invalid assumption). And since the index is really just something used internally by libvirt, we really don't even need to verify that one of the pci controllers has index=0. All we *really* care about is that there is at least one "root" PCI bus, and that no device includes a bus# in its PCI address that isn't represented by the index in one of the PCI controllers.
(But for the purposes of this patch, I think we can just remove the validation in conf and not worry about adding it elsewhere).\\
After looking at the next patch, I may have changed my mind. If we remove the validation here, then we would still have to add in validation when the commandline is generated. But I suppose it's better to give an error at domain definition time rather than domain runtime, so this makes sense. I don't think all of those drivers actually use PCI controllers though, do they? (Look for which ones actually call the functions to assign PCI addresses).
--- src/bhyve/bhyve_domain.c | 15 +++++++++++++++ src/conf/domain_conf.c | 6 ------ src/libxl/libxl_domain.c | 14 ++++++++++++++ src/lxc/lxc_domain.c | 14 ++++++++++++++ src/openvz/openvz_driver.c | 14 ++++++++++++++ src/qemu/qemu_domain.c | 9 +++++++++ src/uml/uml_driver.c | 14 ++++++++++++++ src/vz/vz_driver.c | 14 ++++++++++++++ src/xen/xen_driver.c | 14 ++++++++++++++ 9 files changed, 108 insertions(+), 6 deletions(-)
diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 76b4fac..05c1508 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -122,6 +122,21 @@ bhyveDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, bhyveDomainDiskDefAssignAddress(driver, disk, def) < 0) return -1; } + + if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + virDomainControllerDefPtr cont = dev->data.controller; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + } + return 0; }
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c7e20b8..d5dff8e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9059,12 +9059,6 @@ virDomainControllerDefParseXML(xmlNodePtr node, "have an address")); goto error; } - if (def->idx > 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("pci-root and pcie-root controllers " - "should have index 0")); - goto error; - } if ((rc = virDomainParseScaledValue("./pcihole64", NULL, ctxt, &bytes, 1024, 1024ULL * ULONG_MAX, false)) < 0) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 256cf1d..59ef2fb 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -379,6 +379,20 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW); }
+ if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + virDomainControllerDefPtr cont = dev->data.controller; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + } + return 0; }
diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index 3a7404f..a50f6fe 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -389,6 +389,20 @@ virLXCDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE) dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC;
+ if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + virDomainControllerDefPtr cont = dev->data.controller; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + } + return 0; }
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 9c4a196..058d830 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -127,6 +127,20 @@ openvzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, return -1; }
+ if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + virDomainControllerDefPtr cont = dev->data.controller; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + } + return 0; }
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0a85ee9..18512cb 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3404,6 +3404,15 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, break;
case VIR_DOMAIN_CONTROLLER_TYPE_PCI: + if ((cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS && !qemuDomainIsI440FX(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 03edc89..d9df3c5 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -426,6 +426,20 @@ umlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, return -1; }
+ if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + virDomainControllerDefPtr cont = dev->data.controller; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + } + return 0; }
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index ef7b453..e988e7c 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -283,6 +283,20 @@ vzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, VIR_STRDUP(dev->data.net->model, "e1000") < 0) return -1;
+ if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + virDomainControllerDefPtr cont = dev->data.controller; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + } + return 0; }
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 4d14089..b681764 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -362,6 +362,20 @@ xenDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, } }
+ if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + virDomainControllerDefPtr cont = dev->data.controller; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + } + return 0; }
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list