[libvirt] [PATCH 0/2] virsh completer fixes
Lin Ma (2): virsh: Add mountpoint completion to domfsfreeze/domfsthaw command virsh: Fix completion logic to guestvcpus command tools/virsh-completer-domain.c | 104 +++++++++++++++++++++++++++------ tools/virsh-completer-domain.h | 4 ++ tools/virsh-domain.c | 2 + 3 files changed, 93 insertions(+), 17 deletions(-) -- 2.26.2
Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-completer-domain.c | 43 ++++++++++++++++++++++++++++++++++ tools/virsh-completer-domain.h | 4 ++++ tools/virsh-domain.c | 2 ++ 3 files changed, 49 insertions(+) diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c index 99b36a2f9f..f56dc40125 100644 --- a/tools/virsh-completer-domain.c +++ b/tools/virsh-completer-domain.c @@ -872,3 +872,46 @@ virshKeycodeNameCompleter(vshControl *ctl, return g_steal_pointer(&tmp); } + + +char ** +virshDomainFSMountpointsCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags) +{ + g_auto(GStrv) tmp = NULL; + virDomainPtr dom = NULL; + int rc = -1; + size_t i; + virDomainFSInfoPtr *info = NULL; + size_t ninfos = 0; + char **ret = NULL; + + virCheckFlags(0, NULL); + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return NULL; + + rc = virDomainGetFSInfo(dom, &info, 0); + if (rc <= 0) { + goto cleanup; + } + ninfos = rc; + + if (info) { + tmp = g_new0(char *, ninfos + 1); + for (i = 0; i < ninfos; i++) { + tmp[i] = g_strdup(info[i]->mountpoint); + } + ret = g_steal_pointer(&tmp); + } + + cleanup: + if (info) { + for (i = 0; i < ninfos; i++) + virDomainFSInfoFree(info[i]); + VIR_FREE(info); + } + virshDomainFree(dom); + return ret; +} diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h index 04a3705ff9..ef242d0c68 100644 --- a/tools/virsh-completer-domain.h +++ b/tools/virsh-completer-domain.h @@ -118,3 +118,7 @@ char ** virshCodesetNameCompleter(vshControl *ctl, char ** virshKeycodeNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); + +char ** virshDomainFSMountpointsCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index fac590fbc6..9d315bdbcf 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -13920,6 +13920,7 @@ static const vshCmdOptDef opts_domfsfreeze[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "mountpoint", .type = VSH_OT_ARGV, + .completer = virshDomainFSMountpointsCompleter, .help = N_("mountpoint path to be frozen") }, {.name = NULL} @@ -13969,6 +13970,7 @@ static const vshCmdOptDef opts_domfsthaw[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "mountpoint", .type = VSH_OT_ARGV, + .completer = virshDomainFSMountpointsCompleter, .help = N_("mountpoint path to be thawed") }, {.name = NULL} -- 2.26.2
On 4/22/21 12:38 PM, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-completer-domain.c | 43 ++++++++++++++++++++++++++++++++++ tools/virsh-completer-domain.h | 4 ++++ tools/virsh-domain.c | 2 ++ 3 files changed, 49 insertions(+)
diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c index 99b36a2f9f..f56dc40125 100644 --- a/tools/virsh-completer-domain.c +++ b/tools/virsh-completer-domain.c @@ -872,3 +872,46 @@ virshKeycodeNameCompleter(vshControl *ctl,
return g_steal_pointer(&tmp); } + + +char ** +virshDomainFSMountpointsCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags) +{ + g_auto(GStrv) tmp = NULL; + virDomainPtr dom = NULL; + int rc = -1; + size_t i; + virDomainFSInfoPtr *info = NULL; + size_t ninfos = 0; + char **ret = NULL; + + virCheckFlags(0, NULL); + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return NULL; + + rc = virDomainGetFSInfo(dom, &info, 0); + if (rc <= 0) { + goto cleanup; + } + ninfos = rc; + + if (info) {
This feels redundant. @ninfos is > 0 at this point, thus @info must be !NULL. I know you took inspiration from cmdDomFSInfo() but the same argument applies there.
+ tmp = g_new0(char *, ninfos + 1); + for (i = 0; i < ninfos; i++) { + tmp[i] = g_strdup(info[i]->mountpoint); + } + ret = g_steal_pointer(&tmp); + } + + cleanup: + if (info) { + for (i = 0; i < ninfos; i++) + virDomainFSInfoFree(info[i]); + VIR_FREE(info); + } + virshDomainFree(dom); + return ret; +} diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h index 04a3705ff9..ef242d0c68 100644 --- a/tools/virsh-completer-domain.h +++ b/tools/virsh-completer-domain.h @@ -118,3 +118,7 @@ char ** virshCodesetNameCompleter(vshControl *ctl, char ** virshKeycodeNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); + +char ** virshDomainFSMountpointsCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index fac590fbc6..9d315bdbcf 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -13920,6 +13920,7 @@ static const vshCmdOptDef opts_domfsfreeze[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "mountpoint", .type = VSH_OT_ARGV, + .completer = virshDomainFSMountpointsCompleter, .help = N_("mountpoint path to be frozen") }, {.name = NULL} @@ -13969,6 +13970,7 @@ static const vshCmdOptDef opts_domfsthaw[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "mountpoint", .type = VSH_OT_ARGV, + .completer = virshDomainFSMountpointsCompleter, .help = N_("mountpoint path to be thawed") }, {.name = NULL}
Missed one more: opts_domfstrim. Michal
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> --- tools/virsh-completer-domain.c | 61 ++++++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 17 deletions(-) diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c index f56dc40125..998aa5d646 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, ¶ms, &nparams, 0) < 0) goto cleanup; onlineVcpuStr = vshGetTypedParamValue(ctl, ¶ms[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; - - cpulist[offset++] = g_strdup_printf("%zu", i); + offlinableVcpuStr = vshGetTypedParamValue(ctl, ¶ms[2]); + + 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; - - cpulist[offset++] = g_strdup_printf("%zu", i); + 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[i] = g_strdup_printf("%zu", j++); } } } -- 2.26.2
On 4/22/21 12:38 PM, Lin Ma wrote:
Lin Ma (2): virsh: Add mountpoint completion to domfsfreeze/domfsthaw command virsh: Fix completion logic to guestvcpus command
tools/virsh-completer-domain.c | 104 +++++++++++++++++++++++++++------ tools/virsh-completer-domain.h | 4 ++ tools/virsh-domain.c | 2 + 3 files changed, 93 insertions(+), 17 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed. Michal
participants (2)
-
Lin Ma -
Michal Privoznik