[libvirt] [PATCH 4/4] Fix handling of sparse NUMA topologies

[Daniel Veillard]

On Tue, Aug 17, 2010 at 11:16:35AM -0400, Daniel P. Berrange wrote:
> When finding a sparse NUMA topology, libnuma will return ENOENT
> the first time it is invoked. On subsequent invocations it
> will return success, but with an all-1's CPU mask. Check for
> this, to avoid polluting the capabilities XML with 4096 bogus
> CPUs
> 
> * src/nodeinfo.c: Check for all-1s CPU mask
> ---
>  src/nodeinfo.c |   14 +++++++++++++-
>  1 files changed, 13 insertions(+), 1 deletions(-)
> 
> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> index 57db085..65eeb24 100644
> --- a/src/nodeinfo.c
> +++ b/src/nodeinfo.c
> @@ -361,6 +361,7 @@ nodeCapsInitNUMA(virCapsPtr caps)
>  {
>      int n;
>      unsigned long *mask = NULL;
> +    unsigned long *allonesmask = NULL;
>      int *cpus = NULL;
>      int ret = -1;
>      int max_n_cpus = NUMA_MAX_N_CPUS;
> @@ -371,13 +372,23 @@ nodeCapsInitNUMA(virCapsPtr caps)
>      int mask_n_bytes = max_n_cpus / 8;
>      if (VIR_ALLOC_N(mask, mask_n_bytes / sizeof *mask) < 0)
>          goto cleanup;
> +    if (VIR_ALLOC_N(allonesmask, mask_n_bytes / sizeof *mask) < 0)
> +        goto cleanup;
> +    memset(allonesmask, 0xff, mask_n_bytes);
>  
>      for (n = 0 ; n <= numa_max_node() ; n++) {
>          int i;
>          int ncpus;
> +        /* The first time this returns -1, ENOENT if node doesn't exist... */
>          if (numa_node_to_cpus(n, mask, mask_n_bytes) < 0) {
>              VIR_WARN("NUMA topology for cell %d of %d not available, ignoring",
> -                     n, numa_max_node());
> +                     n, numa_max_node()+1);
> +            continue;
> +        }
> +        /* second, third... times it returns an all-1's mask */
> +        if (memcmp(mask, allonesmask, mask_n_bytes) == 0) {
> +            VIR_DEBUG("NUMA topology for cell %d of %d is all ones, ignoring",
> +                      n, numa_max_node()+1);
>              continue;
>          }

  it's always a bit bizarre to add such code to work around this kind of
breakage in dependancies's code ... shouldn't we check first that we got
the ENOENT before doing that compare test ? Unless we are sure an all
set of 1 is really unfeasible ?

> @@ -406,6 +417,7 @@ nodeCapsInitNUMA(virCapsPtr caps)
>  cleanup:
>      VIR_FREE(cpus);
>      VIR_FREE(mask);
> +    VIR_FREE(allonesmask);
>      return ret;
>  }
>  

  ACK, it's weird, but not really our fault,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/