[PATCH] qemuDomainVcpuValidateConfig: Properly initialize 'firstcpu' variable

Peter Krempa posted 1 patch 4 years ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/5239933759f7411fc3fb98ede12c00af9b23902a.1584467434.git.pkrempa@redhat.com
src/qemu/qemu_hotplug.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
[PATCH] qemuDomainVcpuValidateConfig: Properly initialize 'firstcpu' variable
Posted by Peter Krempa 4 years ago
The loop which checks whether the vcpus are in proper configuration for
the requested hot(un)plug skips the first modified vcpu. This means
that 'firstvcpu' which is used to print the error message in case the
configuration is not suitable would never point to the first modified
vcpu.

In cases such as:

  <vcpu placement='auto' current='5'>8</vcpu>
  <vcpus>
    <vcpu id='0' enabled='yes' hotpluggable='no'/>
    <vcpu id='1' enabled='yes' hotpluggable='no'/>
    <vcpu id='2' enabled='yes' hotpluggable='no'/>
    <vcpu id='3' enabled='yes' hotpluggable='no'/>
    <vcpu id='4' enabled='yes' hotpluggable='no'/>
    <vcpu id='5' enabled='no' hotpluggable='yes'/>
    <vcpu id='6' enabled='no' hotpluggable='yes'/>
    <vcpu id='7' enabled='no' hotpluggable='yes'/>
  </vcpus>

 # virsh setvcpu --config --disable  upstream 1
 error: invalid argument: vcpu '-1' can't be modified as it is followed by non-hotpluggable online vcpus

After this fix the proper vcpu is reported in the error message:

 # virsh setvcpu --config --disable  upstream 1
 error: invalid argument: vcpu '1' can't be modified as it is followed by non-hotpluggable online vcpu

https://bugzilla.redhat.com/show_bug.cgi?id=1611061

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_hotplug.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 47069be900..a76df64a7b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -6478,18 +6478,16 @@ qemuDomainVcpuValidateConfig(virDomainDefPtr def,
         return -1;
     }

+    firstvcpu = virBitmapNextSetBit(map, -1);
+
     /* non-hotpluggable vcpus need to stay clustered starting from vcpu 0 */
-    for (next = virBitmapNextSetBit(map, -1) + 1; next < maxvcpus; next++) {
+    for (next = firstvcpu + 1; next < maxvcpus; next++) {
         if (!(vcpu = virDomainDefGetVcpu(def, next)))
             continue;

         /* skip vcpus being modified */
-        if (virBitmapIsBitSet(map, next)) {
-            if (firstvcpu < 0)
-                firstvcpu = next;
-
+        if (virBitmapIsBitSet(map, next))
             continue;
-        }

         if (vcpu->online && vcpu->hotpluggable == VIR_TRISTATE_BOOL_NO) {
             virReportError(VIR_ERR_INVALID_ARG,
-- 
2.24.1

Re: [PATCH] qemuDomainVcpuValidateConfig: Properly initialize 'firstcpu' variable
Posted by Daniel Henrique Barboza 4 years ago

On 3/17/20 2:50 PM, Peter Krempa wrote:
> The loop which checks whether the vcpus are in proper configuration for
> the requested hot(un)plug skips the first modified vcpu. This means
> that 'firstvcpu' which is used to print the error message in case the
> configuration is not suitable would never point to the first modified
> vcpu.
> 
> In cases such as:
> 
>    <vcpu placement='auto' current='5'>8</vcpu>
>    <vcpus>
>      <vcpu id='0' enabled='yes' hotpluggable='no'/>
>      <vcpu id='1' enabled='yes' hotpluggable='no'/>
>      <vcpu id='2' enabled='yes' hotpluggable='no'/>
>      <vcpu id='3' enabled='yes' hotpluggable='no'/>
>      <vcpu id='4' enabled='yes' hotpluggable='no'/>
>      <vcpu id='5' enabled='no' hotpluggable='yes'/>
>      <vcpu id='6' enabled='no' hotpluggable='yes'/>
>      <vcpu id='7' enabled='no' hotpluggable='yes'/>
>    </vcpus>
> 
>   # virsh setvcpu --config --disable  upstream 1
>   error: invalid argument: vcpu '-1' can't be modified as it is followed by non-hotpluggable online vcpus
> 
> After this fix the proper vcpu is reported in the error message:
> 
>   # virsh setvcpu --config --disable  upstream 1
>   error: invalid argument: vcpu '1' can't be modified as it is followed by non-hotpluggable online vcpu
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1611061
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>