From 093eed7360101a607ba024ccdbe54098322ce0f3 Mon Sep 17 00:00:00 2001
From: Lin Ma <lma@suse.com>
Date: Thu, 22 Apr 2021 18:38:19 +0800
Subject: [PATCH] virsh: Fix completion logic to guestvcpus command

In case of non-continuous vCPU topology, We can't infer the bitmap size
from the combination of onlineVcpuStr and nvcpus.
We should use virBitmapParseUnlimited here instead of virBitmapParse due
to the bitmap size is unknown.

e.g.:

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

 # virsh guestvcpus --domain VM
vcpus          : 0-5
online         : 0-5
offlinable     : 1-5

 # virsh setvcpu --domain VM --disable --vcpulist 2

 # virsh guestvcpus --domain VM --disable --cpulist 4,5

 # virsh guestvcpus --domain VM
vcpus          : 0-1,3-5
online         : 0-1,3
offlinable     : 1,3-5

Before:
 # virsh guestvcpus --domain VM --enable --cpulist <TAB><TAB>
2  4

After:
 # virsh guestvcpus --domain VM --enable --cpulist <TAB><TAB>
4  5

Signed-off-by: Lin Ma <lma@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
---
 tools/virsh-completer-domain.c | 57 +++++++++++++++++++++++++---------
 1 file changed, 42 insertions(+), 15 deletions(-)

diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index 0d087278ac..1f72b856b4 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -624,36 +624,63 @@ virshDomainVcpulistViaAgentCompleter(vshControl *ctl,
             cpulist[i] = g_strdup_printf("%zu", i);
     } else {
         g_autofree char *onlineVcpuStr = NULL;
-        g_autofree unsigned char *vcpumap = NULL;
-        g_autoptr(virBitmap) vcpus = NULL;
-        size_t offset = 0;
+        g_autofree char *offlinableVcpuStr = NULL;
+        g_autofree unsigned char *onlineVcpumap = NULL;
+        g_autofree unsigned char *offlinableVcpumap = NULL;
+        g_autoptr(virBitmap) onlineVcpus = NULL;
+        g_autoptr(virBitmap) offlinableVcpus = NULL;
+        size_t j = 0;
+        int lastcpu;
         int dummy;
 
         if (virDomainGetGuestVcpus(dom, &params, &nparams, 0) < 0)
             goto cleanup;
 
         onlineVcpuStr = vshGetTypedParamValue(ctl, &params[1]);
-        if (virBitmapParse(onlineVcpuStr, &vcpus, nvcpus) < 0)
+        if (!(onlineVcpus = virBitmapParseUnlimited(onlineVcpuStr)))
             goto cleanup;
 
-        if (virBitmapToData(vcpus, &vcpumap, &dummy) < 0)
+        if (virBitmapToData(onlineVcpus, &onlineVcpumap, &dummy) < 0)
             goto cleanup;
 
         if (enable) {
-            cpulist = g_new0(char *, nvcpus - virBitmapCountBits(vcpus) + 1);
-            for (i = 0; i < nvcpus; i++) {
-                if (VIR_CPU_USED(vcpumap, i) != 0)
-                    continue;
+            offlinableVcpuStr = vshGetTypedParamValue(ctl, &params[2]);
 
-                cpulist[offset++] = g_strdup_printf("%zu", i);
+            if (!(offlinableVcpus = virBitmapParseUnlimited(offlinableVcpuStr)))
+                goto cleanup;
+
+            if (virBitmapToData(offlinableVcpus, &offlinableVcpumap, &dummy) < 0)
+                goto cleanup;
+
+            lastcpu = virBitmapLastSetBit(offlinableVcpus);
+            cpulist = g_new0(char *, nvcpus - virBitmapCountBits(onlineVcpus) + 1);
+            for (i = 0; i < nvcpus - virBitmapCountBits(onlineVcpus); i++) {
+                while (j <= lastcpu) {
+                    if (VIR_CPU_USED(onlineVcpumap, j) != 0 ||
+                        VIR_CPU_USED(offlinableVcpumap, j) == 0) {
+                        j += 1;
+                        continue;
+                    } else {
+                        break;
+                    }
+                }
+
+                cpulist[i] = g_strdup_printf("%zu", j++);
             }
         } else if (disable) {
-            cpulist = g_new0(char *, virBitmapCountBits(vcpus) + 1);
-            for (i = 0; i < nvcpus; i++) {
-                if (VIR_CPU_USED(vcpumap, i) == 0)
-                    continue;
+            lastcpu = virBitmapLastSetBit(onlineVcpus);
+            cpulist = g_new0(char *, virBitmapCountBits(onlineVcpus) + 1);
+            for (i = 0; i < virBitmapCountBits(onlineVcpus); i++) {
+                while (j <= lastcpu) {
+                    if (VIR_CPU_USED(onlineVcpumap, j) == 0) {
+                        j += 1;
+                        continue;
+                    } else {
+                        break;
+                    }
+                }
 
-                cpulist[offset++] = g_strdup_printf("%zu", i);
+                cpulist[i] = g_strdup_printf("%zu", j++);
             }
         }
     }