Re: [PATCH 5/5] list: Use virConnectListAllSecrets in virsh

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/14/12 10:38, Osier Yang wrote:
This introduce four new options for secret-list, to filter the

s/introduce/introduces/

returned secrets by whether it's ephemeral or not, and/or by
whether it's private or not.

* tools/virsh-secret.c: (New helper vshSecretSorter,
   vshSecretListFree, and vshCollectSecretList; Use the new
   API for secret-list; error out if flags are specified,
   because there is no way to filter the results when using
   old APIs (no APIs to get the properties (ephemeral, private)
   of a secret yet).

* tools/virsh.pod: Document the 4 new options.
---
  tools/virsh-secret.c |  209 +++++++++++++++++++++++++++++++++++++++++++-------
  tools/virsh.pod      |    8 ++-
  2 files changed, 186 insertions(+), 31 deletions(-)

diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
index abcfdfe..ab50405 100644
--- a/tools/virsh-secret.c
+++ b/tools/virsh-secret.c
@@ -290,6 +290,141 @@ cleanup:
      return ret;
  }

+static int
+vshSecretSorter(const void *a, const void *b)
+{
+    virSecretPtr *sa = (virSecretPtr *) a;
+    virSecretPtr *sb = (virSecretPtr *) b;
+    char uuid_sa[VIR_UUID_STRING_BUFLEN];
+    char uuid_sb[VIR_UUID_STRING_BUFLEN];
+
+    if (*sa && !*sb)
+        return -1;
+
+    if (!*sa)
+        return *sb != NULL;
+
+    virSecretGetUUIDString(*sa, uuid_sa);
+    virSecretGetUUIDString(*sb, uuid_sb);
+
+    return vshStrcasecmp(uuid_sa, uuid_sb);
+}
+
+struct vshSecretList {
+    virSecretPtr *secrets;
+    size_t nsecrets;
+};
+typedef struct vshSecretList *vshSecretListPtr;
+
+static void
+vshSecretListFree(vshSecretListPtr list)
+{
+    int i;
+
+    if (list && list->nsecrets) {
+        for (i = 0; i < list->nsecrets; i++) {
+            if (7list->secrets[i])
+                virSecretFree(list->secrets[i]);
+        }
+        VIR_FREE(list->secrets);
+    }
+    VIR_FREE(list);
+}
+
+static vshSecretListPtr
+vshSecretListCollect(vshControl *ctl,
+                     unsigned int flags)
+{
+    vshSecretListPtr list = vshMalloc(ctl, sizeof(*list));
+    int i;
+    int ret;
+    virSecretPtr secret;
+    bool success = false;
+    size_t deleted = 0;
+    int nsecrets = 0;
+    char **uuids = NULL;
+
+    /* try the list with flags support (0.10.2 and later) */
+    if ((ret = virConnectListAllSecrets(ctl->conn,
+                                        &list->secrets,
+                                        flags)) >= 0) {
+        list->nsecrets = ret;
+        goto finished;
+    }
+
+    /* check if the command is actually supported */
+    if (last_error && last_error->code == VIR_ERR_NO_SUPPORT)
+        goto fallback;
+
+    /* there was an error during the call */
+    vshError(ctl, "%s", _("Failed to list node secrets"));
+    goto cleanup;
+
+
+fallback:
+    /* fall back to old method (0.10.1 and older) */
+    vshResetLibvirtError();
+
+    if (flags) {
+        vshError(ctl, "%s", _("Filtering is not supported by this libvirt"));
+        goto cleanup;
+    }
+
+    nsecrets = virConnectNumOfSecrets(ctl->conn);
+    if (nsecrets < 0) {
+        vshError(ctl, "%s", _("Failed to count secrets"));
+        goto cleanup;
+    }
+
+    if (nsecrets == 0)
+        return list;
+
+    uuids = vshMalloc(ctl, sizeof(char *) * nsecrets);
+
+    nsecrets = virConnectListSecrets(ctl->conn, uuids, nsecrets);
+    if (nsecrets < 0) {
+        vshError(ctl, "%s", _("Failed to list secrets"));
+        goto cleanup;
+    }
+
+    list->secrets = vshMalloc(ctl, sizeof(virSecretPtr) * (nsecrets));
+    list->nsecrets = 0;
+
+    /* get the secrets */
+    for (i = 0; i < nsecrets ; i++) {
+        if (!(secret = virSecretLookupByUUIDString(ctl->conn, uuids[i])))
+            continue;
+        list->secrets[list->nsecrets++] = secret;
+    }
+
+    /* truncate secrets that weren't found */
+    deleted = nsecrets - list->nsecrets;
+
+finished:
+    /* sort the list */
+    if (list->secrets && list->nsecrets)
+        qsort(list->secrets, list->nsecrets,
+              sizeof(*list->secrets), vshSecretSorter);
+
+    /* truncate the list for not found secret objects */
+    if (deleted)
+        VIR_SHRINK_N(list->secrets, list->nsecrets, deleted);
+
+    success = true;
+
+cleanup:
+    for (i = 0; i < nsecrets; i++)
+        VIR_FREE(uuids[i]);
+    VIR_FREE(uuids);
+
+    if (!success) {
+        vshSecretListFree(list);
+        list = NULL;
+    }
+
+    return list;
+}
+
  /*
   * "secret-list" command
   */
@@ -299,59 +434,75 @@ static const vshCmdInfo info_secret_list[] = {
      {NULL, NULL}
  };

+static const vshCmdOptDef opts_secret_list[] = {
+    {"ephemeral", VSH_OT_BOOL, 0, N_("list ephemeral secrets")},
+    {"no-ephemeral", VSH_OT_BOOL, 0, N_("list secrets not ephemeral")},

I'd change the help string to:
"list non-ephmeral secrets"

+    {"private", VSH_OT_BOOL, 0, N_("list private secrets")},
+    {"no-private", VSH_OT_BOOL, 0, N_("list secrets not private")},

same here.

+    {NULL, 0, 0, NULL}
+};
+
  static bool
  cmdSecretList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
  {
-    int maxuuids = 0, i;
-    char **uuids = NULL;
+    int i;
+    vshSecretListPtr list = NULL;
+    bool ret = false;
+    bool ephemeral = vshCommandOptBool(cmd, "ephemeral");
+    bool no_ephemeral = vshCommandOptBool(cmd, "no-ephemeral");
+    bool private = vshCommandOptBool(cmd, "private");
+    bool no_private = vshCommandOptBool(cmd, "no-private");
+    unsigned int flags = 0;


There's no need to store the command options in separate bool variables as they're used only once to create the flag argument.

Use them directly in the conditions that set the flags.

-    maxuuids = virConnectNumOfSecrets(ctl->conn);
-    if (maxuuids < 0) {
-        vshError(ctl, "%s", _("Failed to list secrets"));
-        return false;
-    }
-    uuids = vshMalloc(ctl, sizeof(*uuids) * maxuuids);
+    if (ephemeral)
+        flags |= VIR_CONNECT_LIST_SECRETS_EPHEMERAL;

-    maxuuids = virConnectListSecrets(ctl->conn, uuids, maxuuids);
-    if (maxuuids < 0) {
-        vshError(ctl, "%s", _("Failed to list secrets"));
-        VIR_FREE(uuids);
-        return false;
-    }
+    if (no_ephemeral)
+        flags |= VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL;
+
+    if (private)
+        flags |= VIR_CONNECT_LIST_SECRETS_PRIVATE;

-    qsort(uuids, maxuuids, sizeof(char *), vshNameSorter);
+    if (no_private)
+        flags |= VIR_CONNECT_LIST_SECRETS_NO_PRIVATE;
+
+    if (!(list = vshSecretListCollect(ctl, flags)))
+        return false;

      vshPrintExtra(ctl, "%-36s %s\n", _("UUID"), _("Usage"));
      vshPrintExtra(ctl, "-----------------------------------------------------------\n");

-    for (i = 0; i < maxuuids; i++) {
-        virSecretPtr sec = virSecretLookupByUUIDString(ctl->conn, uuids[i]);
+    for (i = 0; i < list->nsecrets; i++) {
+        virSecretPtr sec = list->secrets[i];
          const char *usageType = NULL;

-        if (!sec) {
-            VIR_FREE(uuids[i]);
-            continue;
-        }
-
          switch (virSecretGetUsageType(sec)) {
          case VIR_SECRET_USAGE_TYPE_VOLUME:
              usageType = _("Volume");
              break;
          }

+        char uuid[VIR_UUID_STRING_BUFLEN];
+        if (virSecretGetUUIDString(list->secrets[i], uuid) < 0) {
+            vshError(ctl, "%s", _("Failed to get uuid of secret"));
+            goto cleanup;
+        }
+
          if (usageType) {
              vshPrint(ctl, "%-36s %s %s\n",
-                     uuids[i], usageType,
+                     uuid, usageType,
                       virSecretGetUsageID(sec));
          } else {
              vshPrint(ctl, "%-36s %s\n",
-                     uuids[i], _("Unused"));
+                     uuid, _("Unused"));
          }
-        virSecretFree(sec);
-        VIR_FREE(uuids[i]);
      }
-    VIR_FREE(uuids);
-    return true;
+
+    ret = true;
+
+cleanup:
+    vshSecretListFree(list);
+    return ret;
  }

  const vshCmdDef secretCmds[] = {
@@ -361,7 +512,7 @@ const vshCmdDef secretCmds[] = {
       info_secret_dumpxml, 0},
      {"secret-get-value", cmdSecretGetValue, opts_secret_get_value,
       info_secret_get_value, 0},
-    {"secret-list", cmdSecretList, NULL, info_secret_list, 0},
+    {"secret-list", cmdSecretList, opts_secret_list, info_secret_list, 0},
      {"secret-set-value", cmdSecretSetValue, opts_secret_set_value,
       info_secret_set_value, 0},
      {"secret-undefine", cmdSecretUndefine, opts_secret_undefine,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index a6390c2..9d35502 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2469,9 +2469,13 @@ encoded using Base64.
  Delete a I<secret> (specified by its UUID), including the associated value, if
  any.

-=item B<secret-list>
+=item B<secret-list> [I<--ephemeral>] [I<--no-ephemeral>]
+                     [I<--private>] [I<--no-private>]

-Output a list of UUIDs of known secrets to stdout.
+Returns the list of secrets. You may also want to filter the returned secrets
+by I<--ephemeral> to list the ephemeral ones, I<--no-ephemeral> to list the
+not ephemeral ones, I<--private> to list the private ones, and

non-ephmeral

+I<--no-private> to list the ones not private.

not private ones.


  =back



ACK with the nits fixed. It would be helpful if Eric could post his opinion on the English strings as I don't feel competent enough.

Peter

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]