[libvirt] [PATCH v3] nodeinfo: fix to parse present cpus rather than possible cpus

[John Ferlan]

On 06/26/2015 06:27 PM, Kothapally Madhu Pavan wrote:
> Currently we are parsing all the possible cpus to get the
> nodeinfo. This fix will perform a check for present cpus
> before parsing.
> 
> Signed-off-by: Kothapally Madhu Pavan <kmp at linux.vnet.ibm.com>
> 
> 
> ---
>  src/nodeinfo.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 

What you described in your response regarding the directory structure
would be a lot easier to visualize if you modified the nodeinfotest in
order to provide an "example". Of course as I found out during this pass
- that won't necessarily do what is expected (yet) either.

Also looks like I mistyped the virBitmapIsBitSet as virBitmapIsSet when
I responded to your prior patch - it was freehand.

So even changing that in this patch, there's an issue with this patch
and existing tests.  Did you run 'make check'?  Did it succeed?  Mine
failed with your patch, but that's partially because I have a 2 CPU 2
core laptop and any nodeinfotest needing data from cpu5 and higher will
fail. Perhaps that may have succeeded if nodeinfo.c was able to handle
non root based paths (e.g. an update to all code paths to pass a "path"
and if NULL, use SYSFS_SYSTEM_PATH else use the "passed path").

If you look for the "tests/*nodeinfo*/linux-test*/cpu/present" files,
you will note only one exists... and since it's presence is essentially
the basis for your check, a modification of the test is necessary in
order to "mimic" the environment since right now it will use whatever is
on the host.

That's a fairly invasive change to the nodeinfo.c - something I started
to see if it's possible. Once in place, it'll be much easier for you to
add a test to exhibit the issue you're trying to fix/resolve.

John


> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> index 2fafe2d..5689c9b 100644
> --- a/src/nodeinfo.c
> +++ b/src/nodeinfo.c
> @@ -43,6 +43,7 @@
>  #include "c-ctype.h"
>  #include "viralloc.h"
>  #include "nodeinfopriv.h"
> +#include "nodeinfo.h"
>  #include "physmem.h"
>  #include "virerror.h"
>  #include "count-one-bits.h"
> @@ -418,6 +419,7 @@ virNodeParseNode(const char *node,
>      int processors = 0;
>      DIR *cpudir = NULL;
>      struct dirent *cpudirent = NULL;
> +    virBitmapPtr present_cpumap = NULL;
>      int sock_max = 0;
>      cpu_set_t sock_map;
>      int sock;
> @@ -438,12 +440,17 @@ virNodeParseNode(const char *node,
>          goto cleanup;
>      }
>  
> +    present_cpumap = nodeGetPresentCPUBitmap();
> +
>      /* enumerate sockets in the node */
>      CPU_ZERO(&sock_map);
>      while ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0) {
>          if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
>              continue;
>  
> +        if (present_cpumap && !(virBitmapIsSet(present_cpumap, cpu)))
> +            continue;
> +
>          if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
>              goto cleanup;
>  
> @@ -477,6 +484,9 @@ virNodeParseNode(const char *node,
>          if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
>              continue;
>  
> +        if (present_cpumap && !(virBitmapIsSet(present_cpumap, cpu)))
> +            continue;
> +
>          if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
>              goto cleanup;
>  
> @@ -537,6 +547,7 @@ virNodeParseNode(const char *node,
>          ret = -1;
>      }
>      VIR_FREE(core_maps);
> +    virBitmapFree(present_cpumap);
>  
>      return ret;
>  }
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>