[PATCH 0/2] virsh: Fix fallback code path for vcpuinfo

Peter Krempa (2): virshDomainGetVcpuBitmap: Return bitmap when taking the fallback path virshDomainGetVcpuBitmap: Refactor cleanup tools/virsh-domain.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) -- 2.31.1

In case the specific VCPU states are not present in the XML we were taking a fallback code path just noting that all cpus of the VM are enabled. This was broken by a mistake in a recent refactor where a 'goto cleanup' was mistakenly replaced by a 'return NULL'. This broke reporting of cpus and also caused a memory leak. Return the fallback cpu map. Fixes: bd1f40fe7d4 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2004429 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index e5bd1fdd75..f5a3e1accc 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6710,7 +6710,7 @@ virshDomainGetVcpuBitmap(vshControl *ctl, for (i = 0; i < curvcpus; i++) ignore_value(virBitmapSetBit(ret, i)); - return NULL; + return ret; } for (i = 0; i < nnodes; i++) { -- 2.31.1

Rename the temp variable that is being returned and use automatic pointer clearing for it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f5a3e1accc..4d328d2174 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6677,7 +6677,7 @@ virshDomainGetVcpuBitmap(vshControl *ctl, bool inactive) { unsigned int flags = 0; - virBitmap *ret = NULL; + g_autoptr(virBitmap) cpumap = NULL; g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; g_autofree xmlNodePtr *nodes = NULL; @@ -6703,14 +6703,14 @@ virshDomainGetVcpuBitmap(vshControl *ctl, if (curvcpus == 0) curvcpus = maxvcpus; - ret = virBitmapNew(maxvcpus); + cpumap = virBitmapNew(maxvcpus); if ((nnodes = virXPathNodeSet("/domain/vcpus/vcpu", ctxt, &nodes)) <= 0) { /* if the specific vcpu state is missing provide a fallback */ for (i = 0; i < curvcpus; i++) - ignore_value(virBitmapSetBit(ret, i)); + ignore_value(virBitmapSetBit(cpumap, i)); - return ret; + return g_steal_pointer(&cpumap); } for (i = 0; i < nnodes; i++) { @@ -6723,16 +6723,15 @@ virshDomainGetVcpuBitmap(vshControl *ctl, continue; if (STREQ(online, "yes")) - ignore_value(virBitmapSetBit(ret, vcpuid)); + ignore_value(virBitmapSetBit(cpumap, vcpuid)); } - if (virBitmapCountBits(ret) != curvcpus) { + if (virBitmapCountBits(cpumap) != curvcpus) { vshError(ctl, "%s", _("Failed to retrieve vcpu state bitmap")); - virBitmapFree(ret); return NULL; } - return ret; + return g_steal_pointer(&cpumap); } -- 2.31.1

On Wed, Sep 15, 2021 at 15:21:10 +0200, Peter Krempa wrote:
Peter Krempa (2): virshDomainGetVcpuBitmap: Return bitmap when taking the fallback path virshDomainGetVcpuBitmap: Refactor cleanup
tools/virsh-domain.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

ACK both patches Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com> On Wed, Sep 15, 2021 at 8:21 AM Peter Krempa <pkrempa@redhat.com> wrote:
Peter Krempa (2): virshDomainGetVcpuBitmap: Return bitmap when taking the fallback path virshDomainGetVcpuBitmap: Refactor cleanup
tools/virsh-domain.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
-- 2.31.1

On a Wednesday in 2021, Peter Krempa wrote:
Peter Krempa (2): virshDomainGetVcpuBitmap: Return bitmap when taking the fallback path virshDomainGetVcpuBitmap: Refactor cleanup
tools/virsh-domain.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (5)
-
Jiri Denemark
-
Jonathon Jongsma
-
Ján Tomko
-
Pavel Hrdina
-
Peter Krempa