[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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




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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]