[libvirt] [PATCH] Regression: Report correct CPUs present on executing virsh nodecpumap

Recent changes to virbitmap.c file created a regression where on executing the virsh nodecpumap command, the number of CPUs present was shown as (last cpu online id + 1). This patch fixes the issue. Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com> --- src/Makefile.am | 2 ++ src/util/virbitmap.c | 10 ++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index f95f30e2f..96c541c9c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -3303,6 +3303,8 @@ libvirt_nss_la_SOURCES = \ util/viratomic.h \ util/virbitmap.c \ util/virbitmap.h \ + util/virhostcpu.c \ + util/virhostcpu.h \ util/virbuffer.c \ util/virbuffer.h \ util/vircommand.c \ diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index eac63d997..dc427f430 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -37,6 +37,7 @@ #include "count-one-bits.h" #include "virstring.h" #include "virerror.h" +#include "virhostcpu.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -565,9 +566,14 @@ virBitmapParseUnlimited(const char *str) const char *cur = str; char *tmp; size_t i; - int start, last; + int start, last, bitmapSize; + + bitmapSize = virHostCPUGetCount(); + + if (bitmapSize < 0) + return NULL; - if (!(bitmap = virBitmapNewEmpty())) + if (!(bitmap = virBitmapNew(bitmapSize))) return NULL; if (!str) -- 2.11.0

On Wed, May 24, 2017 at 07:46:00PM +0530, Nitesh Konkar wrote:
Recent changes to virbitmap.c file created a regression where on executing the virsh nodecpumap command, the number of CPUs present was shown as (last cpu online id + 1). This patch fixes the issue.
Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com> --- src/Makefile.am | 2 ++ src/util/virbitmap.c | 10 ++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index eac63d997..dc427f430 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -37,6 +37,7 @@ #include "count-one-bits.h" #include "virstring.h" #include "virerror.h" +#include "virhostcpu.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -565,9 +566,14 @@ virBitmapParseUnlimited(const char *str) const char *cur = str; char *tmp; size_t i; - int start, last; + int start, last, bitmapSize; + + bitmapSize = virHostCPUGetCount();
NACK. This function should be able to parse any bitmap, regardless of how many CPUs the host has. Jan

On Wed, May 24, 2017 at 8:09 PM, Ján Tomko <jtomko@redhat.com> wrote:
On Wed, May 24, 2017 at 07:46:00PM +0530, Nitesh Konkar wrote:
Recent changes to virbitmap.c file created a regression where on executing the virsh nodecpumap command, the number of CPUs present was shown as (last cpu online id + 1). This patch fixes the issue.
Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com> --- src/Makefile.am | 2 ++ src/util/virbitmap.c | 10 ++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index eac63d997..dc427f430 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -37,6 +37,7 @@ #include "count-one-bits.h" #include "virstring.h" #include "virerror.h" +#include "virhostcpu.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -565,9 +566,14 @@ virBitmapParseUnlimited(const char *str) const char *cur = str; char *tmp; size_t i; - int start, last; + int start, last, bitmapSize; + + bitmapSize = virHostCPUGetCount();
NACK.
This function should be able to parse any bitmap, regardless of how many CPUs the host has.
Jan
Hi Jan, However, currently we get the following output, which is invalid. # virsh nodecpumap CPUs present: 73 <--- should be 80. CPUs online: 10 CPU map: y-------y-------y-------y-------y-------y-------y-------y-------y-------y # lscpu Architecture: ppc64le Byte Order: Little Endian CPU(s): 80 On-line CPU(s) list: 0,8,16,24,32,40,48,56,64,72 Off-line CPU(s) list: 1-7,9-15,17-23,25-31,33-39,41-47,49-55,57-63,65-71,73-79 Thread(s) per core: 1 Core(s) per socket: 5 Socket(s): 2 NUMA node(s): 2 Model: 2.1 (pvr 004b 0201) Model name: POWER8E (raw), altivec supported L1d cache: 64K L1i cache: 32K L2 cache: 512K L3 cache: 8192K NUMA node0 CPU(s): 0,8,16,24,32 NUMA node1 CPU(s): 40,48,56,64,72 # ls /sys/devices/system/cpu | grep cpu[0-9] | wc -l 80 Thanks, Nitesh.

On Wed, May 24, 2017 at 22:05:54 +0530, Nitesh Konkar wrote:
On Wed, May 24, 2017 at 8:09 PM, Ján Tomko <jtomko@redhat.com> wrote:
On Wed, May 24, 2017 at 07:46:00PM +0530, Nitesh Konkar wrote:
Recent changes to virbitmap.c file created a regression where on executing the virsh nodecpumap command, the number of CPUs present was shown as (last cpu online id + 1). This patch fixes the issue.
Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com> --- src/Makefile.am | 2 ++ src/util/virbitmap.c | 10 ++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-)
[...]
@@ -565,9 +566,14 @@ virBitmapParseUnlimited(const char *str) const char *cur = str; char *tmp; size_t i; - int start, last; + int start, last, bitmapSize; + + bitmapSize = virHostCPUGetCount();
NACK.
This function should be able to parse any bitmap, regardless of how many CPUs the host has.
Jan
Hi Jan,
However, currently we get the following output, which is invalid.
# virsh nodecpumap CPUs present: 73 <--- should be 80. CPUs online: 10 CPU map: y-------y-------y-------y-------y-------y-------y-------y-------y-------y
Yes, that is true, but your fix is wrong.

On Wed, May 24, 2017 at 22:05:54 +0530, Nitesh Konkar wrote:
On Wed, May 24, 2017 at 8:09 PM, Ján Tomko <jtomko@redhat.com> wrote:
On Wed, May 24, 2017 at 07:46:00PM +0530, Nitesh Konkar wrote:
Recent changes to virbitmap.c file created a regression where on executing the virsh nodecpumap command, the number of CPUs present was shown as (last cpu online id + 1). This patch fixes the issue.
Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com> --- src/Makefile.am | 2 ++ src/util/virbitmap.c | 10 ++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-)
[...]
However, currently we get the following output, which is invalid.
# virsh nodecpumap CPUs present: 73 <--- should be 80. CPUs online: 10 CPU map: y-------y-------y-------y-------y-------y-------y-------y-------y-------y
# lscpu Architecture: ppc64le Byte Order: Little Endian CPU(s): 80 On-line CPU(s) list: 0,8,16,24,32,40,48,56,64,72 Off-line CPU(s) list: 1-7,9-15,17-23,25-31,33-39,41-47,49-55,57-63,65-71,73-79 Thread(s) per core: 1 Core(s) per socket: 5 Socket(s): 2 NUMA node(s): 2 Model: 2.1 (pvr 004b 0201) Model name: POWER8E (raw), altivec supported L1d cache: 64K L1i cache: 32K L2 cache: 512K L3 cache: 8192K NUMA node0 CPU(s): 0,8,16,24,32 NUMA node1 CPU(s): 40,48,56,64,72
# ls /sys/devices/system/cpu | grep cpu[0-9] | wc -l 80
This should be the proper fix: diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index aa9cfeac2..6d7e8b4f4 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1093,7 +1093,7 @@ virHostCPUGetMap(unsigned char **cpumap, if (online) *online = virBitmapCountBits(cpus); - ret = virBitmapSize(cpus); + ret = virHostCPUParseCountLinux(); cleanup: if (ret < 0 && cpumap)
participants (3)
-
Ján Tomko
-
Nitesh Konkar
-
Peter Krempa