From 7bdac52dd6979f76f9a75a404a76e79f7da11a06 Mon Sep 17 00:00:00 2001

Message-Id: <7bdac52dd6979f76f9a75a404a76e79f7da11a06@dist-git>

From: Jiri Denemark <jdenemar@redhat.com>

Date: Tue, 23 May 2017 09:29:36 +0200

Subject: [PATCH] conf: Refactor virCPUDefParseXML

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

(cherry picked from commit 702013f3b3ad8bd28c326058e2dd9ea8afbd1e61)

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

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/conf/cpu_conf.c    | 109 +++++++++++++++++++++++++++----------------------

 src/conf/cpu_conf.h    |   9 ++--

 src/conf/domain_conf.c |  12 +-----

 src/cpu/cpu.c          |   5 +--

 tests/cputest.c        |   5 +--

 5 files changed, 72 insertions(+), 68 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c

index ffb2e83d67..da40e9ba97 100644

--- a/src/conf/cpu_conf.c

+++ b/src/conf/cpu_conf.c

@@ -245,12 +245,25 @@ virCPUDefCopy(const virCPUDef *cpu)

 }

-virCPUDefPtr

-virCPUDefParseXML(xmlNodePtr node,

-                  xmlXPathContextPtr ctxt,

-                  virCPUType type)

+/*

+ * Parses CPU definition XML from a node pointed to by @xpath. If @xpath is

+ * NULL, the current node of @ctxt is used (i.e., it is a shortcut to ".").

+ *

+ * Missing <cpu> element in the XML document is not considered an error unless

+ * @xpath is NULL in which case the function expects it was provided with a

+ * valid <cpu> element already. In other words, the function returns success

+ * and sets @cpu to NULL if @xpath is not NULL and the node pointed to by

+ * @xpath is not found.

+ *

+ * Returns 0 on success, -1 on error.

+ */

+int

+virCPUDefParseXML(xmlXPathContextPtr ctxt,

+                  const char *xpath,

+                  virCPUType type,

+                  virCPUDefPtr *cpu)

 {

-    virCPUDefPtr def;

+    virCPUDefPtr def = NULL;

     xmlNodePtr *nodes = NULL;

     xmlNodePtr oldnode = ctxt->node;

     int n;

@@ -258,15 +271,23 @@ virCPUDefParseXML(xmlNodePtr node,

     char *cpuMode;

     char *fallback = NULL;

     char *vendor_id = NULL;

+    int ret = -1;

-    if (!xmlStrEqual(node->name, BAD_CAST "cpu")) {

+    *cpu = NULL;

+

+    if (xpath && !(ctxt->node = virXPathNode(xpath, ctxt))) {

+        ret = 0;

+        goto cleanup;

+    }

+

+    if (!xmlStrEqual(ctxt->node->name, BAD_CAST "cpu")) {

         virReportError(VIR_ERR_XML_ERROR, "%s",

                        _("XML does not contain expected 'cpu' element"));

-        return NULL;

+        goto cleanup;

     }

     if (VIR_ALLOC(def) < 0)

-        return NULL;

+        goto cleanup;

     if (type == VIR_CPU_TYPE_AUTO) {

         if (virXPathBoolean("boolean(./arch)", ctxt)) {

@@ -274,7 +295,7 @@ virCPUDefParseXML(xmlNodePtr node,

                 virReportError(VIR_ERR_XML_ERROR, "%s",

                                _("'arch' element cannot be used inside 'cpu'"

                                  " element with 'match' attribute'"));

-                goto error;

+                goto cleanup;

             }

             def->type = VIR_CPU_TYPE_HOST;

         } else {

@@ -284,12 +305,12 @@ virCPUDefParseXML(xmlNodePtr node,

         def->type = type;

     }

-    if ((cpuMode = virXMLPropString(node, "mode"))) {

+    if ((cpuMode = virXMLPropString(ctxt->node, "mode"))) {

         if (def->type == VIR_CPU_TYPE_HOST) {

             VIR_FREE(cpuMode);

             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",

                            _("Attribute mode is only allowed for guest CPU"));

-            goto error;

+            goto cleanup;

         } else {

             def->mode = virCPUModeTypeFromString(cpuMode);

@@ -298,7 +319,7 @@ virCPUDefParseXML(xmlNodePtr node,

                                _("Invalid mode attribute '%s'"),

                                cpuMode);

                 VIR_FREE(cpuMode);

-                goto error;

+                goto cleanup;

             }

             VIR_FREE(cpuMode);

         }

@@ -310,7 +331,7 @@ virCPUDefParseXML(xmlNodePtr node,

     }

     if (def->type == VIR_CPU_TYPE_GUEST) {

-        char *match = virXMLPropString(node, "match");

+        char *match = virXMLPropString(ctxt->node, "match");

         char *check;

         if (!match) {

@@ -326,11 +347,11 @@ virCPUDefParseXML(xmlNodePtr node,

                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",

                                _("Invalid match attribute for CPU "

                                  "specification"));

-                goto error;

+                goto cleanup;

             }

         }

-        if ((check = virXMLPropString(node, "check"))) {

+        if ((check = virXMLPropString(ctxt->node, "check"))) {

             int value = virCPUCheckTypeFromString(check);

             VIR_FREE(check);

@@ -338,7 +359,7 @@ virCPUDefParseXML(xmlNodePtr node,

                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",

                                _("Invalid check attribute for CPU "

                                  "specification"));

-                goto error;

+                goto cleanup;

             }

             def->check = value;

         }

@@ -349,13 +370,13 @@ virCPUDefParseXML(xmlNodePtr node,

         if (!arch) {

             virReportError(VIR_ERR_XML_ERROR, "%s",

                            _("Missing CPU architecture"));

-            goto error;

+            goto cleanup;

         }

         if ((def->arch = virArchFromString(arch)) == VIR_ARCH_NONE) {

             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,

                            _("Unknown architecture %s"), arch);

             VIR_FREE(arch);

-            goto error;

+            goto cleanup;

         }

         VIR_FREE(arch);

     }

@@ -364,7 +385,7 @@ virCPUDefParseXML(xmlNodePtr node,

         def->type == VIR_CPU_TYPE_HOST) {

         virReportError(VIR_ERR_XML_ERROR, "%s",

                         _("Missing CPU model name"));

-        goto error;

+        goto cleanup;

     }

     if (def->type == VIR_CPU_TYPE_GUEST &&

@@ -374,7 +395,7 @@ virCPUDefParseXML(xmlNodePtr node,

             if ((def->fallback = virCPUFallbackTypeFromString(fallback)) < 0) {

                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",

                                _("Invalid fallback attribute"));

-                goto error;

+                goto cleanup;

             }

         }

@@ -384,14 +405,14 @@ virCPUDefParseXML(xmlNodePtr node,

                 virReportError(VIR_ERR_XML_ERROR,

                                _("vendor_id must be exactly %d characters long"),

                                VIR_CPU_VENDOR_ID_LENGTH);

-                goto error;

+                goto cleanup;

             }

             /* ensure that the string can be passed to qemu*/

             if (strchr(vendor_id, ',')) {

                     virReportError(VIR_ERR_XML_ERROR, "%s",

                                    _("vendor id is invalid"));

-                    goto error;

+                    goto cleanup;

             }

             def->vendor_id = vendor_id;

@@ -403,61 +424,54 @@ virCPUDefParseXML(xmlNodePtr node,

     if (def->vendor && !def->model) {

         virReportError(VIR_ERR_XML_ERROR, "%s",

                        _("CPU vendor specified without CPU model"));

-        goto error;

+        goto cleanup;

     }

     if (virXPathNode("./topology[1]", ctxt)) {

-        int ret;

         unsigned long ul;

-        ret = virXPathULong("string(./topology[1]/@sockets)",

-                            ctxt, &ul);

-        if (ret < 0) {

+        if (virXPathULong("string(./topology[1]/@sockets)", ctxt, &ul) < 0) {

             virReportError(VIR_ERR_XML_ERROR, "%s",

                            _("Missing 'sockets' attribute in CPU topology"));

-            goto error;

+            goto cleanup;

         }

         def->sockets = (unsigned int) ul;

-        ret = virXPathULong("string(./topology[1]/@cores)",

-                            ctxt, &ul);

-        if (ret < 0) {

+        if (virXPathULong("string(./topology[1]/@cores)", ctxt, &ul) < 0) {

             virReportError(VIR_ERR_XML_ERROR, "%s",

                            _("Missing 'cores' attribute in CPU topology"));

-            goto error;

+            goto cleanup;

         }

         def->cores = (unsigned int) ul;

-        ret = virXPathULong("string(./topology[1]/@threads)",

-                            ctxt, &ul);

-        if (ret < 0) {

+        if (virXPathULong("string(./topology[1]/@threads)", ctxt, &ul) < 0) {

             virReportError(VIR_ERR_XML_ERROR, "%s",

                            _("Missing 'threads' attribute in CPU topology"));

-            goto error;

+            goto cleanup;

         }

         def->threads = (unsigned int) ul;

         if (!def->sockets || !def->cores || !def->threads) {

             virReportError(VIR_ERR_XML_ERROR, "%s",

                            _("Invalid CPU topology"));

-            goto error;

+            goto cleanup;

         }

     }

     if ((n = virXPathNodeSet("./feature", ctxt, &nodes)) < 0)

-        goto error;

+        goto cleanup;

     if (n > 0) {

         if (!def->model && def->mode == VIR_CPU_MODE_CUSTOM) {

             virReportError(VIR_ERR_XML_ERROR, "%s",

                            _("Non-empty feature list specified without "

                              "CPU model"));

-            goto error;

+            goto cleanup;

         }

         if (VIR_RESIZE_N(def->features, def->nfeatures_max,

                          def->nfeatures, n) < 0)

-            goto error;

+            goto cleanup;

         def->nfeatures = n;

     }

@@ -480,7 +494,7 @@ virCPUDefParseXML(xmlNodePtr node,

             if (policy < 0) {

                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",

                                _("Invalid CPU feature policy"));

-                goto error;

+                goto cleanup;

             }

         } else {

             policy = -1;

@@ -490,7 +504,7 @@ virCPUDefParseXML(xmlNodePtr node,

             VIR_FREE(name);

             virReportError(VIR_ERR_XML_ERROR, "%s",

                            _("Invalid CPU feature name"));

-            goto error;

+            goto cleanup;

         }

         for (j = 0; j < i; j++) {

@@ -499,7 +513,7 @@ virCPUDefParseXML(xmlNodePtr node,

                                _("CPU feature '%s' specified more than once"),

                                name);

                 VIR_FREE(name);

-                goto error;

+                goto cleanup;

             }

         }

@@ -542,17 +556,16 @@ virCPUDefParseXML(xmlNodePtr node,

         def->cache->mode = mode;

     }

+    VIR_STEAL_PTR(*cpu, def);

+    ret = 0;

+

  cleanup:

     ctxt->node = oldnode;

     VIR_FREE(fallback);

     VIR_FREE(vendor_id);

     VIR_FREE(nodes);

-    return def;

-

- error:

     virCPUDefFree(def);

-    def = NULL;

-    goto cleanup;

+    return ret;

 }

diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h

index b0d891552a..b44974f47e 100644

--- a/src/conf/cpu_conf.h

+++ b/src/conf/cpu_conf.h

@@ -182,10 +182,11 @@ virCPUDefCopy(const virCPUDef *cpu);

 virCPUDefPtr

 virCPUDefCopyWithoutModel(const virCPUDef *cpu);

-virCPUDefPtr

-virCPUDefParseXML(xmlNodePtr node,

-                  xmlXPathContextPtr ctxt,

-                  virCPUType mode);

+int

+virCPUDefParseXML(xmlXPathContextPtr ctxt,

+                  const char *xpath,

+                  virCPUType mode,

+                  virCPUDefPtr *cpu);

 bool

 virCPUDefIsEqual(virCPUDefPtr src,

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

index 2197330a22..395dcc0531 100644

--- a/src/conf/domain_conf.c

+++ b/src/conf/domain_conf.c

@@ -17418,16 +17418,8 @@ virDomainDefParseXML(xmlDocPtr xml,

     }

     VIR_FREE(nodes);

-    /* analysis of cpu handling */

-    if ((node = virXPathNode("./cpu[1]", ctxt)) != NULL) {

-        xmlNodePtr oldnode = ctxt->node;

-        ctxt->node = node;

-        def->cpu = virCPUDefParseXML(node, ctxt, VIR_CPU_TYPE_GUEST);

-        ctxt->node = oldnode;

-

-        if (def->cpu == NULL)

-            goto error;

-    }

+    if (virCPUDefParseXML(ctxt, "./cpu[1]", VIR_CPU_TYPE_GUEST, &def->cpu) < 0)

+        goto error;

     if (virDomainNumaDefCPUParseXML(def->numa, ctxt) < 0)

         goto error;

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c

index 702b14dbb7..96160901e1 100644

--- a/src/cpu/cpu.c

+++ b/src/cpu/cpu.c

@@ -130,7 +130,7 @@ virCPUCompareXML(virArch arch,

     if (!(doc = virXMLParseStringCtxt(xml, _("(CPU_definition)"), &ctxt)))

         goto cleanup;

-    if (!(cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO)))

+    if (virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_AUTO, &cpu) < 0)

         goto cleanup;

     ret = virCPUCompare(arch, host, cpu, failIncompatible);

@@ -562,8 +562,7 @@ cpuBaselineXML(const char **xmlCPUs,

         if (!(doc = virXMLParseStringCtxt(xmlCPUs[i], _("(CPU_definition)"), &ctxt)))

             goto error;

-        cpus[i] = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_HOST);

-        if (cpus[i] == NULL)

+        if (virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_HOST, &cpus[i]) < 0)

             goto error;

         xmlXPathFreeContext(ctxt);

diff --git a/tests/cputest.c b/tests/cputest.c

index efa891dc18..97b34de9ed 100644

--- a/tests/cputest.c

+++ b/tests/cputest.c

@@ -88,7 +88,7 @@ cpuTestLoadXML(virArch arch, const char *name)

     if (!(doc = virXMLParseFileCtxt(xml, &ctxt)))

         goto cleanup;

-    cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO);

+    virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_AUTO, &cpu);

  cleanup:

     xmlXPathFreeContext(ctxt);

@@ -126,8 +126,7 @@ cpuTestLoadMultiXML(virArch arch,

     for (i = 0; i < n; i++) {

         ctxt->node = nodes[i];

-        cpus[i] = virCPUDefParseXML(nodes[i], ctxt, VIR_CPU_TYPE_HOST);

-        if (!cpus[i])

+        if (virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_HOST, &cpus[i]) < 0)

             goto cleanup_cpus;

     }

-- 

2.13.1