[libvirt] [PATCH] numa: avoid failure in nodememstats on non-NUMA systems
libvirt reports a fake NUMA topology in virConnectGetCapabilities even if built without numactl support. The fake NUMA topology consists of a single cell representing the host's cpu and memory resources. Currently this is the case for ARM and s390[x] RPM builds. A client iterating over NUMA cells obtained via virConnectGetCapabilities and invoking virNodeGetMemoryStats on them will see an internal failure "NUMA isn't available on this host". An example for such a client is VDSM. Since the intention seems to be that libvirt always reports at least a single cell it is necessary to return "fake" node memory statistics matching the previously reported fake cell in case NUMA isn't supported on the system. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/util/virhostmem.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/util/virhostmem.c b/src/util/virhostmem.c index a9ba278..fa04a37 100644 --- a/src/util/virhostmem.c +++ b/src/util/virhostmem.c @@ -267,6 +267,14 @@ virHostMemGetStats(int cellNum ATTRIBUTE_UNUSED, FILE *meminfo; int max_node; + /* + * Even if built without numactl, libvirt claims + * to have a one-cells NUMA topology. In such a + * case return the statistics for the entire host. + */ + if (!virNumaIsAvailable() && cellNum == 0) + cellNum = VIR_NODE_MEMORY_STATS_ALL_CELLS; + if (cellNum == VIR_NODE_MEMORY_STATS_ALL_CELLS) { if (VIR_STRDUP(meminfo_path, MEMINFO_PATH) < 0) return -1; -- 1.9.1
Looks good to me. On 09/22/2017 04:12 PM, Viktor Mihajlovski wrote:
libvirt reports a fake NUMA topology in virConnectGetCapabilities even if built without numactl support. The fake NUMA topology consists of a single cell representing the host's cpu and memory resources. Currently this is the case for ARM and s390[x] RPM builds.
A client iterating over NUMA cells obtained via virConnectGetCapabilities and invoking virNodeGetMemoryStats on them will see an internal failure "NUMA isn't available on this host". An example for such a client is VDSM.
Since the intention seems to be that libvirt always reports at least a single cell it is necessary to return "fake" node memory statistics matching the previously reported fake cell in case NUMA isn't supported on the system.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/util/virhostmem.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/util/virhostmem.c b/src/util/virhostmem.c index a9ba278..fa04a37 100644 --- a/src/util/virhostmem.c +++ b/src/util/virhostmem.c @@ -267,6 +267,14 @@ virHostMemGetStats(int cellNum ATTRIBUTE_UNUSED, FILE *meminfo; int max_node;
+ /* + * Even if built without numactl, libvirt claims + * to have a one-cells NUMA topology. In such a + * case return the statistics for the entire host. + */ + if (!virNumaIsAvailable() && cellNum == 0) + cellNum = VIR_NODE_MEMORY_STATS_ALL_CELLS; + if (cellNum == VIR_NODE_MEMORY_STATS_ALL_CELLS) { if (VIR_STRDUP(meminfo_path, MEMINFO_PATH) < 0) return -1;
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Ping... and Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> On 10/09/2017 02:57 PM, Boris Fiuczynski wrote:
Looks good to me.
On 09/22/2017 04:12 PM, Viktor Mihajlovski wrote:
libvirt reports a fake NUMA topology in virConnectGetCapabilities even if built without numactl support. The fake NUMA topology consists of a single cell representing the host's cpu and memory resources. Currently this is the case for ARM and s390[x] RPM builds.
A client iterating over NUMA cells obtained via virConnectGetCapabilities and invoking virNodeGetMemoryStats on them will see an internal failure "NUMA isn't available on this host". An example for such a client is VDSM.
Since the intention seems to be that libvirt always reports at least a single cell it is necessary to return "fake" node memory statistics matching the previously reported fake cell in case NUMA isn't supported on the system.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/util/virhostmem.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/util/virhostmem.c b/src/util/virhostmem.c index a9ba278..fa04a37 100644 --- a/src/util/virhostmem.c +++ b/src/util/virhostmem.c @@ -267,6 +267,14 @@ virHostMemGetStats(int cellNum ATTRIBUTE_UNUSED, FILE *meminfo; int max_node;
+ /* + * Even if built without numactl, libvirt claims + * to have a one-cells NUMA topology. In such a + * case return the statistics for the entire host. + */ + if (!virNumaIsAvailable() && cellNum == 0) + cellNum = VIR_NODE_MEMORY_STATS_ALL_CELLS; + if (cellNum == VIR_NODE_MEMORY_STATS_ALL_CELLS) { if (VIR_STRDUP(meminfo_path, MEMINFO_PATH) < 0) return -1;
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 09/22/2017 10:12 AM, Viktor Mihajlovski wrote:
libvirt reports a fake NUMA topology in virConnectGetCapabilities even if built without numactl support. The fake NUMA topology consists of a single cell representing the host's cpu and memory resources. Currently this is the case for ARM and s390[x] RPM builds.
A client iterating over NUMA cells obtained via virConnectGetCapabilities and invoking virNodeGetMemoryStats on them will see an internal failure "NUMA isn't available on this host". An example for such a client is VDSM.
Message is from virNumaGetMaxNode call...
Since the intention seems to be that libvirt always reports at least a single cell it is necessary to return "fake" node memory statistics matching the previously reported fake cell in case NUMA isn't supported on the system.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/util/virhostmem.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/util/virhostmem.c b/src/util/virhostmem.c index a9ba278..fa04a37 100644 --- a/src/util/virhostmem.c +++ b/src/util/virhostmem.c @@ -267,6 +267,14 @@ virHostMemGetStats(int cellNum ATTRIBUTE_UNUSED, FILE *meminfo; int max_node;
+ /*
^ A 'git am' told me there was an extra blank space after the * ;-)
+ * Even if built without numactl, libvirt claims + * to have a one-cells NUMA topology. In such a + * case return the statistics for the entire host. + */ + if (!virNumaIsAvailable() && cellNum == 0) + cellNum = VIR_NODE_MEMORY_STATS_ALL_CELLS; +
I'm NUMA challenged, but this certainly seems reasonable... and it appears you have an ACK from Boris anyway... So I'll give it until tomorrow for anyone else to throw up their hands and say this isn't right. ;-)... if no one does, then I'll push with the space fix... Reviewed-by: John Ferlan <jferlan@redhat.com> John
if (cellNum == VIR_NODE_MEMORY_STATS_ALL_CELLS) { if (VIR_STRDUP(meminfo_path, MEMINFO_PATH) < 0) return -1;
On 07.11.2017 21:53, John Ferlan wrote: [...] >> + /* > ^ > A 'git am' told me there was an extra blank space after the * ;-) > >> + * Even if built without numactl, libvirt claims >> + * to have a one-cells NUMA topology. In such a >> + * case return the statistics for the entire host. >> + */ >> + if (!virNumaIsAvailable() && cellNum == 0) >> + cellNum = VIR_NODE_MEMORY_STATS_ALL_CELLS; >> + > > I'm NUMA challenged, but this certainly seems reasonable... and it > appears you have an ACK from Boris anyway... So I'll give it until > tomorrow for anyone else to throw up their hands and say this isn't > right. ;-)... if no one does, then I'll push with the space fix... thanks for fixing and pushing [...] -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski
participants (3)
-
Boris Fiuczynski -
John Ferlan -
Viktor Mihajlovski