2:44 p.m.
I'm looking into NUMA code in capabilities and came up with a couple of
cleanups. Technically, 3/4 is not needed yet, but I'll be introducing
new data to vircaps2xmltest where a NUMA node has no CPUs and that's
why the patch is needed.
Michal Prívozník (4):
numa_conf: Use virXMLFormatElement() in virDomainNumaDefFormatXML
virnuma: Export virNumaGetMaxCPUs properly
virnumamock: Allow CPU-less NUMA nodes
virCapabilitiesHostNUMAFormat: Bring variables into loops
src/conf/capabilities.c | 9 ++--
src/conf/numa_conf.c | 107 ++++++++++++++++++---------------------
src/libvirt_private.syms | 1 +
src/util/virnuma.h | 2 +-
tests/virnumamock.c | 7 ++-
5 files changed, 61 insertions(+), 65 deletions(-)
--
2.26.3
2:44 p.m.
New subject: [PATCH 1/4] numa_conf: Use virXMLFormatElement() in virDomainNumaDefFormatXML
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/numa_conf.c | 107 +++++++++++++++++++------------------------
1 file changed, 48 insertions(+), 59 deletions(-)
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 525bc28962..537e515b5a 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -1105,92 +1105,81 @@ virDomainNumaDefFormatXML(virBuffer *buf,
virBufferAdjustIndent(buf, 2);
for (i = 0; i < ncells; i++) {
virBitmap *cpumask = virDomainNumaGetNodeCpumask(def, i);
- int ndistances;
- size_t ncaches;
+ g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
+ g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
+ size_t j;
memAccess = virDomainNumaGetNodeMemoryAccessMode(def, i);
discard = virDomainNumaGetNodeDiscard(def, i);
- virBufferAddLit(buf, "<cell");
- virBufferAsprintf(buf, " id='%zu'", i);
+ virBufferAsprintf(&attrBuf, " id='%zu'", i);
if (cpumask) {
g_autofree char *cpustr = virBitmapFormat(cpumask);
if (!cpustr)
return -1;
- virBufferAsprintf(buf, " cpus='%s'", cpustr);
+ virBufferAsprintf(&attrBuf, " cpus='%s'", cpustr);
}
- virBufferAsprintf(buf, " memory='%llu'",
+ virBufferAsprintf(&attrBuf, " memory='%llu'",
virDomainNumaGetNodeMemorySize(def, i));
- virBufferAddLit(buf, " unit='KiB'");
+ virBufferAddLit(&attrBuf, " unit='KiB'");
if (memAccess)
- virBufferAsprintf(buf, " memAccess='%s'",
+ virBufferAsprintf(&attrBuf, " memAccess='%s'",
virDomainMemoryAccessTypeToString(memAccess));
if (discard)
- virBufferAsprintf(buf, " discard='%s'",
+ virBufferAsprintf(&attrBuf, " discard='%s'",
virTristateBoolTypeToString(discard));
- ndistances = def->mem_nodes[i].ndistances;
- ncaches = def->mem_nodes[i].ncaches;
- if (ndistances == 0 && ncaches == 0) {
- virBufferAddLit(buf, "/>\n");
- } else {
- size_t j;
-
- virBufferAddLit(buf, ">\n");
- virBufferAdjustIndent(buf, 2);
-
- if (ndistances) {
- virDomainNumaDistance *distances = def->mem_nodes[i].distances;
-
- virBufferAddLit(buf, "<distances>\n");
- virBufferAdjustIndent(buf, 2);
- for (j = 0; j < ndistances; j++) {
- if (distances[j].value) {
- virBufferAddLit(buf, "<sibling");
- virBufferAsprintf(buf, " id='%d'",
distances[j].cellid);
- virBufferAsprintf(buf, " value='%d'",
distances[j].value);
- virBufferAddLit(buf, "/>\n");
- }
+ if (def->mem_nodes[i].ndistances) {
+ virDomainNumaDistance *distances = def->mem_nodes[i].distances;
+
+ virBufferAddLit(&childBuf, "<distances>\n");
+ virBufferAdjustIndent(&childBuf, 2);
+ for (j = 0; j < def->mem_nodes[i].ndistances; j++) {
+ if (distances[j].value) {
+ virBufferAddLit(&childBuf, "<sibling");
+ virBufferAsprintf(&childBuf, " id='%d'",
distances[j].cellid);
+ virBufferAsprintf(&childBuf, " value='%d'",
distances[j].value);
+ virBufferAddLit(&childBuf, "/>\n");
}
- virBufferAdjustIndent(buf, -2);
- virBufferAddLit(buf, "</distances>\n");
}
+ virBufferAdjustIndent(&childBuf, -2);
+ virBufferAddLit(&childBuf, "</distances>\n");
+ }
- for (j = 0; j < ncaches; j++) {
- virDomainNumaCache *cache = &def->mem_nodes[i].caches[j];
-
- virBufferAsprintf(buf, "<cache level='%u'",
cache->level);
- if (cache->associativity) {
- virBufferAsprintf(buf, " associativity='%s'",
-
virDomainCacheAssociativityTypeToString(cache->associativity));
- }
+ for (j = 0; j < def->mem_nodes[i].ncaches; j++) {
+ virDomainNumaCache *cache = &def->mem_nodes[i].caches[j];
- if (cache->policy) {
- virBufferAsprintf(buf, " policy='%s'",
-
virDomainCachePolicyTypeToString(cache->policy));
- }
- virBufferAddLit(buf, ">\n");
+ virBufferAsprintf(&childBuf, "<cache level='%u'",
cache->level);
+ if (cache->associativity) {
+ virBufferAsprintf(&childBuf, "
associativity='%s'",
+
virDomainCacheAssociativityTypeToString(cache->associativity));
+ }
- virBufferAdjustIndent(buf, 2);
- virBufferAsprintf(buf,
- "<size value='%u'
unit='KiB'/>\n",
- cache->size);
+ if (cache->policy) {
+ virBufferAsprintf(&childBuf, " policy='%s'",
+ virDomainCachePolicyTypeToString(cache->policy));
+ }
+ virBufferAddLit(&childBuf, ">\n");
- if (cache->line) {
- virBufferAsprintf(buf,
- "<line value='%u'
unit='B'/>\n",
- cache->line);
- }
+ virBufferAdjustIndent(&childBuf, 2);
+ virBufferAsprintf(&childBuf,
+ "<size value='%u'
unit='KiB'/>\n",
+ cache->size);
- virBufferAdjustIndent(buf, -2);
- virBufferAddLit(buf, "</cache>\n");
+ if (cache->line) {
+ virBufferAsprintf(&childBuf,
+ "<line value='%u'
unit='B'/>\n",
+ cache->line);
}
- virBufferAdjustIndent(buf, -2);
- virBufferAddLit(buf, "</cell>\n");
+
+ virBufferAdjustIndent(&childBuf, -2);
+ virBufferAddLit(&childBuf, "</cache>\n");
}
+
+ virXMLFormatElement(buf, "cell", &attrBuf, &childBuf);
}
if (def->ninterconnects) {
--
2.26.3
2:44 p.m.
New subject: [PATCH 2/4] virnuma: Export virNumaGetMaxCPUs properly
This function will be used in virnumamock, shortly.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virnuma.h | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1df4b8cfe8..b7bbe46dc7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2872,6 +2872,7 @@ virNodeSuspendGetTargetMask;
virNumaGetAutoPlacementAdvice;
virNumaGetDistances;
virNumaGetHostMemoryNodeset;
+virNumaGetMaxCPUs;
virNumaGetMaxNode;
virNumaGetNodeCPUs;
virNumaGetNodeMemory;
diff --git a/src/util/virnuma.h b/src/util/virnuma.h
index 45342ecf11..bc79bff891 100644
--- a/src/util/virnuma.h
+++ b/src/util/virnuma.h
@@ -43,7 +43,7 @@ int virNumaGetNodeMemory(int node,
unsigned long long *memsize,
unsigned long long *memfree) G_GNUC_NO_INLINE;
-unsigned int virNumaGetMaxCPUs(void);
+unsigned int virNumaGetMaxCPUs(void) G_GNUC_NO_INLINE;
int virNumaGetNodeCPUs(int node, virBitmap **cpus) G_GNUC_NO_INLINE;
int virNumaNodesetToCPUset(virBitmap *nodeset,
--
2.26.3
2:44 p.m.
New subject: [PATCH 3/4] virnumamock: Allow CPU-less NUMA nodes
The original virNumaGetNodeCPUs() returns an empty virBitmap if
given NUMA node has no CPUs. But that's not how our mock behaves
- it looks under $fakesysfs/node/node$N/cpulist only to find an
empty file which is then passed to virBitmapParseUnlimited()
which threats such input as error.
Fortunately, we don't have any fake sysfs data where this path is
hit, but we might soon.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/virnumamock.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tests/virnumamock.c b/tests/virnumamock.c
index 3a203ded77..ff9c6e951d 100644
--- a/tests/virnumamock.c
+++ b/tests/virnumamock.c
@@ -172,7 +172,12 @@ virNumaGetNodeCPUs(int node, virBitmap **cpus)
SYSFS_SYSTEM_PATH, node) < 0)
return -1;
- *cpus = virBitmapParseUnlimited(cpulist);
+ if (STREQ(cpulist, "")) {
+ unsigned int max_n_cpus = virNumaGetMaxCPUs();
+ *cpus = virBitmapNew(max_n_cpus);
+ } else {
+ *cpus = virBitmapParseUnlimited(cpulist);
+ }
if (!*cpus)
goto cleanup;
--
2.26.3
2:44 p.m.
New subject: [PATCH 4/4] virCapabilitiesHostNUMAFormat: Bring variables into loops
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/capabilities.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 1dae6d38cc..915cd3149e 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -807,8 +807,6 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
virCapsHostNUMA *caps)
{
size_t i;
- size_t j;
- char *siblings;
if (!caps)
return 0;
@@ -819,6 +817,8 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
virBufferAdjustIndent(buf, 2);
for (i = 0; i < caps->cells->len; i++) {
virCapsHostNUMACell *cell = g_ptr_array_index(caps->cells, i);
+ size_t j;
+
virBufferAsprintf(buf, "<cell id='%d'>\n",
cell->num);
virBufferAdjustIndent(buf, 2);
@@ -847,10 +847,12 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
virBufferAsprintf(buf, "<cpus num='%d'>\n",
cell->ncpus);
virBufferAdjustIndent(buf, 2);
- for (j = 0; j < cell->ncpus; j++) {
+ for (j = 0; j < cell->ncpus; ++j) {
virBufferAsprintf(buf, "<cpu id='%d'",
cell->cpus[j].id);
if (cell->cpus[j].siblings) {
+ g_autofree char * siblings = NULL;
+
if (!(siblings = virBitmapFormat(cell->cpus[j].siblings)))
return -1;
@@ -860,7 +862,6 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
cell->cpus[j].die_id,
cell->cpus[j].core_id,
siblings);
- VIR_FREE(siblings);
}
virBufferAddLit(buf, "/>\n");
}
--
2.26.3
3:46 p.m.
New subject: [PATCH 4/4] virCapabilitiesHostNUMAFormat: Bring variables into loops
On a Monday in 2021, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/capabilities.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 1dae6d38cc..915cd3149e 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -807,8 +807,6 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
virCapsHostNUMA *caps)
{
size_t i;
- size_t j;
- char *siblings;
if (!caps)
return 0;
@@ -819,6 +817,8 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
virBufferAdjustIndent(buf, 2);
for (i = 0; i < caps->cells->len; i++) {
virCapsHostNUMACell *cell = g_ptr_array_index(caps->cells, i);
+ size_t j;
+
virBufferAsprintf(buf, "<cell id='%d'>\n",
cell->num);
virBufferAdjustIndent(buf, 2);
@@ -847,10 +847,12 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
virBufferAsprintf(buf, "<cpus num='%d'>\n",
cell->ncpus);
virBufferAdjustIndent(buf, 2);
- for (j = 0; j < cell->ncpus; j++) {
+ for (j = 0; j < cell->ncpus; ++j) {
No need to change the post-increment to pre-increment.
Jano
virBufferAsprintf(buf, "<cpu
id='%d'", cell->cpus[j].id);
if (cell->cpus[j].siblings) {
+ g_autofree char * siblings = NULL;
+
if (!(siblings = virBitmapFormat(cell->cpus[j].siblings)))
return -1;
3:57 p.m.
New subject: [PATCH 4/4] virCapabilitiesHostNUMAFormat: Bring variables into loops
On 5/17/21 3:46 PM, Ján Tomko wrote:
On a Monday in 2021, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/conf/capabilities.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 1dae6d38cc..915cd3149e 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -807,8 +807,6 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
> virCapsHostNUMA *caps)
> {
> size_t i;
> - size_t j;
> - char *siblings;
>
> if (!caps)
> return 0;
> @@ -819,6 +817,8 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
> virBufferAdjustIndent(buf, 2);
> for (i = 0; i < caps->cells->len; i++) {
> virCapsHostNUMACell *cell = g_ptr_array_index(caps->cells, i);
> + size_t j;
> +
> virBufferAsprintf(buf, "<cell id='%d'>\n",
cell->num);
> virBufferAdjustIndent(buf, 2);
>
> @@ -847,10 +847,12 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
>
> virBufferAsprintf(buf, "<cpus num='%d'>\n",
cell->ncpus);
> virBufferAdjustIndent(buf, 2);
> - for (j = 0; j < cell->ncpus; j++) {
> + for (j = 0; j < cell->ncpus; ++j) {
No need to change the post-increment to pre-increment.
Huh, I have no idea how that slipped in there :-)
Michal
3:46 p.m.
On a Monday in 2021, Michal Privoznik wrote:
I'm looking into NUMA code in capabilities and came up with a
couple of
cleanups. Technically, 3/4 is not needed yet, but I'll be introducing
new data to vircaps2xmltest where a NUMA node has no CPUs and that's
why the patch is needed.
Michal Prívozník (4):
numa_conf: Use virXMLFormatElement() in virDomainNumaDefFormatXML
virnuma: Export virNumaGetMaxCPUs properly
virnumamock: Allow CPU-less NUMA nodes
virCapabilitiesHostNUMAFormat: Bring variables into loops
src/conf/capabilities.c | 9 ++--
src/conf/numa_conf.c | 107 ++++++++++++++++++---------------------
src/libvirt_private.syms | 1 +
src/util/virnuma.h | 2 +-
tests/virnumamock.c | 7 ++-
5 files changed, 61 insertions(+), 65 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1222
days inactive
1222
days old
7 comments
3 participants
participants (3)
-
Ján Tomko -
Michal Privoznik -
Michal Prívozník