1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] numa: avoid failure in nodememstats on non-NUMA systems

Viktor Mihajlovski posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1506089543-21288-1-git-send-email-mihajlov@linux.vnet.ibm.com
src/util/virhostmem.c | 8 ++++++++
1 file changed, 8 insertions(+)
[libvirt] [PATCH] numa: avoid failure in nodememstats on non-NUMA systems
Posted by Viktor Mihajlovski 6 years, 6 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] numa: avoid failure in nodememstats on non-NUMA systems
Posted by Boris Fiuczynski 6 years, 6 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] numa: avoid failure in nodememstats on non-NUMA systems
Posted by Boris Fiuczynski 6 years, 6 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] numa: avoid failure in nodememstats on non-NUMA systems
Posted by John Ferlan 6 years, 5 months ago 

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;
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] numa: avoid failure in nodememstats on non-NUMA systems
Posted by Viktor Mihajlovski 6 years, 5 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.