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

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



On 2012年09月14日 19:50, Peter Krempa wrote:
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.

Okay. I adopted these, and pushed the set, with nits fixed.

Osier


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