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

2015-07-13 Thread 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 k...@linux.vnet.ibm.com
 
 
 ---
  src/nodeinfo.c |   11 +++
  1 file changed, 11 insertions(+)
 

So along with a test that Andrea Bologani generated :

http://www.redhat.com/archives/libvir-list/2015-July/msg00517.html

and the sysfs_path adjustments I made:

http://www.redhat.com/archives/libvir-list/2015-July/msg00278.html

This is now pushed.

Thanks,

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@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list
 

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


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

2015-07-06 Thread 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 k...@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@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list
 

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


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

2015-06-26 Thread Kothapally Madhu Pavan
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 k...@linux.vnet.ibm.com


---
 src/nodeinfo.c |   11 +++
 1 file changed, 11 insertions(+)

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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list