[libvirt] [PATCH v3] nodeinfo: fix to parse present cpus rather than possible cpus
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@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; }
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@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
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@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
participants (2)
-
John Ferlan -
Kothapally Madhu Pavan