1. Patchew
  2. Libvirt
  3. View series

[PATCH v3] virsh: Provide completer for some pool-X-as commands

Abhiram Tilak posted 1 patch 3 months, 1 week ago 
src/libvirt_private.syms     |  2 ++
tools/virsh-completer-pool.c | 70 ++++++++++++++++++++++++++++++++++++
tools/virsh-completer-pool.h | 20 +++++++++++
tools/virsh-pool.c           |  9 +++++
4 files changed, 101 insertions(+)
[PATCH v3] virsh: Provide completer for some pool-X-as commands
Posted by Abhiram Tilak 3 months, 1 week ago 
From: notpua <atp@tutamail.com>

Provides completers for auth-type and source-format commands for
virsh pool-create-as and pool-define-as commands. Use Empty completers
for options where completions are not required.

Related Issue: https://gitlab.com/libvirt/libvirt/-/issues/9
Signed-off-by: Abhiram Tilak <atp.exp@gmail.com>
---
Changes in v2:
  - Fix all formatting errors
  - Change some options using Empty completers, to use
    LocalPath completers.
  - Add completers for AdapterName and AdapterParent using information
    from node devices.
Changes in v3:
  - Call virshNodeDeviceNameComplete with modified flags instead of 
    duplicating the whole implementation.

 src/libvirt_private.syms     |  2 ++
 tools/virsh-completer-pool.c | 70 ++++++++++++++++++++++++++++++++++++
 tools/virsh-completer-pool.h | 20 +++++++++++
 tools/virsh-pool.c           |  9 +++++
 4 files changed, 101 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c35366c9e1..6ba1e8e2c5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1123,6 +1123,8 @@ virStorageAuthDefCopy;
 virStorageAuthDefFormat;
 virStorageAuthDefFree;
 virStorageAuthDefParse;
+virStorageAuthTypeFromString;
+virStorageAuthTypeToString;
 virStorageFileFeatureTypeFromString;
 virStorageFileFeatureTypeToString;
 virStorageFileFormatTypeFromString;
diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c
index 3568bb985b..ba7855fdba 100644
--- a/tools/virsh-completer-pool.c
+++ b/tools/virsh-completer-pool.c
@@ -23,6 +23,7 @@
 #include "virsh-completer-pool.h"
 #include "virsh-util.h"
 #include "conf/storage_conf.h"
+#include "conf/node_device_conf.h"
 #include "virsh-pool.h"
 #include "virsh.h"
 
@@ -106,3 +107,72 @@ virshPoolTypeCompleter(vshControl *ctl,
 
     return virshCommaStringListComplete(type_str, (const char **)tmp);
 }
+
+
+char **
+virshPoolFormatCompleter(vshControl *ctl G_GNUC_UNUSED,
+                         const vshCmd *cmd G_GNUC_UNUSED,
+                         unsigned int flags)
+{
+    size_t i = 0;
+    size_t j = 0;
+    g_auto(GStrv) tmp = NULL;
+    size_t nformats = VIR_STORAGE_POOL_FS_LAST + VIR_STORAGE_POOL_NETFS_LAST +
+	              VIR_STORAGE_POOL_DISK_LAST + VIR_STORAGE_POOL_LOGICAL_LAST;
+
+    virCheckFlags(0, NULL);
+
+    tmp = g_new0(char *, nformats + 1);
+
+    /* Club all PoolFormats for completion */
+    for (i = 0; i < VIR_STORAGE_POOL_FS_LAST; i++)
+        tmp[j++] = g_strdup(virStoragePoolFormatFileSystemTypeToString(i));
+
+    for (i = 0; i < VIR_STORAGE_POOL_NETFS_LAST; i++)
+        tmp[j++] = g_strdup(virStoragePoolFormatFileSystemNetTypeToString(i));
+
+    for (i = 1; i < VIR_STORAGE_POOL_DISK_LAST; i++)
+        tmp[j++] = g_strdup(virStoragePoolFormatDiskTypeToString(i));
+
+    for (i = 1; i < VIR_STORAGE_POOL_LOGICAL_LAST; i++)
+        tmp[j++] = g_strdup(virStoragePoolFormatLogicalTypeToString(i));
+
+    return g_steal_pointer(&tmp);
+}
+
+
+char **
+virshPoolAuthTypeCompleter(vshControl *ctl G_GNUC_UNUSED,
+                           const vshCmd *cmd G_GNUC_UNUSED,
+                           unsigned int flags)
+{
+    size_t i = 0;
+    g_auto(GStrv) tmp = NULL;
+
+    virCheckFlags(0, NULL);
+
+    tmp = g_new0(char *, VIR_STORAGE_AUTH_TYPE_LAST + 1);
+
+    for (i = 0; i < VIR_STORAGE_AUTH_TYPE_LAST; i++)
+        tmp[i] = g_strdup(virStorageAuthTypeToString(i));
+
+    return g_steal_pointer(&tmp);
+}
+
+
+char **
+virshAdapterNameCompleter(vshControl *ctl,
+                          const vshCmd *cmd, unsigned int flags)
+{
+    flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST;
+    return virshNodeDeviceNameCompleter(ctl, cmd, flags);
+}
+
+
+char **
+virshAdapterParentCompleter(vshControl *ctl,
+                            const vshCmd *cmd, unsigned int flags)
+{
+    flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS;
+    return virshNodeDeviceNameCompleter(ctl, cmd, flags);
+}
diff --git a/tools/virsh-completer-pool.h b/tools/virsh-completer-pool.h
index bff3e5742b..eccc08a73f 100644
--- a/tools/virsh-completer-pool.h
+++ b/tools/virsh-completer-pool.h
@@ -40,3 +40,23 @@ char **
 virshPoolTypeCompleter(vshControl *ctl,
                        const vshCmd *cmd,
                        unsigned int flags);
+
+char **
+virshPoolFormatCompleter(vshControl *ctl,
+                         const vshCmd *cmd,
+                         unsigned int flags);
+
+char **
+virshPoolAuthTypeCompleter(vshControl *ctl,
+                         const vshCmd *cmd,
+                         unsigned int flags);
+
+char **
+virshAdapterNameCompleter(vshControl *ctl,
+                          const vshCmd *cmd,
+                          unsigned int flags);
+
+char **
+virshAdapterParentCompleter(vshControl *ctl,
+                            const vshCmd *cmd,
+                            unsigned int flags);
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index f9aad8ded0..0cbd1417e6 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -80,31 +80,37 @@
     {.name = "source-path", \
      .type = VSH_OT_STRING, \
      .unwanted_positional = true, \
+     .completer = virshCompletePathLocalExisting, \
      .help = N_("source path for underlying storage") \
     }, \
     {.name = "source-dev", \
      .type = VSH_OT_STRING, \
      .unwanted_positional = true, \
+     .completer = virshCompletePathLocalExisting, \
      .help = N_("source device for underlying storage") \
     }, \
     {.name = "source-name", \
      .type = VSH_OT_STRING, \
      .unwanted_positional = true, \
+     .completer = virshCompleteEmpty, \
      .help = N_("source name for underlying storage") \
     }, \
     {.name = "target", \
      .type = VSH_OT_STRING, \
      .unwanted_positional = true, \
+     .completer = virshCompletePathLocalExisting, \
      .help = N_("target for underlying storage") \
     }, \
     {.name = "source-format", \
      .type = VSH_OT_STRING, \
      .unwanted_positional = true, \
+     .completer = virshPoolFormatCompleter, \
      .help = N_("format for underlying storage") \
     }, \
     {.name = "auth-type", \
      .type = VSH_OT_STRING, \
      .unwanted_positional = true, \
+     .completer = virshPoolAuthTypeCompleter, \
      .help = N_("auth type to be used for underlying storage") \
     }, \
     {.name = "auth-username", \
@@ -126,6 +132,7 @@
     {.name = "adapter-name", \
      .type = VSH_OT_STRING, \
      .unwanted_positional = true, \
+     .completer = virshAdapterNameCompleter, \
      .help = N_("adapter name to be used for underlying storage") \
     }, \
     {.name = "adapter-wwnn", \
@@ -141,6 +148,7 @@
     {.name = "adapter-parent", \
      .type = VSH_OT_STRING, \
      .unwanted_positional = true, \
+     .completer = virshAdapterParentCompleter, \
      .help = N_("adapter parent scsi_hostN to be used for underlying vHBA storage") \
     }, \
     {.name = "adapter-parent-wwnn", \
@@ -161,6 +169,7 @@
     {.name = "source-protocol-ver", \
      .type = VSH_OT_STRING, \
      .unwanted_positional = true, \
+     .completer = virshCompleteEmpty, \
      .help = N_("nfsvers value for NFS pool mount option") \
     }, \
     {.name = "source-initiator", \
-- 
2.39.2
Re: [PATCH v3] virsh: Provide completer for some pool-X-as commands
Posted by Michal Prívozník 3 months, 1 week ago 
On 7/23/24 15:34, Abhiram Tilak wrote:
> From: notpua <atp@tutamail.com>
> 
> Provides completers for auth-type and source-format commands for
> virsh pool-create-as and pool-define-as commands. Use Empty completers
> for options where completions are not required.
> 
> Related Issue: https://gitlab.com/libvirt/libvirt/-/issues/9
> Signed-off-by: Abhiram Tilak <atp.exp@gmail.com>
> ---
> Changes in v2:
>   - Fix all formatting errors
>   - Change some options using Empty completers, to use
>     LocalPath completers.
>   - Add completers for AdapterName and AdapterParent using information
>     from node devices.
> Changes in v3:
>   - Call virshNodeDeviceNameComplete with modified flags instead of 
>     duplicating the whole implementation.
> 
>  src/libvirt_private.syms     |  2 ++
>  tools/virsh-completer-pool.c | 70 ++++++++++++++++++++++++++++++++++++
>  tools/virsh-completer-pool.h | 20 +++++++++++
>  tools/virsh-pool.c           |  9 +++++
>  4 files changed, 101 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index c35366c9e1..6ba1e8e2c5 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1123,6 +1123,8 @@ virStorageAuthDefCopy;
>  virStorageAuthDefFormat;
>  virStorageAuthDefFree;
>  virStorageAuthDefParse;
> +virStorageAuthTypeFromString;
> +virStorageAuthTypeToString;
>  virStorageFileFeatureTypeFromString;
>  virStorageFileFeatureTypeToString;
>  virStorageFileFormatTypeFromString;

This should be in a separate commit. It is a change that's semantically
different to the rest.

> diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c
> index 3568bb985b..ba7855fdba 100644
> --- a/tools/virsh-completer-pool.c
> +++ b/tools/virsh-completer-pool.c
> @@ -23,6 +23,7 @@
>  #include "virsh-completer-pool.h"
>  #include "virsh-util.h"
>  #include "conf/storage_conf.h"
> +#include "conf/node_device_conf.h"
>  #include "virsh-pool.h"
>  #include "virsh.h"
>  
> @@ -106,3 +107,72 @@ virshPoolTypeCompleter(vshControl *ctl,
>  
>      return virshCommaStringListComplete(type_str, (const char **)tmp);
>  }
> +
> +
> +char **
> +virshPoolFormatCompleter(vshControl *ctl G_GNUC_UNUSED,
> +                         const vshCmd *cmd G_GNUC_UNUSED,
> +                         unsigned int flags)
> +{
> +    size_t i = 0;
> +    size_t j = 0;
> +    g_auto(GStrv) tmp = NULL;
> +    size_t nformats = VIR_STORAGE_POOL_FS_LAST + VIR_STORAGE_POOL_NETFS_LAST +
> +	              VIR_STORAGE_POOL_DISK_LAST + VIR_STORAGE_POOL_LOGICAL_LAST;

We don't use TABs. I believe 'meson test' would have reported this.

> +
> +    virCheckFlags(0, NULL);
> +
> +    tmp = g_new0(char *, nformats + 1);
> +
> +    /* Club all PoolFormats for completion */
> +    for (i = 0; i < VIR_STORAGE_POOL_FS_LAST; i++)
> +        tmp[j++] = g_strdup(virStoragePoolFormatFileSystemTypeToString(i));
> +
> +    for (i = 0; i < VIR_STORAGE_POOL_NETFS_LAST; i++)
> +        tmp[j++] = g_strdup(virStoragePoolFormatFileSystemNetTypeToString(i));
> +
> +    for (i = 1; i < VIR_STORAGE_POOL_DISK_LAST; i++)
> +        tmp[j++] = g_strdup(virStoragePoolFormatDiskTypeToString(i));
> +
> +    for (i = 1; i < VIR_STORAGE_POOL_LOGICAL_LAST; i++)
> +        tmp[j++] = g_strdup(virStoragePoolFormatLogicalTypeToString(i));
> +
> +    return g_steal_pointer(&tmp);
> +}
> +
> +
> +char **
> +virshPoolAuthTypeCompleter(vshControl *ctl G_GNUC_UNUSED,
> +                           const vshCmd *cmd G_GNUC_UNUSED,
> +                           unsigned int flags)
> +{
> +    size_t i = 0;
> +    g_auto(GStrv) tmp = NULL;
> +
> +    virCheckFlags(0, NULL);
> +
> +    tmp = g_new0(char *, VIR_STORAGE_AUTH_TYPE_LAST + 1);
> +
> +    for (i = 0; i < VIR_STORAGE_AUTH_TYPE_LAST; i++)
> +        tmp[i] = g_strdup(virStorageAuthTypeToString(i));

There's no need to copy internals of virshEnumComplete().

> +
> +    return g_steal_pointer(&tmp);
> +}
> +
> +
> +char **
> +virshAdapterNameCompleter(vshControl *ctl,
> +                          const vshCmd *cmd, unsigned int flags)
> +{
> +    flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST;
> +    return virshNodeDeviceNameCompleter(ctl, cmd, flags);

I get the idea, but this can hardly work, because
virshNodeDeviceNameCompleter() has virCheckFlags(0, NULL);

which means it exits early and returns NULL (i.e. virsh falls back to
file name completion - which is surely not something desired).

Michal

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.