Re: [libvirt] [PATCH] Support CPU less NUMA node.

Some platform NUMA node does not include cpu, such as Phi. This patch support CPU less NUMA node. But there must be one CPU cell node for a guest. Also must assign the host nodeset for this guest cell node.

please enable numactl with "--with-numactl" for libvirt config. Test this patch: 1. define a numa cell without "cpus", such as cell 1. <numatune> <memnode cellid='1' mode='strict' nodeset='0'/> </numatune> <cpu> <numa> <cell id='0' cpus='0-1' memory='512000' unit='KiB'/> <cell id='1' memory='512000' unit='KiB'/> </numa> </cpu> libvirt can edit and start the VM successfully. 2. define a numa cell without "cpus", without nodeset. <cpu> <numa> <cell id='0' cpus='0-1' memory='512000' unit='KiB'/> <cell id='1' memory='512000' unit='KiB'/>

</numa> </cpu> This case, libvirt will failed to edit the CPUS. --- src/conf/domain_conf.c | 4 +++ src/conf/numa_conf.c | 89 ++++++++++++++++++++++++++++++++++--------------- src/conf/numa_conf.h | 2 ++ src/qemu/qemu_command.c | 12 ++++--- src/util/virbitmap.c | 10 ++++-- 5 files changed, 83 insertions(+), 34 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 705deb3..d6e5489 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17330,6 +17330,10 @@ virDomainDefParseXML(xmlDocPtr xml, ctxt) < 0) goto error; + if (virDomainNumaCPULessCheck(def->numa) < 0){ + goto error; + }

+ if (virDomainNumatuneHasPlacementAuto(def->numa) && !def->cpumask && !virDomainDefHasVcpuPin(def) && !def->cputune.emulatorpin && diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index bfd3703..1b6f7ff 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -742,34 +742,31 @@ virDomainNumaDefCPUParseXML(virDomainNumaPtr def, goto cleanup; } - if (!(tmp = virXMLPropString(nodes[i], "cpus"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Missing 'cpus' attribute in NUMA cell")); - goto cleanup; - } - - if (virBitmapParse(tmp, &def->mem_nodes[cur_cell].cpumask, - VIR_DOMAIN_CPUMASK_LEN) < 0) - goto cleanup; - - if (virBitmapIsAllClear(def->mem_nodes[cur_cell].cpumask)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("NUMA cell %d has no vCPUs assigned"), cur_cell); - goto cleanup; - } - VIR_FREE(tmp); - - for (j = 0; j < n; j++) { - if (j == cur_cell || !def->mem_nodes[j].cpumask) - continue; + tmp = virXMLPropString(nodes[i], "cpus"); + if (tmp) { + if (virBitmapParse(tmp, &def->mem_nodes[cur_cell].cpumask, + VIR_DOMAIN_CPUMASK_LEN) < 0) + goto cleanup; - if (virBitmapOverlaps(def->mem_nodes[j].cpumask, - def->mem_nodes[cur_cell].cpumask)) { + if (virBitmapIsAllClear(def->mem_nodes[cur_cell].cpumask)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("NUMA cells %u and %zu have overlapping vCPU ids"), - cur_cell, j); + _("NUMA cell %d has no vCPUs assigned"), cur_cell); goto cleanup; } + VIR_FREE(tmp); + + for (j = 0; j < n; j++) { + if (j == cur_cell || !def->mem_nodes[j].cpumask) + continue; + + if (virBitmapOverlaps(def->mem_nodes[j].cpumask, + def->mem_nodes[cur_cell].cpumask)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("NUMA cells %u and %zu have overlapping vCPU ids"), + cur_cell, j); + goto cleanup; + } + } } ctxt->node = nodes[i]; @@ -808,6 +805,7 @@ virDomainNumaDefCPUFormat(virBufferPtr buf, char *cpustr; size_t ncells = virDomainNumaGetNodeCount(def); size_t i; + size_t ncpuless = 0; if (ncells == 0) return 0; @@ -817,12 +815,24 @@ virDomainNumaDefCPUFormat(virBufferPtr buf, for (i = 0; i < ncells; i++) { memAccess = virDomainNumaGetNodeMemoryAccessMode(def, i); - if (!(cpustr = virBitmapFormat(virDomainNumaGetNodeCpumask(def, i)))) - return -1; + cpustr = virBitmapFormat(virDomainNumaGetNodeCpumask(def, i)); virBufferAddLit(buf, "<cell"); virBufferAsprintf(buf, " id='%zu'", i); - virBufferAsprintf(buf, " cpus='%s'", cpustr); + if (cpustr && (*cpustr != 0)) + virBufferAsprintf(buf, " cpus='%s'", cpustr); + else { + /* Note, at present we only allow cpuless numa node with */ + /* nodeset assign. */ + ncpuless++; + if (!virDomainNumatuneNodeSpecified(def, i)) { + VIR_FREE(cpustr); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("CPUless NUMA node cellid %zu must be " + "specified node set."), i); + return -1;

+ } + } virBufferAsprintf(buf, " memory='%llu'", virDomainNumaGetNodeMemorySize(def, i)); virBufferAddLit(buf, " unit='KiB'"); @@ -835,6 +845,12 @@ virDomainNumaDefCPUFormat(virBufferPtr buf, virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</numa>\n"); + if (ncpuless == ncells) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("At lease one NUMA node should " + "include CPU element.")); + return -1;

+ } return 0; } @@ -861,6 +877,7 @@ virDomainNumaGetMaxCPUID(virDomainNumaPtr numa) int bit; bit = virBitmapLastSetBit(virDomainNumaGetNodeCpumask(numa, i)); + bit = (bit > 0) ? bit : 0;

if (bit > ret) ret = bit; } @@ -973,3 +990,21 @@ virDomainNumaGetMemorySize(virDomainNumaPtr numa) return ret; } + +// NOTE, For some plateforms, there are some node without cpus, such as Phi. +// We just need do some check for these plateforms, we must assigned nodeset +// for CPU less node.

+int +virDomainNumaCPULessCheck(virDomainNumaPtr numa) +{ + int ret = 0; + int i = 0; + for (i = 0; i < numa->nmem_nodes; i++){ + if (!numa->mem_nodes[i].cpumask && !numa->mem_nodes[i].nodeset) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cell: %d is a cpuless numa, must specfiy the nodeset."), i); + return -1; + } + } + return ret;

+} diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index b6a5354..1f711e6 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -155,5 +155,7 @@ int virDomainNumaDefCPUFormat(virBufferPtr buf, virDomainNumaPtr def); unsigned int virDomainNumaGetCPUCountTotal(virDomainNumaPtr numa); +int virDomainNumaCPULessCheck(virDomainNumaPtr numa); + #endif /* __NUMA_CONF_H__ */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b2e76ca..b07ca66 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7665,11 +7665,13 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virCommandAddArg(cmd, "-numa"); virBufferAsprintf(&buf, "node,nodeid=%zu", i); - for (tmpmask = cpumask; tmpmask; tmpmask = next) { - if ((next = strchr(tmpmask, ','))) - *(next++) = '\0'; - virBufferAddLit(&buf, ",cpus="); - virBufferAdd(&buf, tmpmask, -1); + if (*cpumask != 0) {

+ for (tmpmask = cpumask; tmpmask; tmpmask = next) { + if ((next = strchr(tmpmask, ','))) + *(next++) = '\0'; + virBufferAddLit(&buf, ",cpus="); + virBufferAdd(&buf, tmpmask, -1); + } } if (needBackend) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index eac63d9..b2803ce 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -953,6 +953,9 @@ virBitmapLastSetBit(virBitmapPtr bitmap) unsigned long bits; /* If bitmap is empty then there is no set bit */ + if (!bitmap) + return -1;

+ if (bitmap->map_len == 0) return -1; @@ -1039,9 +1042,12 @@ virBitmapCountBits(virBitmapPtr bitmap) size_t i; size_t ret = 0; - for (i = 0; i < bitmap->map_len; i++) - ret += count_one_bits_l(bitmap->map[i]); + if (bitmap == NULL) + return ret; + for (i = 0; i < bitmap->map_len; i++) { + ret += count_one_bits_l(bitmap->map[i]); + }

return ret; } -- 2.7.4 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list