[libvirt] [PATCH 5/5] list: Use virConnectListAllSecrets in virsh
Peter Krempa
pkrempa at redhat.com
Fri Sep 14 11:50:41 UTC 2012
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
More information about the libvir-list
mailing list