[libvirt] [PATCH v2 02/20] conf: Refactor virCPUDefParseXML

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - no need to distinguish 0 vs 1 return codes - better documentation 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 ffb2e83d6..ae9f52c8d 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 b0d891552..b44974f47 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 5f99dad9c..53c145f16 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17444,16 +17444,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 702b14dbb..96160901e 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 d5e023c40..89c645e64 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.0