[PATCH v3 12/14] domain_conf: move virDomainPCIControllerOpts checks to domain_validate.c

Daniel Henrique Barboza danielhb413 at gmail.com
Tue Dec 8 22:20:28 UTC 2020


virDomainControllerDefParseXML() does a lot of checks with
virDomainPCIControllerOpts parameters that can be moved to
virDomainControllerDefValidate, sharing the logic with other use
cases that does not rely on XML parsing.

'pseries-default-phb-numa-node' parse error was changed to reflect
the error that is being thrown by qemuValidateDomainDeviceDefController()
via deviceValidateCallback, that is executed before
virDomainControllerDefValidate().

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
---
 src/conf/domain_conf.c                        | 42 +---------------
 src/conf/domain_validate.c                    | 48 +++++++++++++++++++
 .../pseries-default-phb-numa-node.err         |  2 +-
 tests/qemuxml2argvtest.c                      |  6 ++-
 4 files changed, 56 insertions(+), 42 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 453dc6cf6a..7adf2700ae 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10864,14 +10864,6 @@ virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt,
                                chassisNr);
                 return NULL;
             }
-            if (def->opts.pciopts.chassisNr < 1 ||
-                def->opts.pciopts.chassisNr > 255) {
-                virReportError(VIR_ERR_XML_ERROR,
-                               _("PCI controller chassisNr '%s' out of range "
-                                 "- must be 1-255"),
-                               chassisNr);
-                return NULL;
-            }
         }
         if (chassis) {
             if (virStrToLong_i(chassis, NULL, 0,
@@ -10881,14 +10873,6 @@ virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt,
                                chassis);
                 return NULL;
             }
-            if (def->opts.pciopts.chassis < 0 ||
-                def->opts.pciopts.chassis > 255) {
-                virReportError(VIR_ERR_XML_ERROR,
-                               _("PCI controller chassis '%s' out of range "
-                                 "- must be 0-255"),
-                               chassis);
-                return NULL;
-            }
         }
         if (port) {
             if (virStrToLong_i(port, NULL, 0,
@@ -10898,14 +10882,6 @@ virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt,
                                port);
                 return NULL;
             }
-            if (def->opts.pciopts.port < 0 ||
-                def->opts.pciopts.port > 255) {
-                virReportError(VIR_ERR_XML_ERROR,
-                               _("PCI controller port '%s' out of range "
-                                 "- must be 0-255"),
-                               port);
-                return NULL;
-            }
         }
         if (busNr) {
             if (virStrToLong_i(busNr, NULL, 0,
@@ -10915,14 +10891,6 @@ virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt,
                                busNr);
                 return NULL;
             }
-            if (def->opts.pciopts.busNr < 1 ||
-                def->opts.pciopts.busNr > 254) {
-                virReportError(VIR_ERR_XML_ERROR,
-                               _("PCI controller busNr '%s' out of range "
-                                 "- must be 1-254"),
-                               busNr);
-                return NULL;
-            }
         }
         if (targetIndex) {
             if (virStrToLong_i(targetIndex, NULL, 0,
@@ -10934,15 +10902,9 @@ virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt,
                 return NULL;
             }
         }
-        if (numaNode >= 0) {
-            if (def->idx == 0) {
-                virReportError(VIR_ERR_XML_ERROR, "%s",
-                               _("The PCI controller with index=0 can't "
-                                 "be associated with a NUMA node"));
-                return NULL;
-            }
+        if (numaNode >= 0)
             def->opts.pciopts.numaNode = numaNode;
-        }
+
         if (hotplug) {
             int val = virTristateSwitchTypeFromString(hotplug);
 
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 416c24f97b..f47e80ca38 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -551,6 +551,54 @@ virDomainControllerDefValidate(const virDomainControllerDef *controller)
                 return -1;
             }
         }
+
+        if (opts->chassisNr != -1) {
+            if (opts->chassisNr < 1 || opts->chassisNr > 255) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("PCI controller chassisNr '%d' out of range "
+                                 "- must be 1-255"),
+                               opts->chassisNr);
+                return -1;
+            }
+        }
+
+        if (opts->chassis != -1) {
+            if (opts->chassis < 0 || opts->chassis > 255) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("PCI controller chassis '%d' out of range "
+                                 "- must be 0-255"),
+                               opts->chassis);
+                return -1;
+            }
+        }
+
+        if (opts->port != -1) {
+            if (opts->port < 0 || opts->port > 255) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("PCI controller port '%d' out of range "
+                                 "- must be 0-255"),
+                               opts->port);
+                return -1;
+            }
+        }
+
+        if (opts->busNr != -1) {
+            if (opts->busNr < 1 || opts->busNr > 254) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("PCI controller busNr '%d' out of range "
+                                 "- must be 1-254"),
+                               opts->busNr);
+                return -1;
+            }
+        }
+
+        if (opts->numaNode >= 0 && controller->idx == 0) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("The PCI controller with index=0 can't "
+                             "be associated with a NUMA node"));
+            return -1;
+        }
     }
+
     return 0;
 }
diff --git a/tests/qemuxml2argvdata/pseries-default-phb-numa-node.err b/tests/qemuxml2argvdata/pseries-default-phb-numa-node.err
index 5d11109317..e46b710330 100644
--- a/tests/qemuxml2argvdata/pseries-default-phb-numa-node.err
+++ b/tests/qemuxml2argvdata/pseries-default-phb-numa-node.err
@@ -1 +1 @@
-XML error: The PCI controller with index=0 can't be associated with a NUMA node
+unsupported configuration: Option 'numaNode' is not valid for PCI controller with index '0', model 'pci-root' and modelName 'spapr-pci-host-bridge'
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0e7d8d5ba3..9b853c6d59 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2115,7 +2115,11 @@ mymain(void)
             QEMU_CAPS_OBJECT_MEMORY_RAM,
             QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE);
-    DO_TEST_PARSE_ERROR("pseries-default-phb-numa-node", NONE);
+    DO_TEST_PARSE_ERROR("pseries-default-phb-numa-node",
+                        QEMU_CAPS_NUMA,
+                        QEMU_CAPS_OBJECT_MEMORY_RAM,
+                        QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
+                        QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE);
     DO_TEST_PARSE_ERROR("pseries-phb-invalid-target-index-1", NONE);
     DO_TEST_PARSE_ERROR("pseries-phb-invalid-target-index-2", NONE);
     DO_TEST_PARSE_ERROR("pseries-phb-invalid-target-index-3", NONE);
-- 
2.26.2




More information about the libvir-list mailing list