Changeset
src/qemu/qemu_domain.c                         | 531 +++++++++++++++++--------
tests/qemuxml2argvdata/pcie-expander-bus.xml   |   3 -
tests/qemuxml2argvtest.c                       |  38 +-
tests/qemuxml2xmloutdata/pcie-expander-bus.xml |   4 +-
tests/qemuxml2xmltest.c                        |  31 +-
5 files changed, 432 insertions(+), 175 deletions(-)
Git apply log
Switched to a new branch '20180308133433.12361-1-abologna@redhat.com'
Applying: qemu: Create new qemuDomainDeviceDefValidateControllerPCI()
Applying: qemu: Validate PCI controller options (modelName)
Applying: qemu: Validate PCI controller options (index)
Applying: qemu: Validate PCI controller options (targetIndex)
Applying: qemu: Validate PCI controller options (pcihole64)
Applying: qemu: Validate PCI controller options (busNr)
Applying: qemu: Validate PCI controller options (numaNode)
Applying: qemu: Validate PCI controller options (chassisNr)
Applying: qemu: Validate PCI controller options (chassis and port)
Applying: qemu: Validate PCI controllers (QEMU capabilities)
Applying: qemu: Remove old qemuDomainDeviceDefValidateControllerPCI()
To https://github.com/patchew-project/libvirt
 + 0de9cd46d...db6510de4 patchew/20180308133433.12361-1-abologna@redhat.com -> patchew/20180308133433.12361-1-abologna@redhat.com (forced update)
Test passed: syntax-check

loading

[libvirt] [PATCH v7 00/11] qemu: Validate PCI controller options
Posted by Andrea Bolognani, 15 weeks ago
Applies cleanly on top of b932ed69f6664f42e211bdde84c8ab04e1f19033.

Patches 2/11 and 4/11 are the only ones missing R-bs, everything
else hasn't been significantly altered since [v5].

Changes from [v6]:

  * don't special-case the default PHB when checking capabilities.

Changes from [v5]:

  * patch 1 has been pushed;
  * add virReportController*() functions to cut down on redundant
    error reporting, as suggested by laine;
  * report index and modelName along with model in error messages.

Changes from [v4]:

  * patch 1/12 is new;
  * use virReportEnumRangeError(), as suggested by laine.

Changes from [v3]:

  * don't introduce new test cases that won't be able to provide
    full test coverage anyway, as suggested by laine.

Changes from [v2]:

  * replace the old implementation bit by bit using a clever trick
    suggested by pkrempa;
  * don't move QEMU capability validation;
  * add a default: label to all switch statements as recommended
    by danpb.

Changes from [v1]:

  * error out instead of silently accept invalid options;
  * shave quite a lot of yaks.

[v6] https://www.redhat.com/archives/libvir-list/2018-March/msg00203.html
[v5] https://www.redhat.com/archives/libvir-list/2018-March/msg00096.html
[v4] https://www.redhat.com/archives/libvir-list/2018-February/msg01232.html
[v3] https://www.redhat.com/archives/libvir-list/2018-February/msg00996.html
[v2] https://www.redhat.com/archives/libvir-list/2018-February/msg00813.html
[v1] https://www.redhat.com/archives/libvir-list/2018-February/msg00244.html

Andrea Bolognani (11):
  qemu: Create new qemuDomainDeviceDefValidateControllerPCI()
  qemu: Validate PCI controller options (modelName)
  qemu: Validate PCI controller options (index)
  qemu: Validate PCI controller options (targetIndex)
  qemu: Validate PCI controller options (pcihole64)
  qemu: Validate PCI controller options (busNr)
  qemu: Validate PCI controller options (numaNode)
  qemu: Validate PCI controller options (chassisNr)
  qemu: Validate PCI controller options (chassis and port)
  qemu: Validate PCI controllers (QEMU capabilities)
  qemu: Remove old qemuDomainDeviceDefValidateControllerPCI()

 src/qemu/qemu_domain.c                         | 531 +++++++++++++++++--------
 tests/qemuxml2argvdata/pcie-expander-bus.xml   |   3 -
 tests/qemuxml2argvtest.c                       |  38 +-
 tests/qemuxml2xmloutdata/pcie-expander-bus.xml |   4 +-
 tests/qemuxml2xmltest.c                        |  31 +-
 5 files changed, 432 insertions(+), 175 deletions(-)

-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v7 00/11] qemu: Validate PCI controller options
Posted by Laine Stump, 15 weeks ago
On 03/08/2018 08:34 AM, Andrea Bolognani wrote:
> Applies cleanly on top of b932ed69f6664f42e211bdde84c8ab04e1f19033.
>
> Patches 2/11 and 4/11 are the only ones missing R-bs, everything
> else hasn't been significantly altered since [v5].

This all looks good now. I replied to 2 & 4 with R-b's. Sorry I didn't
see this yesterday (was busy at the doctor all morning), so it's all
ACKed and pushable!

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 01/11] qemu: Create new qemuDomainDeviceDefValidateControllerPCI()
Posted by Andrea Bolognani, 15 weeks ago
The existing function is renamed and called from the new one, so
that even while we're in the process of implementing new checks
all the existing ones will be performed.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
---
 src/qemu/qemu_domain.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ee02ecd0cd..f1139cbac3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4268,9 +4268,9 @@ qemuDomainDeviceDefValidateControllerSCSI(const virDomainControllerDef *controll
 
 
 static int
-qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller,
-                                         const virDomainDef *def,
-                                         virQEMUCapsPtr qemuCaps)
+qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *controller,
+                                            const virDomainDef *def,
+                                            virQEMUCapsPtr qemuCaps)
 {
     virDomainControllerModelPCI model = controller->model;
     const virDomainPCIControllerOpts *pciopts;
@@ -4547,6 +4547,29 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle
 }
 
 
+static int
+qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
+                                         const virDomainDef *def,
+                                         virQEMUCapsPtr qemuCaps)
+
+{
+    const virDomainPCIControllerOpts *pciopts = &cont->opts.pciopts;
+    const char *model = virDomainControllerModelPCITypeToString(cont->model);
+    const char *modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
+
+    if (!model) {
+        virReportEnumRangeError(virDomainControllerModelPCI, cont->model);
+        return -1;
+    }
+    if (!modelName) {
+        virReportEnumRangeError(virDomainControllerPCIModelName, pciopts->modelName);
+        return -1;
+    }
+
+    return qemuDomainDeviceDefValidateControllerPCIOld(cont, def, qemuCaps);
+}
+
+
 static int
 qemuDomainDeviceDefValidateControllerSATA(const virDomainControllerDef *controller,
                                           const virDomainDef *def,
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 02/11] qemu: Validate PCI controller options (modelName)
Posted by Andrea Bolognani, 15 weeks ago
https://bugzilla.redhat.com/show_bug.cgi?id=1483816

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/qemu/qemu_domain.c | 226 +++++++++++++++++++++++++++++++------------------
 1 file changed, 142 insertions(+), 84 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f1139cbac3..d8669ee9b3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4274,7 +4274,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
 {
     virDomainControllerModelPCI model = controller->model;
     const virDomainPCIControllerOpts *pciopts;
-    const char *modelName = NULL;
 
     /* skip pcie-root */
     if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
@@ -4312,24 +4311,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
     }
 
     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) {
@@ -4340,14 +4321,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
             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;
-        }
-
         if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("the pci-bridge controller is not supported "
@@ -4364,14 +4337,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
             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;
-        }
-
         if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("the pxb controller is not supported in this "
@@ -4382,14 +4347,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
         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;
-        }
-
         if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("the dmi-to-pci-bridge (i82801b11-bridge) "
@@ -4406,15 +4363,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
             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;
-        }
-
         if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) &&
             !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -4434,14 +4382,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
         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;
-        }
-
         if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("the pcie-switch-upstream-port (x3130-upstream) "
@@ -4459,14 +4399,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
             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;
-        }
-
         if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("The pcie-switch-downstream-port "
@@ -4484,14 +4416,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
             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;
-        }
-
         if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB_PCIE)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("the pxb-pcie controller is not supported "
@@ -4512,14 +4436,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
         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;
-        }
-
         if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("the spapr-pci-host-bridge controller is not "
@@ -4547,6 +4463,23 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
 }
 
 
+#define virReportControllerMissingOption(cont, model, modelName, option) \
+    virReportError(VIR_ERR_INTERNAL_ERROR, \
+                   _("Required option '%s' is not set for PCI controller " \
+                     "with index '%d', model '%s' and modelName '%s'"), \
+                   (option), (cont->idx), (model), (modelName));
+#define virReportControllerInvalidOption(cont, model, modelName, option) \
+    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
+                   _("Option '%s' is not valid for PCI controller " \
+                     "with index '%d', model '%s' and modelName '%s'"), \
+                   (option), (cont->idx), (model), (modelName));
+#define virReportControllerInvalidValue(cont, model, modelName, option) \
+    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
+                   _("Option '%s' has invalid value for PCI controller " \
+                     "with index '%d', model '%s' and modelName '%s'"), \
+                   (option), (cont->idx), (model), (modelName));
+
+
 static int
 qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
                                          const virDomainDef *def,
@@ -4566,10 +4499,135 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
         return -1;
     }
 
+    /* modelName */
+    switch ((virDomainControllerModelPCI) cont->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:
+        /* modelName should have been set automatically */
+        if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
+            virReportControllerMissingOption(cont, model, modelName, "modelName");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+        /* modelName must be set for pSeries guests, but it's an error
+         * for it to be set for any other guest */
+        if (qemuDomainIsPSeries(def)) {
+            if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
+                virReportControllerMissingOption(cont, model, modelName, "modelName");
+                return -1;
+            }
+        } else {
+            if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
+                virReportControllerInvalidOption(cont, model, modelName, "modelName");
+                return -1;
+            }
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+        if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
+            virReportControllerInvalidOption(cont, model, modelName, "modelName");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+    default:
+        virReportEnumRangeError(virDomainControllerModelPCI, cont->model);
+        return -1;
+    }
+
+    /* modelName (cont'd) */
+    switch ((virDomainControllerModelPCI) cont->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) {
+            virReportControllerInvalidValue(cont, model, modelName, "modelName");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+        if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE) {
+            virReportControllerInvalidValue(cont, model, modelName, "modelName");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+        if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE) {
+            virReportControllerInvalidValue(cont, model, modelName, "modelName");
+            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) {
+            virReportControllerInvalidValue(cont, model, modelName, "modelName");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+        if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM) {
+            virReportControllerInvalidValue(cont, model, modelName, "modelName");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+        if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM) {
+            virReportControllerInvalidValue(cont, model, modelName, "modelName");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+        if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB) {
+            virReportControllerInvalidValue(cont, model, modelName, "modelName");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+        if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE) {
+            virReportControllerInvalidValue(cont, model, modelName, "modelName");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+        if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
+            virReportControllerInvalidValue(cont, model, modelName, "modelName");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+    default:
+        virReportEnumRangeError(virDomainControllerModelPCI, cont->model);
+        return -1;
+    }
+
     return qemuDomainDeviceDefValidateControllerPCIOld(cont, def, qemuCaps);
 }
 
 
+#undef virReportControllerInvalidValue
+#undef virReportControllerInvalidOption
+#undef virReportControllerMissingOption
+
+
 static int
 qemuDomainDeviceDefValidateControllerSATA(const virDomainControllerDef *controller,
                                           const virDomainDef *def,
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v7 02/11] qemu: Validate PCI controller options (modelName)
Posted by Laine Stump, 15 weeks ago
On 03/08/2018 08:34 AM, Andrea Bolognani wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1483816
>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  src/qemu/qemu_domain.c | 226 +++++++++++++++++++++++++++++++------------------
>  1 file changed, 142 insertions(+), 84 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index f1139cbac3..d8669ee9b3 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4274,7 +4274,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
>  {
>      virDomainControllerModelPCI model = controller->model;
>      const virDomainPCIControllerOpts *pciopts;
> [...]
>  
>  
> +#define virReportControllerMissingOption(cont, model, modelName, option) \
> +    virReportError(VIR_ERR_INTERNAL_ERROR, \
> +                   _("Required option '%s' is not set for PCI controller " \
> +                     "with index '%d', model '%s' and modelName '%s'"), \
> +                   (option), (cont->idx), (model), (modelName));
> +#define virReportControllerInvalidOption(cont, model, modelName, option) \
> +    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
> +                   _("Option '%s' is not valid for PCI controller " \
> +                     "with index '%d', model '%s' and modelName '%s'"), \
> +                   (option), (cont->idx), (model), (modelName));
> +#define virReportControllerInvalidValue(cont, model, modelName, option) \
> +    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
> +                   _("Option '%s' has invalid value for PCI controller " \
> +                     "with index '%d', model '%s' and modelName '%s'"), \
> +                   (option), (cont->idx), (model), (modelName));
> +
> +
>  static int
>  qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
>                                           const virDomainDef *def,
> @@ -4566,10 +4499,135 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
>          return -1;
>      }
>  
> +    /* modelName */
> +    switch ((virDomainControllerModelPCI) cont->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:
> +        /* modelName should have been set automatically */
> +        if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
> +            virReportControllerMissingOption(cont, model, modelName, "modelName");

"Required option 'modelName' is not set for PCI controller with index
'1', model 'pcie-root-port', modelName 'none'."

Yep, looks good to me.

> +            return -1;
> +        }
> +        break;
> +
> +    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
> +        /* modelName must be set for pSeries guests, but it's an error
> +         * for it to be set for any other guest */
> +        if (qemuDomainIsPSeries(def)) {
> +            if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
> +                virReportControllerMissingOption(cont, model, modelName, "modelName");
> +                return -1;
> +            }
> +        } else {
> +            if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
> +                virReportControllerInvalidOption(cont, model, modelName, "modelName");
> +                return -1;

...and since we've already errored out on modelName values outside the
accepted range, this is okay too.

> [...]

Reviewed-by: Laine Stump <laine@laine.org>


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 03/11] qemu: Validate PCI controller options (index)
Posted by Andrea Bolognani, 15 weeks ago
https://bugzilla.redhat.com/show_bug.cgi?id=1483816

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
---
 src/qemu/qemu_domain.c | 68 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 43 insertions(+), 25 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d8669ee9b3..5e028653d0 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4285,31 +4285,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
         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_DEFAULT:
-    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
-        break;
-    }
-
     pciopts = &controller->opts.pciopts;
 
     /* Second pass - now the model specific checks */
@@ -4619,6 +4594,49 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
         return -1;
     }
 
+    /* index */
+    switch ((virDomainControllerModelPCI) cont->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 (cont->idx == 0) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Index for '%s' controllers must be > 0"),
+                           model);
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+        /* pSeries guests can have multiple PHBs, so it's expected that
+         * the index will not be zero for some of them */
+        if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT &&
+            pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) {
+            break;
+        }
+
+        /* For all other pci-root and pcie-root controllers, though,
+         * the index must be zero*/
+        if (cont->idx != 0) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Index for '%s' controllers must be 0"),
+                           model);
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+    default:
+        virReportEnumRangeError(virDomainControllerModelPCI, cont->model);
+        return -1;
+    }
+
     return qemuDomainDeviceDefValidateControllerPCIOld(cont, def, qemuCaps);
 }
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 04/11] qemu: Validate PCI controller options (targetIndex)
Posted by Andrea Bolognani, 15 weeks ago
https://bugzilla.redhat.com/show_bug.cgi?id=1483816

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/qemu/qemu_domain.c | 46 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 5e028653d0..d67ef53043 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4401,12 +4401,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
         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;
@@ -4637,6 +4631,46 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
         return -1;
     }
 
+    /* targetIndex */
+    switch ((virDomainControllerModelPCI) cont->model) {
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+        /* PHBs for pSeries guests must have been assigned a targetIndex */
+        if (pciopts->targetIndex == -1 &&
+            pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) {
+            virReportControllerMissingOption(cont, model, modelName, "targetIndex");
+            return -1;
+        }
+
+        /* targetIndex only applies to PHBs, so for any other pci-root
+         * controller it being present is an error */
+        if (pciopts->targetIndex != -1 &&
+            pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) {
+            virReportControllerInvalidOption(cont, model, modelName, "targetIndex");
+            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) {
+            virReportControllerInvalidOption(cont, model, modelName, "targetIndex");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+    default:
+        virReportEnumRangeError(virDomainControllerModelPCI, cont->model);
+        return -1;
+    }
+
     return qemuDomainDeviceDefValidateControllerPCIOld(cont, def, qemuCaps);
 }
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v7 04/11] qemu: Validate PCI controller options (targetIndex)
Posted by Laine Stump, 15 weeks ago
On 03/08/2018 08:34 AM, Andrea Bolognani wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1483816
>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>

Reviewed-by: Laine Stump <laine@laine.org>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 05/11] qemu: Validate PCI controller options (pcihole64)
Posted by Andrea Bolognani, 15 weeks ago
https://bugzilla.redhat.com/show_bug.cgi?id=1483816

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
---
 src/qemu/qemu_domain.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d67ef53043..ab2420ea9c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4671,6 +4671,40 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
         return -1;
     }
 
+    /* pcihole64 */
+    switch ((virDomainControllerModelPCI) cont->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)) {
+            virReportControllerInvalidOption(cont, model, modelName, "pcihole64");
+            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) {
+            virReportControllerInvalidOption(cont, model, modelName, "pcihole64");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+    default:
+        virReportEnumRangeError(virDomainControllerModelPCI, cont->model);
+        return -1;
+    }
+
     return qemuDomainDeviceDefValidateControllerPCIOld(cont, def, qemuCaps);
 }
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 06/11] qemu: Validate PCI controller options (busNr)
Posted by Andrea Bolognani, 15 weeks ago
This change catches an invalid use of the option in our
test suite.

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

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
---
 src/qemu/qemu_domain.c                         | 42 ++++++++++++++++++--------
 tests/qemuxml2argvdata/pcie-expander-bus.xml   |  2 +-
 tests/qemuxml2xmloutdata/pcie-expander-bus.xml |  2 +-
 3 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ab2420ea9c..cc7236fbdb 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4306,12 +4306,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
         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 (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("the pxb controller is not supported in this "
@@ -4385,12 +4379,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
         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 (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB_PCIE)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("the pxb-pcie controller is not supported "
@@ -4705,6 +4693,36 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
         return -1;
     }
 
+    /* busNr */
+    switch ((virDomainControllerModelPCI) cont->model) {
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+        if (pciopts->busNr == -1) {
+            virReportControllerMissingOption(cont, model, modelName, "busNr");
+            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) {
+            virReportControllerInvalidOption(cont, model, modelName, "busNr");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+    default:
+        virReportEnumRangeError(virDomainControllerModelPCI, cont->model);
+        return -1;
+    }
+
     return qemuDomainDeviceDefValidateControllerPCIOld(cont, def, qemuCaps);
 }
 
diff --git a/tests/qemuxml2argvdata/pcie-expander-bus.xml b/tests/qemuxml2argvdata/pcie-expander-bus.xml
index ac01c26ccf..237430a1e5 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 aaac423cac..d5e741b80d 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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 07/11] qemu: Validate PCI controller options (numaNode)
Posted by Andrea Bolognani, 15 weeks ago
This change catches an invalid use of the option in our
test suite.

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

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
---
 src/qemu/qemu_domain.c                         | 44 ++++++++++++++++++++++++++
 tests/qemuxml2argvdata/pcie-expander-bus.xml   |  3 --
 tests/qemuxml2xmloutdata/pcie-expander-bus.xml |  4 +--
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index cc7236fbdb..8264f2bb3f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4723,6 +4723,50 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
         return -1;
     }
 
+    /* numaNode */
+    switch ((virDomainControllerModelPCI) cont->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:
+        /* Only PHBs support numaNode */
+        if (pciopts->numaNode != -1 &&
+            pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) {
+            virReportControllerInvalidOption(cont, model, modelName, "numaNode");
+            return -1;
+        }
+
+        /* However, the default PHB doesn't support numaNode */
+        if (pciopts->numaNode != -1 &&
+            pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE &&
+            pciopts->targetIndex == 0) {
+            virReportControllerInvalidOption(cont, model, modelName, "numaNode");
+            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) {
+            virReportControllerInvalidOption(cont, model, modelName, "numaNode");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+    default:
+        virReportEnumRangeError(virDomainControllerModelPCI, cont->model);
+        return -1;
+    }
+
     return qemuDomainDeviceDefValidateControllerPCIOld(cont, def, qemuCaps);
 }
 
diff --git a/tests/qemuxml2argvdata/pcie-expander-bus.xml b/tests/qemuxml2argvdata/pcie-expander-bus.xml
index 237430a1e5..5c5d34d1e0 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 d5e741b80d..b6498fd2eb 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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 08/11] qemu: Validate PCI controller options (chassisNr)
Posted by Andrea Bolognani, 15 weeks ago
https://bugzilla.redhat.com/show_bug.cgi?id=1483816

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
---
 src/qemu/qemu_domain.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8264f2bb3f..a8b94f6a4a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4290,12 +4290,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
     /* 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 (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("the pci-bridge controller is not supported "
@@ -4767,6 +4761,36 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
         return -1;
     }
 
+    /* chassisNr */
+    switch ((virDomainControllerModelPCI) cont->model) {
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+        if (pciopts->chassisNr == -1) {
+            virReportControllerMissingOption(cont, model, modelName, "chassisNr");
+            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) {
+            virReportControllerInvalidOption(cont, model, modelName, "chassisNr");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+    default:
+        virReportEnumRangeError(virDomainControllerModelPCI, cont->model);
+        return -1;
+    }
+
     return qemuDomainDeviceDefValidateControllerPCIOld(cont, def, qemuCaps);
 }
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 09/11] qemu: Validate PCI controller options (chassis and port)
Posted by Andrea Bolognani, 15 weeks ago
https://bugzilla.redhat.com/show_bug.cgi?id=1483816

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
---
 src/qemu/qemu_domain.c | 50 +++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a8b94f6a4a..1f09b65e52 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4320,12 +4320,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
         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) &&
             !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -4355,13 +4349,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
         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 (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("The pcie-switch-downstream-port "
@@ -4791,6 +4778,43 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
         return -1;
     }
 
+    /* chassis and port */
+    switch ((virDomainControllerModelPCI) cont->model) {
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+        if (pciopts->chassis == -1) {
+            virReportControllerMissingOption(cont, model, modelName, "chassis");
+            return -1;
+        }
+        if (pciopts->port == -1) {
+            virReportControllerMissingOption(cont, model, modelName, "port");
+            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) {
+            virReportControllerInvalidOption(cont, model, modelName, "chassis");
+            return -1;
+        }
+        if (pciopts->port != -1) {
+            virReportControllerInvalidOption(cont, model, modelName, "port");
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+    default:
+        virReportEnumRangeError(virDomainControllerModelPCI, cont->model);
+    }
+
     return qemuDomainDeviceDefValidateControllerPCIOld(cont, def, qemuCaps);
 }
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 10/11] qemu: Validate PCI controllers (QEMU capabilities)
Posted by Andrea Bolognani, 15 weeks ago
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
---
 src/qemu/qemu_domain.c   | 181 ++++++++++++++++++-----------------------------
 tests/qemuxml2argvtest.c |  38 +++++++++-
 tests/qemuxml2xmltest.c  |  31 ++++++--
 3 files changed, 127 insertions(+), 123 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1f09b65e52..d18205c525 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4270,11 +4270,8 @@ qemuDomainDeviceDefValidateControllerSCSI(const virDomainControllerDef *controll
 static int
 qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *controller,
                                             const virDomainDef *def,
-                                            virQEMUCapsPtr qemuCaps)
+                                            virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED)
 {
-    virDomainControllerModelPCI model = controller->model;
-    const virDomainPCIControllerOpts *pciopts;
-
     /* skip pcie-root */
     if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
         return 0;
@@ -4285,119 +4282,50 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
         controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
         return 0;
 
-    pciopts = &controller->opts.pciopts;
-
-    /* Second pass - now the model specific checks */
-    switch (model) {
-    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
-        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:
-        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:
-        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:
-        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:
-        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:
-        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:
-        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:
-        /* Skip the implicit one */
-        if (pciopts->targetIndex == 0)
-            return 0;
-
-        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;
-        }
+    return 0;
+}
 
-        break;
 
-    case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
-    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
-    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
-        break;
+/**
+ * virDomainControllerPCIModelNameToQEMUCaps:
+ * @modelName: model name
+ *
+ * Maps model names for PCI controllers (virDomainControllerPCIModelName)
+ * to the QEMU capabilities required to use them (virQEMUCapsFlags).
+ *
+ * Returns: the QEMU capability itself (>0) on success; 0 if no QEMU
+ *          capability is needed; <0 on error.
+ */
+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:
+        return 0;
+    case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST:
+    default:
+        return -1;
     }
 
-    return 0;
+    return -1;
 }
 
 
@@ -4427,6 +4355,7 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
     const virDomainPCIControllerOpts *pciopts = &cont->opts.pciopts;
     const char *model = virDomainControllerModelPCITypeToString(cont->model);
     const char *modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
+    int cap = virDomainControllerPCIModelNameToQEMUCaps(pciopts->modelName);
 
     if (!model) {
         virReportEnumRangeError(virDomainControllerModelPCI, cont->model);
@@ -4815,6 +4744,32 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
         virReportEnumRangeError(virDomainControllerModelPCI, cont->model);
     }
 
+    /* QEMU device availability */
+    if (cap < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unknown QEMU device for '%s' controller"),
+                       modelName);
+        return -1;
+    }
+    if (cap > 0 && !virQEMUCapsGet(qemuCaps, cap)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("The '%s' device is not supported by this QEMU binary"),
+                       modelName);
+        return -1;
+    }
+
+    /* PHBs didn't support numaNode from the very beginning, so an extra
+     * capability check is required */
+    if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT &&
+        pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE &&
+        pciopts->numaNode != -1 &&
+        !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Option '%s' is not supported by '%s' device with this QEMU binary"),
+                       "numaNode", modelName);
+        return -1;
+    }
+
     return qemuDomainDeviceDefValidateControllerPCIOld(cont, def, qemuCaps);
 }
 
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 688846b9b4..74f51ac325 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -882,7 +882,8 @@ mymain(void)
             QEMU_CAPS_VIRTIO_TX_ALG);
     DO_TEST("disk-cdrom-tray-no-device-cap", NONE);
     DO_TEST("disk-floppy", NONE);
-    DO_TEST_FAILURE("disk-floppy-pseries", NONE);
+    DO_TEST_FAILURE("disk-floppy-pseries",
+                    QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
     DO_TEST("disk-floppy-tray-no-device-cap", NONE);
     DO_TEST("disk-floppy-tray", NONE);
     DO_TEST("disk-virtio-s390",
@@ -1784,34 +1785,43 @@ mymain(void)
     DO_TEST_PARSE_ERROR("seclabel-device-duplicates", NONE);
 
     DO_TEST("pseries-basic",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY,
             QEMU_CAPS_NODEFCONFIG);
     DO_TEST("pseries-vio",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY,
             QEMU_CAPS_NODEFCONFIG);
     DO_TEST("pseries-usb-default",
             QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY,
             QEMU_CAPS_PIIX3_USB_UHCI,
             QEMU_CAPS_PCI_OHCI,
             QEMU_CAPS_PCI_MULTIFUNCTION);
     DO_TEST("pseries-usb-multi",
             QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY,
             QEMU_CAPS_PIIX3_USB_UHCI,
             QEMU_CAPS_PCI_OHCI,
             QEMU_CAPS_PCI_MULTIFUNCTION);
     DO_TEST("pseries-vio-user-assigned",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY,
             QEMU_CAPS_NODEFCONFIG);
     DO_TEST_PARSE_ERROR("pseries-vio-address-clash",
             QEMU_CAPS_NODEFCONFIG);
-    DO_TEST("pseries-nvram", QEMU_CAPS_DEVICE_NVRAM);
+    DO_TEST("pseries-nvram",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
+            QEMU_CAPS_DEVICE_NVRAM);
     DO_TEST("pseries-usb-kbd", QEMU_CAPS_PCI_OHCI,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_USB_KBD,
             QEMU_CAPS_DEVICE_SPAPR_VTY,
             QEMU_CAPS_NODEFCONFIG);
     DO_TEST("pseries-cpu-exact",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY,
             QEMU_CAPS_NODEFCONFIG);
     DO_TEST_PARSE_ERROR("pseries-no-parallel",
@@ -1819,21 +1829,27 @@ mymain(void)
 
     qemuTestSetHostArch(driver.caps, VIR_ARCH_PPC64);
     DO_TEST("pseries-cpu-compat", QEMU_CAPS_KVM,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY,
             QEMU_CAPS_NODEFCONFIG);
     DO_TEST("pseries-machine-max-cpu-compat",
             QEMU_CAPS_KVM,
             QEMU_CAPS_MACHINE_OPT,
             QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_NODEFCONFIG);
     DO_TEST("pseries-cpu-le", QEMU_CAPS_KVM,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY,
             QEMU_CAPS_NODEFCONFIG);
-    DO_TEST_FAILURE("pseries-cpu-compat-power9", QEMU_CAPS_KVM);
+    DO_TEST_FAILURE("pseries-cpu-compat-power9",
+                    QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
+                    QEMU_CAPS_KVM);
 
     qemuTestSetHostCPU(driver.caps, cpuPower9);
     DO_TEST("pseries-cpu-compat-power9",
             QEMU_CAPS_KVM,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY,
             QEMU_CAPS_NODEFCONFIG);
     qemuTestSetHostCPU(driver.caps, NULL);
@@ -1841,12 +1857,15 @@ mymain(void)
     qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE);
 
     DO_TEST("pseries-panic-missing",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY,
             QEMU_CAPS_NODEFCONFIG);
     DO_TEST("pseries-panic-no-address",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY,
             QEMU_CAPS_NODEFCONFIG);
     DO_TEST_FAILURE("pseries-panic-address",
+                    QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
                     QEMU_CAPS_NODEFCONFIG);
 
     DO_TEST("pseries-phb-simple",
@@ -1897,32 +1916,41 @@ mymain(void)
             QEMU_CAPS_DEVICE_VFIO_PCI);
 
     DO_TEST("pseries-features-hpt",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_MACHINE_OPT,
             QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT);
     DO_TEST_FAILURE("pseries-features-hpt",
+                    QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
                     QEMU_CAPS_MACHINE_OPT);
     DO_TEST_PARSE_ERROR("pseries-features-invalid-machine", NONE);
 
     DO_TEST("pseries-serial-native",
             QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY);
     DO_TEST("pseries-serial+console-native",
             QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY);
     DO_TEST("pseries-serial-compat",
             QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY);
     DO_TEST("pseries-serial-pci",
             QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_PCI_SERIAL);
     DO_TEST("pseries-serial-usb",
             QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_QEMU_XHCI,
             QEMU_CAPS_DEVICE_USB_SERIAL);
     DO_TEST("pseries-console-native",
             QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY);
     DO_TEST("pseries-console-virtio",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_NODEFCONFIG);
     DO_TEST_PARSE_ERROR("pseries-serial-invalid-machine", NONE);
 
@@ -2723,6 +2751,7 @@ mymain(void)
     DO_TEST("memory-hotplug-dimm-addr", QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_NUMA,
             QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
     DO_TEST("memory-hotplug-ppc64-nonuma", QEMU_CAPS_KVM, QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_NUMA,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
     DO_TEST("memory-hotplug-nvdimm", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_DEVICE_NVDIMM,
             QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
@@ -2843,11 +2872,14 @@ mymain(void)
     DO_TEST("virtio-input-passthrough", QEMU_CAPS_VIRTIO_INPUT_HOST);
 
     DO_TEST("ppc64-usb-controller",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_PCI_OHCI);
     DO_TEST("ppc64-usb-controller-legacy",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_PIIX3_USB_UHCI);
     DO_TEST_FULL("ppc64-usb-controller-qemu-xhci", NULL, -1, 0,
                  VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, GIC_NONE,
+                 QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
                  QEMU_CAPS_NEC_USB_XHCI,
                  QEMU_CAPS_DEVICE_QEMU_XHCI);
 
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 0eb9e6c77a..5306b819a3 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -674,8 +674,10 @@ mymain(void)
             QEMU_CAPS_PIIX3_USB_UHCI,
             QEMU_CAPS_NEC_USB_XHCI);
     DO_TEST("ppc64-usb-controller",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_PCI_OHCI);
     DO_TEST("ppc64-usb-controller-legacy",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_PIIX3_USB_UHCI);
     DO_TEST("usb-port-missing", NONE);
     DO_TEST("usb-redir", NONE);
@@ -717,9 +719,12 @@ mymain(void)
     DO_TEST("virtio-rng-random", NONE);
     DO_TEST("virtio-rng-egd", NONE);
 
-    DO_TEST("pseries-nvram", NONE);
-    DO_TEST("pseries-panic-missing", NONE);
-    DO_TEST("pseries-panic-no-address", NONE);
+    DO_TEST("pseries-nvram",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
+    DO_TEST("pseries-panic-missing",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
+    DO_TEST("pseries-panic-no-address",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
 
     DO_TEST("pseries-phb-simple",
             QEMU_CAPS_NODEFCONFIG,
@@ -764,29 +769,37 @@ mymain(void)
             QEMU_CAPS_DEVICE_VFIO_PCI);
 
     DO_TEST("pseries-features-hpt",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_MACHINE_OPT,
             QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT);
 
     DO_TEST("pseries-serial-native",
             QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY);
     DO_TEST("pseries-serial+console-native",
             QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY);
     DO_TEST("pseries-serial-compat",
             QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY);
     DO_TEST("pseries-serial-pci",
             QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_PCI_SERIAL);
     DO_TEST("pseries-serial-usb",
             QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_QEMU_XHCI,
             QEMU_CAPS_DEVICE_USB_SERIAL);
     DO_TEST("pseries-console-native",
             QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY);
     DO_TEST("pseries-console-virtio",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_NODEFCONFIG);
 
     DO_TEST("mach-virt-serial-native",
@@ -1182,7 +1195,8 @@ mymain(void)
 
     DO_TEST("panic", NONE);
     DO_TEST("panic-isa", NONE);
-    DO_TEST("panic-pseries", NONE);
+    DO_TEST("panic-pseries",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
     DO_TEST("panic-double", NONE);
     DO_TEST("panic-no-address", NONE);
 
@@ -1338,9 +1352,12 @@ mymain(void)
     DO_TEST("smartcard-passthrough-spicevmc", NONE);
     DO_TEST("smartcard-controller", NONE);
 
-    DO_TEST("pseries-cpu-compat-power9", NONE);
-    DO_TEST("pseries-cpu-compat", NONE);
-    DO_TEST("pseries-cpu-exact", NONE);
+    DO_TEST("pseries-cpu-compat-power9",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
+    DO_TEST("pseries-cpu-compat",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
+    DO_TEST("pseries-cpu-exact",
+            QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
 
     DO_TEST("user-aliases", NONE);
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 11/11] qemu: Remove old qemuDomainDeviceDefValidateControllerPCI()
Posted by Andrea Bolognani, 15 weeks ago
We've implemented all existing checks, and more, in the new
function, so we can finally drop the old one.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
---
 src/qemu/qemu_domain.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d18205c525..c1359c0c83 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4267,25 +4267,6 @@ qemuDomainDeviceDefValidateControllerSCSI(const virDomainControllerDef *controll
 }
 
 
-static int
-qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *controller,
-                                            const virDomainDef *def,
-                                            virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED)
-{
-    /* 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;
-
-    return 0;
-}
-
-
 /**
  * virDomainControllerPCIModelNameToQEMUCaps:
  * @modelName: model name
@@ -4770,7 +4751,7 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
         return -1;
     }
 
-    return qemuDomainDeviceDefValidateControllerPCIOld(cont, def, qemuCaps);
+    return 0;
 }
 
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list