[libvirt] [PATCH v2 0/5] virsh bash completion improvement
v1 -> v2: * 01-08 were merged, So remove them. * Reworked virshStorageVolKeyCompleter to avoid leaking objects. Thanks to Michal Privoznik for helping reviewing these patches. Lin Ma (5): virsh-pool: Remove static from virshStoragePoolList{Free,Collect} virsh-volume: Introduce virshStorageVolKeyCompleter virsh-volume: Add macros VIRSH_COMMON_OPT_VOL_* virsh-volume: Apply virshStorageVolKeyCompleter to vol-{name,pool} commands virsh-volume: Apply virshStorageVolNameCompleter to vol-{key,path} commands tools/virsh-completer-volume.c | 47 ++++++++++++++++++++++++++ tools/virsh-completer-volume.h | 5 +++ tools/virsh-pool.c | 10 ++---- tools/virsh-pool.h | 11 +++++++ tools/virsh-volume.c | 60 ++++++++++++++++------------------ 5 files changed, 94 insertions(+), 39 deletions(-) -- 2.26.2
The functions will be used by next patch. Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-pool.c | 10 ++-------- tools/virsh-pool.h | 11 +++++++++++ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 34ed86152e..18f3839a4c 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -835,13 +835,7 @@ virshStoragePoolSorter(const void *a, const void *b) virStoragePoolGetName(*pb)); } -struct virshStoragePoolList { - virStoragePoolPtr *pools; - size_t npools; -}; - -static void -virshStoragePoolListFree(struct virshStoragePoolList *list) +void virshStoragePoolListFree(struct virshStoragePoolList *list) { size_t i; @@ -855,7 +849,7 @@ virshStoragePoolListFree(struct virshStoragePoolList *list) g_free(list); } -static struct virshStoragePoolList * +struct virshStoragePoolList * virshStoragePoolListCollect(vshControl *ctl, unsigned int flags) { diff --git a/tools/virsh-pool.h b/tools/virsh-pool.h index 219f0eea42..d7bacd8731 100644 --- a/tools/virsh-pool.h +++ b/tools/virsh-pool.h @@ -40,3 +40,14 @@ typedef struct virshPoolEventCallback virshPoolEventCallback; extern virshPoolEventCallback virshPoolEventCallbacks[]; extern const vshCmdDef storagePoolCmds[]; + +struct virshStoragePoolList { + virStoragePoolPtr *pools; + size_t npools; +}; + +struct virshStoragePoolList * +virshStoragePoolListCollect(vshControl *ctl, + unsigned int flags); + +void virshStoragePoolListFree(struct virshStoragePoolList *list); -- 2.26.2
Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-completer-volume.c | 47 ++++++++++++++++++++++++++++++++++ tools/virsh-completer-volume.h | 5 ++++ 2 files changed, 52 insertions(+) diff --git a/tools/virsh-completer-volume.c b/tools/virsh-completer-volume.c index 301ee982a5..a29c5eca41 100644 --- a/tools/virsh-completer-volume.c +++ b/tools/virsh-completer-volume.c @@ -69,3 +69,50 @@ virshStorageVolNameCompleter(vshControl *ctl, g_free(vols); return ret; } + +char ** +virshStorageVolKeyCompleter(vshControl *ctl, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + virshControl *priv = ctl->privData; + struct virshStoragePoolList *list = NULL; + virStorageVolPtr *vols = NULL; + int rc; + int nvols = 0; + size_t i = 0, j = 0; + char **ret = NULL; + g_auto(GStrv) tmp = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if (!(list = virshStoragePoolListCollect(ctl, + VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE))) + goto cleanup; + + for (i = 0; i < list->npools; i++) { + if ((rc = virStoragePoolListAllVolumes(list->pools[i], &vols, 0)) < 0) + goto cleanup; + + tmp = g_renew(char *, tmp, nvols + rc + 1); + memset(&tmp[nvols], 0, sizeof(*tmp) * (rc + 1)); + + for (j = 0; j < rc; j++) { + const char *key = virStorageVolGetKey(vols[j]); + tmp[nvols] = g_strdup(key); + nvols++; + virStorageVolFree(vols[j]); + } + + g_free(vols); + } + + ret = g_steal_pointer(&tmp); + + cleanup: + virshStoragePoolListFree(list); + return ret; +} diff --git a/tools/virsh-completer-volume.h b/tools/virsh-completer-volume.h index 6591e13fdf..b41d8f4f3e 100644 --- a/tools/virsh-completer-volume.h +++ b/tools/virsh-completer-volume.h @@ -26,3 +26,8 @@ char ** virshStorageVolNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); + + +char ** virshStorageVolKeyCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -- 2.26.2
On 6/16/21 10:02 AM, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-completer-volume.c | 47 ++++++++++++++++++++++++++++++++++ tools/virsh-completer-volume.h | 5 ++++ 2 files changed, 52 insertions(+)
diff --git a/tools/virsh-completer-volume.c b/tools/virsh-completer-volume.c index 301ee982a5..a29c5eca41 100644 --- a/tools/virsh-completer-volume.c +++ b/tools/virsh-completer-volume.c @@ -69,3 +69,50 @@ virshStorageVolNameCompleter(vshControl *ctl, g_free(vols); return ret; } + +char ** +virshStorageVolKeyCompleter(vshControl *ctl, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + virshControl *priv = ctl->privData; + struct virshStoragePoolList *list = NULL; + virStorageVolPtr *vols = NULL; + int rc; + int nvols = 0; + size_t i = 0, j = 0; + char **ret = NULL; + g_auto(GStrv) tmp = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if (!(list = virshStoragePoolListCollect(ctl, + VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE)))
This is not aligned. One way to resolve this is to break this compound statement into two: list = virshBLAH(); if (!list) goto cleanup;
+ goto cleanup; +
Michal
Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-volume.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index c7d5ab8c3b..41b366a833 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -54,14 +54,28 @@ .completer_flags = VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE, \ } -#define VIRSH_COMMON_OPT_VOLUME_VOL \ +#define VIRSH_COMMON_OPT_VOL_NAME(_helpstr) \ {.name = "vol", \ .type = VSH_OT_DATA, \ .flags = VSH_OFLAG_REQ, \ - .help = N_("vol name, key or path"), \ + .help = _helpstr, \ .completer = virshStorageVolNameCompleter, \ } +#define VIRSH_COMMON_OPT_VOL_KEY(_helpstr) \ + {.name = "vol", \ + .type = VSH_OT_DATA, \ + .flags = VSH_OFLAG_REQ, \ + .help = _helpstr, \ + .completer = virshStorageVolKeyCompleter, \ + } + +#define VIRSH_COMMON_OPT_VOL_FULL \ + VIRSH_COMMON_OPT_VOL_NAME(N_("vol name, key or path")) + +#define VIRSH_COMMON_OPT_VOL_BY_KEY \ + VIRSH_COMMON_OPT_VOL_KEY(N_("volume key or path")) + virStorageVolPtr virshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd, const char *optname, @@ -440,7 +454,7 @@ static const vshCmdInfo info_vol_create_from[] = { static const vshCmdOptDef opts_vol_create_from[] = { VIRSH_COMMON_OPT_POOL_FULL, VIRSH_COMMON_OPT_FILE(N_("file containing an XML vol description")), - VIRSH_COMMON_OPT_VOLUME_VOL, + VIRSH_COMMON_OPT_VOL_FULL, {.name = "inputpool", .type = VSH_OT_STRING, .help = N_("pool name or uuid of the input volume's pool") @@ -550,7 +564,7 @@ static const vshCmdInfo info_vol_clone[] = { }; static const vshCmdOptDef opts_vol_clone[] = { - VIRSH_COMMON_OPT_VOLUME_VOL, + VIRSH_COMMON_OPT_VOL_FULL, {.name = "newname", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -646,7 +660,7 @@ static const vshCmdInfo info_vol_upload[] = { }; static const vshCmdOptDef opts_vol_upload[] = { - VIRSH_COMMON_OPT_VOLUME_VOL, + VIRSH_COMMON_OPT_VOL_FULL, VIRSH_COMMON_OPT_FILE(N_("file")), VIRSH_COMMON_OPT_POOL_OPTIONAL, {.name = "offset", @@ -768,7 +782,7 @@ static const vshCmdInfo info_vol_download[] = { }; static const vshCmdOptDef opts_vol_download[] = { - VIRSH_COMMON_OPT_VOLUME_VOL, + VIRSH_COMMON_OPT_VOL_FULL, VIRSH_COMMON_OPT_FILE(N_("file")), VIRSH_COMMON_OPT_POOL_OPTIONAL, {.name = "offset", @@ -889,7 +903,7 @@ static const vshCmdInfo info_vol_delete[] = { }; static const vshCmdOptDef opts_vol_delete[] = { - VIRSH_COMMON_OPT_VOLUME_VOL, + VIRSH_COMMON_OPT_VOL_FULL, VIRSH_COMMON_OPT_POOL_OPTIONAL, {.name = "delete-snapshots", .type = VSH_OT_BOOL, @@ -939,7 +953,7 @@ static const vshCmdInfo info_vol_wipe[] = { }; static const vshCmdOptDef opts_vol_wipe[] = { - VIRSH_COMMON_OPT_VOLUME_VOL, + VIRSH_COMMON_OPT_VOL_FULL, VIRSH_COMMON_OPT_POOL_OPTIONAL, {.name = "algorithm", .type = VSH_OT_STRING, @@ -1027,7 +1041,7 @@ static const vshCmdInfo info_vol_info[] = { }; static const vshCmdOptDef opts_vol_info[] = { - VIRSH_COMMON_OPT_VOLUME_VOL, + VIRSH_COMMON_OPT_VOL_FULL, VIRSH_COMMON_OPT_POOL_OPTIONAL, {.name = "bytes", .type = VSH_OT_BOOL, @@ -1117,7 +1131,7 @@ static const vshCmdInfo info_vol_resize[] = { }; static const vshCmdOptDef opts_vol_resize[] = { - VIRSH_COMMON_OPT_VOLUME_VOL, + VIRSH_COMMON_OPT_VOL_FULL, {.name = "capacity", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -1214,7 +1228,7 @@ static const vshCmdInfo info_vol_dumpxml[] = { }; static const vshCmdOptDef opts_vol_dumpxml[] = { - VIRSH_COMMON_OPT_VOLUME_VOL, + VIRSH_COMMON_OPT_VOL_FULL, VIRSH_COMMON_OPT_POOL_OPTIONAL, {.name = NULL} }; -- 2.26.2
Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-volume.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 41b366a833..daa205ea67 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -1552,11 +1552,7 @@ static const vshCmdInfo info_vol_name[] = { }; static const vshCmdOptDef opts_vol_name[] = { - {.name = "vol", - .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ, - .help = N_("volume key or path") - }, + VIRSH_COMMON_OPT_VOL_BY_KEY, {.name = NULL} }; @@ -1588,11 +1584,7 @@ static const vshCmdInfo info_vol_pool[] = { }; static const vshCmdOptDef opts_vol_pool[] = { - {.name = "vol", - .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ, - .help = N_("volume key or path") - }, + VIRSH_COMMON_OPT_VOL_BY_KEY, {.name = "uuid", .type = VSH_OT_BOOL, .help = N_("return the pool uuid rather than pool name") -- 2.26.2
Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-volume.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index daa205ea67..1da9b7217f 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -1643,11 +1643,7 @@ static const vshCmdInfo info_vol_key[] = { }; static const vshCmdOptDef opts_vol_key[] = { - {.name = "vol", - .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ, - .help = N_("volume name or path") - }, + VIRSH_COMMON_OPT_VOL_NAME(N_("volume name or path")), VIRSH_COMMON_OPT_POOL_OPTIONAL, {.name = NULL} }; @@ -1679,11 +1675,7 @@ static const vshCmdInfo info_vol_path[] = { }; static const vshCmdOptDef opts_vol_path[] = { - {.name = "vol", - .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ, - .help = N_("volume name or key") - }, + VIRSH_COMMON_OPT_VOL_NAME(N_("volume name or key")), VIRSH_COMMON_OPT_POOL_OPTIONAL, {.name = NULL} }; -- 2.26.2
On 6/16/21 10:02 AM, Lin Ma wrote:
v1 -> v2: * 01-08 were merged, So remove them. * Reworked virshStorageVolKeyCompleter to avoid leaking objects.
Thanks to Michal Privoznik for helping reviewing these patches.
Lin Ma (5): virsh-pool: Remove static from virshStoragePoolList{Free,Collect} virsh-volume: Introduce virshStorageVolKeyCompleter virsh-volume: Add macros VIRSH_COMMON_OPT_VOL_* virsh-volume: Apply virshStorageVolKeyCompleter to vol-{name,pool} commands virsh-volume: Apply virshStorageVolNameCompleter to vol-{key,path} commands
tools/virsh-completer-volume.c | 47 ++++++++++++++++++++++++++ tools/virsh-completer-volume.h | 5 +++ tools/virsh-pool.c | 10 ++---- tools/virsh-pool.h | 11 +++++++ tools/virsh-volume.c | 60 ++++++++++++++++------------------ 5 files changed, 94 insertions(+), 39 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed. Michal
participants (2)
-
Lin Ma -
Michal Prívozník