Re: [libvirt] [PATCH 02/24] conf: Move NUMA cell parsing code from cpu conf to numa conf

For weird historical reasons NUMA cells are added as a subelement of <cpu> while the actual configuration is done in <numatune>. This patch splits out the cell parser code from cpu config to NUMA config. Note that the changes to the code are minimal just to make it work and the function will be refactored in the next patch. --- src/conf/cpu_conf.c | 90 --------------------------------------- src/conf/domain_conf.c | 17 +++++--- src/conf/numa_conf.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/numa_conf.h | 4 ++ 4 files changed, 126 insertions(+), 96 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 4a367a1..f14b37a 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -426,96 +426,6 @@ virCPUDefParseXML(xmlNodePtr node, def->features[i].policy = policy; } - if (virXPathNode("./numa[1]", ctxt)) { - VIR_FREE(nodes); - n = virXPathNodeSet("./numa[1]/cell", ctxt, &nodes); - if (n <= 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("NUMA topology defined without NUMA cells")); - goto error; - } - - 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 ret, ncpus = 0; - unsigned int cur_cell; - char *tmp = NULL; - - tmp = virXMLPropString(nodes[i], "id"); - if (!tmp) { - cur_cell = i; - } else { - ret = virStrToLong_ui(tmp, NULL, 10, &cur_cell); - if (ret == -1) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid 'id' attribute in NUMA cell: %s"), - tmp); - VIR_FREE(tmp); - goto error; - } - VIR_FREE(tmp); - } - - if (cur_cell >= n) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("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; - } - - 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; - - ncpus = virBitmapParse(cpus, 0, &def->cells[cur_cell].cpumask, - VIR_DOMAIN_CPUMASK_LEN); - if (ncpus <= 0) - goto error; - def->cells_cpus += ncpus; - - ctxt->node = nodes[i]; - if (virDomainParseMemory("./@memory", "./@unit", ctxt, - &def->cells[cur_cell].mem, true, false) < 0) - goto cleanup; - - memAccessStr = virXMLPropString(nodes[i], "memAccess"); - if (memAccessStr) { - int rc = virMemAccessTypeFromString(memAccessStr); - - if (rc <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Invalid 'memAccess' attribute " - "value '%s'"), - memAccessStr); - VIR_FREE(memAccessStr); - goto error; - } - - def->cells[cur_cell].memAccess = rc; - - VIR_FREE(memAccessStr); - } - } - } - cleanup: ctxt->node = oldnode; VIR_FREE(fallback); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b13cae8..06ed0fd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13495,12 +13495,17 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; } - if (def->cpu->cells_cpus > def->maxvcpus) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Number of CPUs in <numa> exceeds the" - " <vcpu> count")); - goto error; - } + } + + if (virDomainNumaDefCPUParseXML(def->cpu, ctxt) < 0) + goto error; + + if (def->cpu && + def->cpu->cells_cpus > def->maxvcpus) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Number of CPUs in <numa> exceeds the" + " <vcpu> count")); + goto error; } if (virDomainNumatuneParseXML(&def->numatune, diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index f6a8248..e36a4be 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -680,3 +680,114 @@ virDomainNumatuneNodesetIsAvailable(virDomainNumatunePtr numatune, return true; } + + +int +virDomainNumaDefCPUParseXML(virCPUDefPtr def, + xmlXPathContextPtr ctxt) +{ + xmlNodePtr *nodes = NULL; + xmlNodePtr oldNode = ctxt->node; + int n; + size_t i; + int ret = -1; + + if (virXPathNode("/domain/cpu/numa[1]", ctxt)) { + VIR_FREE(nodes); + + 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 (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); + } + + if (cur_cell >= n) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("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; + } + + 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; + + ncpus = virBitmapParse(cpus, 0, &def->cells[cur_cell].cpumask, + VIR_DOMAIN_CPUMASK_LEN); + if (ncpus <= 0) + goto error; + def->cells_cpus += ncpus; + + ctxt->node = nodes[i]; + if (virDomainParseMemory("./@memory", "./@unit", ctxt, + &def->cells[cur_cell].mem, true, false) < 0) + goto cleanup; + + memAccessStr = virXMLPropString(nodes[i], "memAccess"); + if (memAccessStr) { + rc = virMemAccessTypeFromString(memAccessStr); + + if (rc <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid 'memAccess' attribute " + "value '%s'"), + memAccessStr); + VIR_FREE(memAccessStr); + goto error; + } + + def->cells[cur_cell].memAccess = rc; + + VIR_FREE(memAccessStr); + } + } + } + + ret = 0; + + error: + cleanup:

+ ctxt->node = oldNode; + VIR_FREE(nodes); + return ret; +} diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index bcc8c8a..276d25a 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -29,6 +29,7 @@ # include "virutil.h" # include "virbitmap.h" # include "virbuffer.h" +# include "cpu_conf.h" typedef struct _virDomainNumatune virDomainNumatune; @@ -111,4 +112,7 @@ int virDomainNumatuneSpecifiedMaxNode(virDomainNumatunePtr numatune); bool virDomainNumatuneNodesetIsAvailable(virDomainNumatunePtr numatune, virBitmapPtr auto_nodeset); + +int virDomainNumaDefCPUParseXML(virCPUDefPtr def, xmlXPathContextPtr ctxt); + #endif /* __NUMA_CONF_H__ */