Re: [libvirt] [PATCH 07/17] virsh: Add network uuid completion to network-name command

[Michal Privoznik]

On 9/11/20 9:13 AM, Lin Ma wrote:
Signed-off-by: Lin Ma <l...@suse.de>
---
  tools/virsh-completer-network.c | 42 +++++++++++++++++++++++++++++++++
  tools/virsh-completer-network.h |  4 ++++
  tools/virsh-network.c           |  2 ++
  3 files changed, 48 insertions(+)
diff --git a/tools/virsh-completer-network.c b/tools/virsh-completer-network.c
index 8f0048ed6f..22ab4a80c3 100644
--- a/tools/virsh-completer-network.c
+++ b/tools/virsh-completer-network.c
@@ -137,3 +137,45 @@ virshNetworkPortUUIDCompleter(vshControl *ctl,
      VIR_FREE(ret);
      return NULL;
  }
+
+
+char **
+virshNetworkUUIDCompleter(vshControl *ctl,
+                          const vshCmd *cmd G_GNUC_UNUSED,
+                          unsigned int flags)
+{
+    virshControlPtr priv = ctl->privData;
+    virNetworkPtr *nets = NULL;
+    int nnets = 0;
+    size_t i = 0;
+    char **ret = NULL;
+    VIR_AUTOSTRINGLIST tmp = NULL;
+
+    virCheckFlags(0, NULL);
+
+    if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+        return NULL;
+
+    if ((nnets = virConnectListAllNetworks(priv->conn, &nets, flags)) < 0)
+        return NULL;
+
+    if (VIR_ALLOC_N(tmp, nnets + 1) < 0)
+        goto cleanup;
+
+    for (i = 0; i < nnets; i++) {
+        char uuid[VIR_UUID_STRING_BUFLEN];
+        if (virNetworkGetUUIDString(nets[i], uuid) < 0) {
+            vshError(ctl, "%s", _("Failed to get network's UUID"));

We don't like completers to report any kind of error because that would 
harm the user experience. For instance it doesn't print the error on an 
empty line:

virsh # net-name --network error: Failed to get ...

It's acceptable if completer returns nothing when failing.

+            goto cleanup;
+        }
+        tmp[i] = g_strdup(uuid);
+    }
+
+    ret = g_steal_pointer(&tmp);
+
+ cleanup:
+    for (i = 0; i < nnets; i++)
+        virNetworkFree(nets[i]);
+    VIR_FREE(nets);
+    return ret;
+}
diff --git a/tools/virsh-completer-network.h b/tools/virsh-completer-network.h
index e317e483c1..8910e4525c 100644
--- a/tools/virsh-completer-network.h
+++ b/tools/virsh-completer-network.h
@@ -33,3 +33,7 @@ char ** virshNetworkEventNameCompleter(vshControl *ctl,
  char ** virshNetworkPortUUIDCompleter(vshControl *ctl,
                                        const vshCmd *cmd,
                                        unsigned int flags);
+
+char ** virshNetworkUUIDCompleter(vshControl *ctl,
+                                  const vshCmd *cmd,
+                                  unsigned int flags);
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index f0f5358625..f488e840ac 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -806,6 +806,8 @@ static const vshCmdOptDef opts_network_name[] = {
      {.name = "network",
       .type = VSH_OT_DATA,
       .flags = VSH_OFLAG_REQ,
+     .completer = virshNetworkUUIDCompleter,
+     .completer_flags = 0,

This is not needed. flags are zero by default.

       .help = N_("network uuid")
      },
      {.name = NULL}


I see you used the same pattern for next patches - the same comment 
applies to them.

Michal