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

[Osier Yang]

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