conf: Refactor virDomainNumaDefCPUParseXML

Rewrite the function to save a few local variables and reorder the code
to make more sense.

Additionally the ncells_max member of the virCPUDef structure is used
only for tracking allocation when parsing the numa definition, which can
be avoided by switching to VIR_ALLOC_N as the array is not resized
after initial allocation.

4 files changed, 59 insertions, 74 deletions

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index f14b37ae19..98b309b0f9 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -154,7 +154,6 @@ virCPUDefCopy(const virCPUDef *cpu)
     if (cpu->ncells) {
         if (VIR_ALLOC_N(copy->cells, cpu->ncells) < 0)
             goto error;
-        copy->ncells_max = copy->ncells = cpu->ncells;
 
         for (i = 0; i < cpu->ncells; i++) {
             copy->cells[i].mem = cpu->cells[i].mem;
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index 46fce25b5c..e201656205 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -127,7 +127,6 @@ struct _virCPUDef {
     size_t nfeatures_max;
     virCPUFeatureDefPtr features;
     size_t ncells;
-    size_t ncells_max;
     virCellDefPtr cells;
     unsigned int cells_cpus;
 };
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index b4f0fca13e..8879c16711 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -688,45 +688,36 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def,
 {
     xmlNodePtr *nodes = NULL;
     xmlNodePtr oldNode = ctxt->node;
+    char *tmp = NULL;
     int n;
     size_t i;
     int ret = -1;
 
-    if (virXPathNode("/domain/cpu/numa[1]", ctxt)) {
-        VIR_FREE(nodes);
+    /* check if NUMA definition is present */
+    if (!virXPathNode("/domain/cpu/numa[1]", ctxt))
+        return 0;
 
-        n = virXPathNodeSet("/domain/cpu/numa[1]/cell", ctxt, &nodes);
-        if (n <= 0) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("NUMA topology defined without NUMA cells"));
-            goto error;
-        }
+    if ((n = virXPathNodeSet("/domain/cpu/numa[1]/cell", ctxt, &nodes)) <= 0) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("NUMA topology defined without NUMA cells"));
+        goto cleanup;
+    }
+
+    if (VIR_ALLOC_N(def->cells, n) < 0)
+        goto cleanup;
+    def->ncells = n;
 
-        if (VIR_RESIZE_N(def->cells, def->ncells_max,
-                         def->ncells, n) < 0)
-            goto error;
-
-        def->ncells = n;
-
-        for (i = 0; i < n; i++) {
-            char *cpus, *memAccessStr;
-            int rc, ncpus = 0;
-            unsigned int cur_cell;
-            char *tmp = NULL;
-
-            tmp = virXMLPropString(nodes[i], "id");
-            if (!tmp) {
-                cur_cell = i;
-            } else {
-                rc  = virStrToLong_ui(tmp, NULL, 10, &cur_cell);
-                if (rc == -1) {
-                    virReportError(VIR_ERR_XML_ERROR,
-                                   _("Invalid 'id' attribute in NUMA cell: %s"),
-                                   tmp);
-                    VIR_FREE(tmp);
-                    goto error;
-                }
-                VIR_FREE(tmp);
+    for (i = 0; i < n; i++) {
+        int rc, ncpus = 0;
+        unsigned int cur_cell = i;
+
+        /* cells are in order of parsing or explicitly numbered */
+        if ((tmp = virXMLPropString(nodes[i], "id"))) {
+            if (virStrToLong_uip(tmp, NULL, 10, &cur_cell) < 0) {
+                virReportError(VIR_ERR_XML_ERROR,
+                               _("Invalid 'id' attribute in NUMA cell: '%s'"),
+                               tmp);
+                goto cleanup;
             }
 
             if (cur_cell >= n) {
@@ -734,60 +725,57 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def,
                                _("Exactly one 'cell' element per guest "
                                  "NUMA cell allowed, non-contiguous ranges or "
                                  "ranges not starting from 0 are not allowed"));
-                goto error;
-            }
-
-            if (def->cells[cur_cell].cpustr) {
-                virReportError(VIR_ERR_XML_ERROR,
-                               _("Duplicate NUMA cell info for cell id '%u'"),
-                               cur_cell);
-                goto error;
+                goto cleanup;
             }
+        }
+        VIR_FREE(tmp);
 
-            cpus = virXMLPropString(nodes[i], "cpus");
-            if (!cpus) {
-                virReportError(VIR_ERR_XML_ERROR, "%s",
-                               _("Missing 'cpus' attribute in NUMA cell"));
-                goto error;
-            }
-            def->cells[cur_cell].cpustr = cpus;
+        if (def->cells[cur_cell].cpumask) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("Duplicate NUMA cell info for cell id '%u'"),
+                           cur_cell);
+            goto cleanup;
+        }
 
-            ncpus = virBitmapParse(cpus, 0, &def->cells[cur_cell].cpumask,
-                                   VIR_DOMAIN_CPUMASK_LEN);
-            if (ncpus <= 0)
-                goto error;
-            def->cells_cpus += ncpus;
+        if (!(tmp = virXMLPropString(nodes[i], "cpus"))) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("Missing 'cpus' attribute in NUMA cell"));
+            goto cleanup;
+        }
 
-            ctxt->node = nodes[i];
-            if (virDomainParseMemory("./@memory", "./@unit", ctxt,
-                                     &def->cells[cur_cell].mem, true, false) < 0)
-                goto cleanup;
+        ncpus = virBitmapParse(tmp, 0, &def->cells[cur_cell].cpumask,
+                               VIR_DOMAIN_CPUMASK_LEN);
 
-            memAccessStr = virXMLPropString(nodes[i], "memAccess");
-            if (memAccessStr) {
-                rc = virMemAccessTypeFromString(memAccessStr);
+        if (ncpus <= 0)
+            goto cleanup;
+        def->cells_cpus += ncpus;
 
-                if (rc <= 0) {
-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                   _("Invalid 'memAccess' attribute "
-                                     "value '%s'"),
-                                   memAccessStr);
-                    VIR_FREE(memAccessStr);
-                    goto error;
-                }
+        def->cells[cur_cell].cpustr = tmp;
+        tmp = NULL;
 
-                def->cells[cur_cell].memAccess = rc;
+        ctxt->node = nodes[i];
+        if (virDomainParseMemory("./@memory", "./@unit", ctxt,
+                                 &def->cells[cur_cell].mem, true, false) < 0)
+            goto cleanup;
 
-                VIR_FREE(memAccessStr);
+        if ((tmp = virXMLPropString(nodes[i], "memAccess"))) {
+            if ((rc = virMemAccessTypeFromString(tmp)) <= 0) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("Invalid 'memAccess' attribute value '%s'"),
+                               tmp);
+                goto cleanup;
             }
+
+            def->cells[cur_cell].memAccess = rc;
+            VIR_FREE(tmp);
         }
     }
 
     ret = 0;
 
- error:
  cleanup:
     ctxt->node = oldNode;
     VIR_FREE(nodes);
+    VIR_FREE(tmp);
     return ret;
 }
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 7b26e50bde..64f709ae62 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -243,7 +243,6 @@ virCapsPtr testQemuCapsInit(void)
         ARRAY_CARDINALITY(host_cpu_features), /* nfeatures_max */
         host_cpu_features,      /* features */
         0,                      /* ncells */
-        0,                      /* ncells_max */
         NULL,                   /* cells */
         0,                      /* cells_cpus */
     };