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

[John Ferlan]

On 06/10/2015 02:49 AM, 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 |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> index 2fafe2d..9e6684f 100644
> --- a/src/nodeinfo.c
> +++ b/src/nodeinfo.c
> @@ -57,6 +57,7 @@
>  #define VIR_FROM_THIS VIR_FROM_NONE
>  
>  VIR_LOG_INIT("nodeinfo");
> +virBitmapPtr nodeGetPresentCPUBitmap(void);
>  
>  #if defined(__FreeBSD__) || defined(__APPLE__)
>  static int
> @@ -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();
> +

And what happens if the call fails?  returns NULL?

>      /* 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 (!(virBitmapIsBitSet(present_cpumap, cpu)))
> +            continue;
> +

Call doesn't check for !present_cpumap

>          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 (!(virBitmapIsBitSet(present_cpumap, cpu)))
> +            continue;
> +

Call doesn't check for !present_cpumap

>          if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
>              goto cleanup;
>  
> 

If you do get a 'present_cpumap' you don't virBitmapFree it in cleanup
leading to resource leak.

Looking forward to a v2

John
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>