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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
