1. Patchew
  2. Libvirt
  3. View series

[PATCH v2] virsh-completer: modify and fix bug in virshPoolTypeCompleter, now used for more commands

Adam Julis posted 1 patch 2 months, 1 week ago
Failed in applying to current master (apply log)
tools/virsh-completer-domain.h |  4 ++++
tools/virsh-completer-pool.c   | 14 +++++++++-----
tools/virsh-pool.c             |  3 +++
3 files changed, 16 insertions(+), 5 deletions(-)
[PATCH v2] virsh-completer: modify and fix bug in virshPoolTypeCompleter, now used for more commands
Posted by Adam Julis 2 months, 1 week ago 
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/K6GT4P5S52DSRYMVQD3HLGIN33N4XPJR/

 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
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v2] virsh-completer: modify and fix bug in virshPoolTypeCompleter, now used for more commands
Posted by Michal Prívozník 2 months, 1 week ago 
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/K6GT4P5S52DSRYMVQD3HLGIN33N4XPJR/
> 
>  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
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v2] virsh-completer: modify and fix bug in virshPoolTypeCompleter, now used for more commands
Posted by Adam Julis 2 months, 1 week ago 
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/K6GT4P5S52DSRYMVQD3HLGIN33N4XPJR/
>
>   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;
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.