[PATCH v2] virsh-completer: modify and fix bug in virshPoolTypeCompleter, now used for more commands
Signed-off-by: Adam Julis <ajulis@redhat.com> --- v2: - instead new completer is used already existing virshPoolTypeCompleter - added flag VIRSH_POOL_TYPE_COMPLETER_COMMA for the completer - fixed bug in pool-list --type command v1: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/K6GT... tools/virsh-completer-domain.h | 4 ++++ tools/virsh-completer-pool.c | 14 +++++++++----- tools/virsh-pool.c | 3 +++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h index 27cf963912..fe4af07937 100644 --- a/tools/virsh-completer-domain.h +++ b/tools/virsh-completer-domain.h @@ -31,6 +31,10 @@ enum { VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC = 1 << 0, /* Return just MACs */ }; +enum { + VIRSH_POOL_TYPE_COMPLETER_COMMA = 1 << 0, +}; + char ** virshDomainInterfaceCompleter(vshControl *ctl, const vshCmd *cmd, diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c index 0600394411..abad0fca1b 100644 --- a/tools/virsh-completer-pool.c +++ b/tools/virsh-completer-pool.c @@ -93,13 +93,17 @@ virshPoolTypeCompleter(vshControl *ctl, g_auto(GStrv) tmp = NULL; const char *type_str = NULL; - virCheckFlags(0, NULL); - - if (vshCommandOptStringQuiet(ctl, cmd, "type", &type_str) < 0) - return NULL; + virCheckFlags(VIRSH_POOL_TYPE_COMPLETER_COMMA, NULL); tmp = virshEnumComplete(VIR_STORAGE_POOL_LAST, virStoragePoolTypeToString); - return virshCommaStringListComplete(type_str, (const char **)tmp); + if (flags & VIRSH_POOL_TYPE_COMPLETER_COMMA){ + if (vshCommandOptStringQuiet(ctl, cmd, "type", &type_str) < 0) + return NULL; + + return virshCommaStringListComplete(type_str, (const char **)tmp); + } + + return g_steal_pointer(&tmp); } diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 36f00cf643..6936406087 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -1088,6 +1088,7 @@ static const vshCmdOptDef opts_pool_list[] = { }, {.name = "type", .type = VSH_OT_STRING, + .completer_flags = VIRSH_POOL_TYPE_COMPLETER_COMMA, .completer = virshPoolTypeCompleter, .help = N_("only list pool of specified type(s) (if supported)") }, @@ -1414,6 +1415,7 @@ static const vshCmdOptDef opts_find_storage_pool_sources_as[] = { {.name = "type", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshPoolTypeCompleter, .help = N_("type of storage pool sources to find") }, {.name = "host", @@ -1501,6 +1503,7 @@ static const vshCmdOptDef opts_find_storage_pool_sources[] = { {.name = "type", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshPoolTypeCompleter, .help = N_("type of storage pool sources to discover") }, {.name = "srcSpec", -- 2.43.0
On 2/20/24 10:31, Adam Julis wrote:
Signed-off-by: Adam Julis <ajulis@redhat.com> --- v2: - instead new completer is used already existing virshPoolTypeCompleter - added flag VIRSH_POOL_TYPE_COMPLETER_COMMA for the completer - fixed bug in pool-list --type command
v1: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/K6GT...
tools/virsh-completer-domain.h | 4 ++++ tools/virsh-completer-pool.c | 14 +++++++++----- tools/virsh-pool.c | 3 +++ 3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c index 0600394411..abad0fca1b 100644 --- a/tools/virsh-completer-pool.c +++ b/tools/virsh-completer-pool.c @@ -93,13 +93,17 @@ virshPoolTypeCompleter(vshControl *ctl, g_auto(GStrv) tmp = NULL; const char *type_str = NULL;
- virCheckFlags(0, NULL); - - if (vshCommandOptStringQuiet(ctl, cmd, "type", &type_str) < 0) - return NULL; + virCheckFlags(VIRSH_POOL_TYPE_COMPLETER_COMMA, NULL);
tmp = virshEnumComplete(VIR_STORAGE_POOL_LAST, virStoragePoolTypeToString);
- return virshCommaStringListComplete(type_str, (const char **)tmp); + if (flags & VIRSH_POOL_TYPE_COMPLETER_COMMA){ + if (vshCommandOptStringQuiet(ctl, cmd, "type", &type_str) < 0) + return NULL; + + return virshCommaStringListComplete(type_str, (const char **)tmp); + } + + return g_steal_pointer(&tmp); }
Consider this squashed in diff --git i/tools/virsh-completer-pool.c w/tools/virsh-completer-pool.c index abad0fca1b..f3f10de943 100644 --- i/tools/virsh-completer-pool.c +++ w/tools/virsh-completer-pool.c @@ -98,7 +98,7 @@ virshPoolTypeCompleter(vshControl *ctl, tmp = virshEnumComplete(VIR_STORAGE_POOL_LAST, virStoragePoolTypeToString); - if (flags & VIRSH_POOL_TYPE_COMPLETER_COMMA){ + if (flags & VIRSH_POOL_TYPE_COMPLETER_COMMA) { if (vshCommandOptStringQuiet(ctl, cmd, "type", &type_str) < 0) return NULL;
On 2/20/24 10:31, Adam Julis wrote:
Signed-off-by: Adam Julis <ajulis@redhat.com> --- v2: - instead new completer is used already existing virshPoolTypeCompleter - added flag VIRSH_POOL_TYPE_COMPLETER_COMMA for the completer - fixed bug in pool-list --type command
v1: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/K6GT...
tools/virsh-completer-domain.h | 4 ++++ tools/virsh-completer-pool.c | 14 +++++++++----- tools/virsh-pool.c | 3 +++ 3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h index 27cf963912..fe4af07937 100644 --- a/tools/virsh-completer-domain.h +++ b/tools/virsh-completer-domain.h @@ -31,6 +31,10 @@ enum { VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC = 1 << 0, /* Return just MACs */ };
+enum { + VIRSH_POOL_TYPE_COMPLETER_COMMA = 1 << 0, +}; +
This doesn't need to be in virsh-completer-domain.h but can live in virsh-completer-pool.h happilly since its only purpose is to be passed to virshPoolTypeCompleter() which is declared in the latter header file.
char ** virshDomainInterfaceCompleter(vshControl *ctl, const vshCmd *cmd, diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c index 0600394411..abad0fca1b 100644 --- a/tools/virsh-completer-pool.c +++ b/tools/virsh-completer-pool.c @@ -93,13 +93,17 @@ virshPoolTypeCompleter(vshControl *ctl, g_auto(GStrv) tmp = NULL; const char *type_str = NULL;
- virCheckFlags(0, NULL); - - if (vshCommandOptStringQuiet(ctl, cmd, "type", &type_str) < 0) - return NULL; + virCheckFlags(VIRSH_POOL_TYPE_COMPLETER_COMMA, NULL);
tmp = virshEnumComplete(VIR_STORAGE_POOL_LAST, virStoragePoolTypeToString);
- return virshCommaStringListComplete(type_str, (const char **)tmp); + if (flags & VIRSH_POOL_TYPE_COMPLETER_COMMA){ + if (vshCommandOptStringQuiet(ctl, cmd, "type", &type_str) < 0) + return NULL; + + return virshCommaStringListComplete(type_str, (const char **)tmp); + } + + return g_steal_pointer(&tmp);
After playing with this for a while I found it more readable if the code is reorganized a bit: if (!(flags & VIRSH_POOL....)) return g_steal_pointer(&tmp); if (vshCommandOptStringQuiet() < 0) return NULL; return virshCommaStringListComplete();
} diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 36f00cf643..6936406087 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -1088,6 +1088,7 @@ static const vshCmdOptDef opts_pool_list[] = { }, {.name = "type", .type = VSH_OT_STRING, + .completer_flags = VIRSH_POOL_TYPE_COMPLETER_COMMA, .completer = virshPoolTypeCompleter,
Personal preference - .completer_flags can be declared after .completer.
.help = N_("only list pool of specified type(s) (if supported)") }, @@ -1414,6 +1415,7 @@ static const vshCmdOptDef opts_find_storage_pool_sources_as[] = { {.name = "type", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshPoolTypeCompleter, .help = N_("type of storage pool sources to find") }, {.name = "host", @@ -1501,6 +1503,7 @@ static const vshCmdOptDef opts_find_storage_pool_sources[] = { {.name = "type", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshPoolTypeCompleter, .help = N_("type of storage pool sources to discover") }, {.name = "srcSpec",
I've fixed all the small issues and merged. Congratulations on your first libvirt contribution! Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Adam Julis -
Michal Prívozník