[libvirt] [PATCH 0/5 v5] Atomic API to list secrets
v4 - v5: * Support filter the returned secrets by its properties "ephemeral" and "private". Osier Yang (5): list: Define new API virConnectListAllSecrets list: Implement RPC calls for virConnectListAllSecrets list: Implement listAllSecrets list: Expose virConnectListAllSecrets to Python binding list: Use virConnectListAllSecrets in virsh daemon/remote.c | 54 +++++++++ include/libvirt/libvirt.h.in | 21 ++++ python/generator.py | 1 + python/libvirt-override-api.xml | 6 + python/libvirt-override-virConnect.py | 12 ++ python/libvirt-override.c | 48 ++++++++ src/conf/secret_conf.h | 12 ++ src/driver.h | 5 + src/libvirt.c | 66 +++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 64 ++++++++++ src/remote/remote_protocol.x | 13 ++- src/remote_protocol-structs | 12 ++ src/secret/secret_driver.c | 82 +++++++++++++- tools/virsh-secret.c | 209 ++++++++++++++++++++++++++++----- tools/virsh.pod | 8 +- 16 files changed, 581 insertions(+), 33 deletions(-) -- 1.7.7.3
This is to list the secret objects. No flags are supported include/libvirt/libvirt.h.in: Declare enum virConnectListAllSecretFlags and virConnectListAllSecrets. python/generator.py: Skip auto-generating src/driver.h: (virDrvConnectListAllSecrets) src/libvirt.c: Implement the public API src/libvirt_public.syms: Export the symbol to public --- include/libvirt/libvirt.h.in | 21 +++++++++++++ python/generator.py | 1 + src/driver.h | 5 +++ src/libvirt.c | 66 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 5 files changed, 94 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 3d41026..c38ab23 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3266,6 +3266,27 @@ int virConnectNumOfSecrets (virConnectPtr conn); int virConnectListSecrets (virConnectPtr conn, char **uuids, int maxuuids); + +/* + * virConnectListAllSecrets: + * + * Flags used to filter the returned secrets. Flags in each group + * are exclusive attributes of a secret. + */ +typedef enum { + /* kept in memory, never stored persistently */ + VIR_CONNECT_LIST_SECRETS_EPHEMERAL = 1 << 0, + VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL = 1 << 1, + + /* not revealed to any caller of libvirt, nor + * to any other node */ + VIR_CONNECT_LIST_SECRETS_PRIVATE = 1 << 2, + VIR_CONNECT_LIST_SECRETS_NO_PRIVATE = 1 << 3, +} virConnectListAllSecretsFlags; + +int virConnectListAllSecrets(virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags); virSecretPtr virSecretLookupByUUID(virConnectPtr conn, const unsigned char *uuid); virSecretPtr virSecretLookupByUUIDString(virConnectPtr conn, diff --git a/python/generator.py b/python/generator.py index d3163e4..955c893 100755 --- a/python/generator.py +++ b/python/generator.py @@ -466,6 +466,7 @@ skip_function = ( 'virConnectListAllInterfaces', # overridden in virConnect.py 'virConnectListAllNodeDevices', # overridden in virConnect.py 'virConnectListAllNWFilters', # overridden in virConnect.py + 'virConnectListAllSecrets', # overridden in virConnect.py 'virStreamRecvAll', # Pure python libvirt-override-virStream.py 'virStreamSendAll', # Pure python libvirt-override-virStream.py diff --git a/src/driver.h b/src/driver.h index 9984a85..3e69dae 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1567,6 +1567,10 @@ typedef int (*virDrvListSecrets) (virConnectPtr conn, char **uuids, int maxuuids); +typedef int + (*virDrvListAllSecrets) (virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags); typedef struct _virSecretDriver virSecretDriver; typedef virSecretDriver *virSecretDriverPtr; @@ -1588,6 +1592,7 @@ struct _virSecretDriver { virDrvNumOfSecrets numOfSecrets; virDrvListSecrets listSecrets; + virDrvListAllSecrets listAllSecrets; virDrvSecretLookupByUUID lookupByUUID; virDrvSecretLookupByUsage lookupByUsage; virDrvSecretDefineXML defineXML; diff --git a/src/libvirt.c b/src/libvirt.c index cae9bc9..6a9be6d 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -14594,6 +14594,72 @@ error: } /** + * virConnectListAllSecrets: + * @conn: Pointer to the hypervisor connection. + * @secrets: Pointer to a variable to store the array containing the secret + * objects or NULL if the list is not required (just returns the + * number of secrets). + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Collect the list of secrets, and allocate an array to store those + * objects. + * + * Normally, all secrets are returned; however, @flags can be used to + * filter the results for a smaller list of targeted secrets. The valid + * flags are divided into groups, where each group contains bits that + * describe mutually exclusive attributes of a secret, and where all bits + * within a group describe all possible secrets. + * + * The first group of @flags is VIR_CONNECT_LIST_SECRETS_EPHEMERAL(kept in + * memory, not persistent) and VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL + * (nor ephemeral) to filter the secrets by whether it's ephemeral or not. + * + * The second group of @flags is VIR_CONNECT_LIST_SECRETS_PRIVATE + * (not revealed to any caller of libvirt, nor to any other node) + * and VIR_CONNECT_LIST_SECRETS_NO_PRIVATE (not private), to filter + * the secrets by whether it's private or not. + * + * Returns the number of secrets found or -1 and sets @secrets to NULL in case + * of error. On success, the array stored into @secrets is guaranteed to + * have an extra allocated element set to NULL but not included in the return count, + * to make iteration easier. The caller is responsible for calling + * virSecretFree() on each array element, then calling free() on @secrets. + */ +int +virConnectListAllSecrets(virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, secrets=%p, flags=%x", conn, secrets, flags); + + virResetLastError(); + + if (secrets) + *secrets = NULL; + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (conn->secretDriver && + conn->secretDriver->listAllSecrets) { + int ret; + ret = conn->secretDriver->listAllSecrets(conn, secrets, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return -1; +} + +/** * virConnectListSecrets: * @conn: virConnect connection * @uuids: Pointer to an array to store the UUIDs diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index a918bc8..828b315 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -560,6 +560,7 @@ LIBVIRT_0.10.2 { virConnectListAllNetworks; virConnectListAllNodeDevices; virConnectListAllNWFilters; + virConnectListAllSecrets; virConnectListAllStoragePools; virStoragePoolListAllVolumes; } LIBVIRT_0.10.0; -- 1.7.7.3
On 09/14/12 10:38, Osier Yang wrote:
This is to list the secret objects. No flags are supported
This statement isn't accurate as you added filtering flags.
include/libvirt/libvirt.h.in: Declare enum virConnectListAllSecretFlags and virConnectListAllSecrets. python/generator.py: Skip auto-generating src/driver.h: (virDrvConnectListAllSecrets) src/libvirt.c: Implement the public API src/libvirt_public.syms: Export the symbol to public --- include/libvirt/libvirt.h.in | 21 +++++++++++++ python/generator.py | 1 + src/driver.h | 5 +++ src/libvirt.c | 66 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 5 files changed, 94 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 3d41026..c38ab23 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3266,6 +3266,27 @@ int virConnectNumOfSecrets (virConnectPtr conn); int virConnectListSecrets (virConnectPtr conn, char **uuids, int maxuuids); + +/* + * virConnectListAllSecrets: + * + * Flags used to filter the returned secrets. Flags in each group + * are exclusive attributes of a secret. + */ +typedef enum { + /* kept in memory, never stored persistently */ + VIR_CONNECT_LIST_SECRETS_EPHEMERAL = 1 << 0, + VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL = 1 << 1, + + /* not revealed to any caller of libvirt, nor + * to any other node */ + VIR_CONNECT_LIST_SECRETS_PRIVATE = 1 << 2, + VIR_CONNECT_LIST_SECRETS_NO_PRIVATE = 1 << 3, +} virConnectListAllSecretsFlags; + +int virConnectListAllSecrets(virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags); virSecretPtr virSecretLookupByUUID(virConnectPtr conn, const unsigned char *uuid); virSecretPtr virSecretLookupByUUIDString(virConnectPtr conn, diff --git a/python/generator.py b/python/generator.py index d3163e4..955c893 100755 --- a/python/generator.py +++ b/python/generator.py @@ -466,6 +466,7 @@ skip_function = ( 'virConnectListAllInterfaces', # overridden in virConnect.py 'virConnectListAllNodeDevices', # overridden in virConnect.py 'virConnectListAllNWFilters', # overridden in virConnect.py + 'virConnectListAllSecrets', # overridden in virConnect.py
'virStreamRecvAll', # Pure python libvirt-override-virStream.py 'virStreamSendAll', # Pure python libvirt-override-virStream.py diff --git a/src/driver.h b/src/driver.h index 9984a85..3e69dae 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1567,6 +1567,10 @@ typedef int (*virDrvListSecrets) (virConnectPtr conn, char **uuids, int maxuuids); +typedef int + (*virDrvListAllSecrets) (virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags);
typedef struct _virSecretDriver virSecretDriver; typedef virSecretDriver *virSecretDriverPtr; @@ -1588,6 +1592,7 @@ struct _virSecretDriver {
virDrvNumOfSecrets numOfSecrets; virDrvListSecrets listSecrets; + virDrvListAllSecrets listAllSecrets; virDrvSecretLookupByUUID lookupByUUID; virDrvSecretLookupByUsage lookupByUsage; virDrvSecretDefineXML defineXML; diff --git a/src/libvirt.c b/src/libvirt.c index cae9bc9..6a9be6d 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -14594,6 +14594,72 @@ error: }
/** + * virConnectListAllSecrets: + * @conn: Pointer to the hypervisor connection. + * @secrets: Pointer to a variable to store the array containing the secret + * objects or NULL if the list is not required (just returns the + * number of secrets). + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Collect the list of secrets, and allocate an array to store those + * objects. + * + * Normally, all secrets are returned; however, @flags can be used to + * filter the results for a smaller list of targeted secrets. The valid + * flags are divided into groups, where each group contains bits that + * describe mutually exclusive attributes of a secret, and where all bits + * within a group describe all possible secrets. + * + * The first group of @flags is VIR_CONNECT_LIST_SECRETS_EPHEMERAL(kept in + * memory, not persistent) and VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL + * (nor ephemeral) to filter the secrets by whether it's ephemeral or not.
I'd rephrase this paragraph a little bit: The first group of @flags is used to filter the list by storage location of the secret. Flag VIR_CONNECT_LIST_SECRETS_EPHEMERAL selects secrets that are kept only in memory. Flag VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL selects secrets that are allowed to be kept in persistent storage. (In any case, it'd be best if Eric would state his opinion on this :) )
+ * + * The second group of @flags is VIR_CONNECT_LIST_SECRETS_PRIVATE + * (not revealed to any caller of libvirt, nor to any other node) + * and VIR_CONNECT_LIST_SECRETS_NO_PRIVATE (not private), to filter + * the secrets by whether it's private or not.
This paragraph also contains a lot of redundant information: The second group of @flags allows filtering secrets by privacy. When flag VIR_CONNECT_LIST_SECRETS_PRIVATE is specified secrets that are never revealed to any caller of libvirt nor to any other node are returned. Flag VIR_CONNECT_LIST_SECRETS_NO_PRIVATE can be used to select non-private secrets.
+ * + * Returns the number of secrets found or -1 and sets @secrets to NULL in case + * of error. On success, the array stored into @secrets is guaranteed to + * have an extra allocated element set to NULL but not included in the return count, + * to make iteration easier. The caller is responsible for calling + * virSecretFree() on each array element, then calling free() on @secrets. + */ +int +virConnectListAllSecrets(virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, secrets=%p, flags=%x", conn, secrets, flags); + + virResetLastError(); + + if (secrets) + *secrets = NULL; + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (conn->secretDriver && + conn->secretDriver->listAllSecrets) { + int ret; + ret = conn->secretDriver->listAllSecrets(conn, secrets, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return -1; +} + +/** * virConnectListSecrets: * @conn: virConnect connection * @uuids: Pointer to an array to store the UUIDs diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index a918bc8..828b315 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -560,6 +560,7 @@ LIBVIRT_0.10.2 { virConnectListAllNetworks; virConnectListAllNodeDevices; virConnectListAllNWFilters; + virConnectListAllSecrets; virConnectListAllStoragePools; virStoragePoolListAllVolumes; } LIBVIRT_0.10.0;
Otherwise the code is OK. So ACK if somebody speaks his opinion on the docs and you fix the commit message. Peter
On 09/14/12 11:47, Peter Krempa wrote:
On 09/14/12 10:38, Osier Yang wrote:
This is to list the secret objects. No flags are supported
This statement isn't accurate as you added filtering flags.
include/libvirt/libvirt.h.in: Declare enum virConnectListAllSecretFlags and virConnectListAllSecrets. python/generator.py: Skip auto-generating src/driver.h: (virDrvConnectListAllSecrets) src/libvirt.c: Implement the public API src/libvirt_public.syms: Export the symbol to public --- include/libvirt/libvirt.h.in | 21 +++++++++++++ python/generator.py | 1 + src/driver.h | 5 +++ src/libvirt.c | 66 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 5 files changed, 94 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 3d41026..c38ab23 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3266,6 +3266,27 @@ int virConnectNumOfSecrets (virConnectPtr conn); int virConnectListSecrets (virConnectPtr conn, char **uuids, int maxuuids); + +/* + * virConnectListAllSecrets: + * + * Flags used to filter the returned secrets. Flags in each group + * are exclusive attributes of a secret. + */ +typedef enum { + /* kept in memory, never stored persistently */ + VIR_CONNECT_LIST_SECRETS_EPHEMERAL = 1 << 0, + VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL = 1 << 1, + + /* not revealed to any caller of libvirt, nor + * to any other node */ + VIR_CONNECT_LIST_SECRETS_PRIVATE = 1 << 2, + VIR_CONNECT_LIST_SECRETS_NO_PRIVATE = 1 << 3,
You also need to move the comments for this after the first value, otherwise the docs generator is confused and moves the comments "off-by-one". Peter
On 2012年09月14日 17:47, Peter Krempa wrote:
On 09/14/12 10:38, Osier Yang wrote:
This is to list the secret objects. No flags are supported
This statement isn't accurate as you added filtering flags.
include/libvirt/libvirt.h.in: Declare enum virConnectListAllSecretFlags and virConnectListAllSecrets. python/generator.py: Skip auto-generating src/driver.h: (virDrvConnectListAllSecrets) src/libvirt.c: Implement the public API src/libvirt_public.syms: Export the symbol to public --- include/libvirt/libvirt.h.in | 21 +++++++++++++ python/generator.py | 1 + src/driver.h | 5 +++ src/libvirt.c | 66 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 5 files changed, 94 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 3d41026..c38ab23 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3266,6 +3266,27 @@ int virConnectNumOfSecrets (virConnectPtr conn); int virConnectListSecrets (virConnectPtr conn, char **uuids, int maxuuids); + +/* + * virConnectListAllSecrets: + * + * Flags used to filter the returned secrets. Flags in each group + * are exclusive attributes of a secret. + */ +typedef enum { + /* kept in memory, never stored persistently */ + VIR_CONNECT_LIST_SECRETS_EPHEMERAL = 1 << 0, + VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL = 1 << 1, + + /* not revealed to any caller of libvirt, nor + * to any other node */ + VIR_CONNECT_LIST_SECRETS_PRIVATE = 1 << 2, + VIR_CONNECT_LIST_SECRETS_NO_PRIVATE = 1 << 3, +} virConnectListAllSecretsFlags; + +int virConnectListAllSecrets(virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags); virSecretPtr virSecretLookupByUUID(virConnectPtr conn, const unsigned char *uuid); virSecretPtr virSecretLookupByUUIDString(virConnectPtr conn, diff --git a/python/generator.py b/python/generator.py index d3163e4..955c893 100755 --- a/python/generator.py +++ b/python/generator.py @@ -466,6 +466,7 @@ skip_function = ( 'virConnectListAllInterfaces', # overridden in virConnect.py 'virConnectListAllNodeDevices', # overridden in virConnect.py 'virConnectListAllNWFilters', # overridden in virConnect.py + 'virConnectListAllSecrets', # overridden in virConnect.py
'virStreamRecvAll', # Pure python libvirt-override-virStream.py 'virStreamSendAll', # Pure python libvirt-override-virStream.py diff --git a/src/driver.h b/src/driver.h index 9984a85..3e69dae 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1567,6 +1567,10 @@ typedef int (*virDrvListSecrets) (virConnectPtr conn, char **uuids, int maxuuids); +typedef int + (*virDrvListAllSecrets) (virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags);
typedef struct _virSecretDriver virSecretDriver; typedef virSecretDriver *virSecretDriverPtr; @@ -1588,6 +1592,7 @@ struct _virSecretDriver {
virDrvNumOfSecrets numOfSecrets; virDrvListSecrets listSecrets; + virDrvListAllSecrets listAllSecrets; virDrvSecretLookupByUUID lookupByUUID; virDrvSecretLookupByUsage lookupByUsage; virDrvSecretDefineXML defineXML; diff --git a/src/libvirt.c b/src/libvirt.c index cae9bc9..6a9be6d 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -14594,6 +14594,72 @@ error: }
/** + * virConnectListAllSecrets: + * @conn: Pointer to the hypervisor connection. + * @secrets: Pointer to a variable to store the array containing the secret + * objects or NULL if the list is not required (just returns the + * number of secrets). + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Collect the list of secrets, and allocate an array to store those + * objects. + * + * Normally, all secrets are returned; however, @flags can be used to + * filter the results for a smaller list of targeted secrets. The valid + * flags are divided into groups, where each group contains bits that + * describe mutually exclusive attributes of a secret, and where all bits + * within a group describe all possible secrets. + * + * The first group of @flags is VIR_CONNECT_LIST_SECRETS_EPHEMERAL(kept in + * memory, not persistent) and VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL + * (nor ephemeral) to filter the secrets by whether it's ephemeral or not.
I'd rephrase this paragraph a little bit:
The first group of @flags is used to filter the list by storage location of the secret. Flag VIR_CONNECT_LIST_SECRETS_EPHEMERAL selects secrets that are kept only in memory. Flag VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL selects secrets that are allowed to be kept in persistent storage.
(In any case, it'd be best if Eric would state his opinion on this :) )
+ * + * The second group of @flags is VIR_CONNECT_LIST_SECRETS_PRIVATE + * (not revealed to any caller of libvirt, nor to any other node) + * and VIR_CONNECT_LIST_SECRETS_NO_PRIVATE (not private), to filter + * the secrets by whether it's private or not.
This paragraph also contains a lot of redundant information:
The second group of @flags allows filtering secrets by privacy. When flag VIR_CONNECT_LIST_SECRETS_PRIVATE is specified secrets that are never revealed to any caller of libvirt nor to any other node are returned. Flag VIR_CONNECT_LIST_SECRETS_NO_PRIVATE can be used to select non-private secrets.
I reword your suggestion a bit like: * The first group of @flags is used to filter secrets by its storage * location. Flag VIR_CONNECT_LIST_SECRETS_EPHEMERAL * selects secrets that are kept only in memory. Flag * VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL selects secrets that are * kept in persistent storage. * * The second group of @flags is used to filter secrets by privacy. * Flag VIR_CONNECT_LIST_SECRETS_PRIVATE seclets secrets that are * never revealed to any caller of libvirt nor to any other node. * Flag VIR_CONNECT_LIST_SECRETS_NO_PRIVATE selects non-private secrets. Osier
The RPC generator doesn't support returning list of object yet, this patch do the work manually. * daemon/remote.c: Implemente the server side handler remoteDispatchConnectListAllSecrets. * src/remote/remote_driver.c: Add remote driver handler remoteConnectListAllSecrets. * src/remote/remote_protocol.x: New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_SECRETS and structs to represent the args and ret for it. * src/remote_protocol-structs: Likewise. --- daemon/remote.c | 54 +++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 64 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 13 ++++++++- src/remote_protocol-structs | 12 ++++++++ 4 files changed, 142 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index abd7edf..6f20761 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4429,6 +4429,60 @@ cleanup: return rv; } +static int +remoteDispatchConnectListAllSecrets(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_connect_list_all_secrets_args *args, + remote_connect_list_all_secrets_ret *ret) +{ + virSecretPtr *secrets = NULL; + int nsecrets = 0; + int i; + int rv = -1; + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if ((nsecrets = virConnectListAllSecrets(priv->conn, + args->need_results ? &secrets : NULL, + args->flags)) < 0) + goto cleanup; + + if (secrets && nsecrets) { + if (VIR_ALLOC_N(ret->secrets.secrets_val, nsecrets) < 0) { + virReportOOMError(); + goto cleanup; + } + + ret->secrets.secrets_len = nsecrets; + + for (i = 0; i < nsecrets; i++) + make_nonnull_secret(ret->secrets.secrets_val + i, secrets[i]); + } else { + ret->secrets.secrets_len = 0; + ret->secrets.secrets_val = NULL; + } + + ret->ret = nsecrets; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (secrets) { + for (i = 0; i < nsecrets; i++) + virSecretFree(secrets[i]); + VIR_FREE(secrets); + } + return rv; +} + /*----- Helpers. -----*/ /* get_nonnull_domain and get_nonnull_network turn an on-wire diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f5bb6ac..ee2cd50 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2970,6 +2970,69 @@ done: return rv; } +static int +remoteConnectListAllSecrets(virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags) +{ + int rv = -1; + int i; + virSecretPtr *tmp_secrets = NULL; + remote_connect_list_all_secrets_args args; + remote_connect_list_all_secrets_ret ret; + + struct private_data *priv = conn->privateData; + + remoteDriverLock(priv); + + args.need_results = !!secrets; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + if (call(conn, + priv, + 0, + REMOTE_PROC_CONNECT_LIST_ALL_SECRETS, + (xdrproc_t) xdr_remote_connect_list_all_secrets_args, + (char *) &args, + (xdrproc_t) xdr_remote_connect_list_all_secrets_ret, + (char *) &ret) == -1) + goto done; + + if (secrets) { + if (VIR_ALLOC_N(tmp_secrets, ret.secrets.secrets_len + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0; i < ret.secrets.secrets_len; i++) { + tmp_secrets[i] = get_nonnull_secret (conn, ret.secrets.secrets_val[i]); + if (!tmp_secrets[i]) { + virReportOOMError(); + goto cleanup; + } + } + *secrets = tmp_secrets; + tmp_secrets = NULL; + } + + rv = ret.ret; + +cleanup: + if (tmp_secrets) { + for (i = 0; i < ret.secrets.secrets_len; i++) + if (tmp_secrets[i]) + virSecretFree(tmp_secrets[i]); + VIR_FREE(tmp_secrets); + } + + xdr_free((xdrproc_t) xdr_remote_connect_list_all_secrets_ret, (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + /*----------------------------------------------------------------------*/ static virDrvOpenStatus ATTRIBUTE_NONNULL (1) @@ -6047,6 +6110,7 @@ static virSecretDriver secret_driver = { .close = remoteSecretClose, /* 0.7.1 */ .numOfSecrets = remoteNumOfSecrets, /* 0.7.1 */ .listSecrets = remoteListSecrets, /* 0.7.1 */ + .listAllSecrets = remoteConnectListAllSecrets, /* 0.10.2 */ .lookupByUUID = remoteSecretLookupByUUID, /* 0.7.1 */ .lookupByUsage = remoteSecretLookupByUsage, /* 0.7.1 */ .defineXML = remoteSecretDefineXML, /* 0.7.1 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 46269c8..1fc7f25 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2619,6 +2619,16 @@ struct remote_connect_list_all_nwfilters_ret { unsigned int ret; }; +struct remote_connect_list_all_secrets_args { + int need_results; + unsigned int flags; +}; + +struct remote_connect_list_all_secrets_ret { + remote_nonnull_secret secrets<>; + unsigned int ret; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -2955,7 +2965,8 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_NETWORKS = 283, /* skipgen skipgen priority:high */ REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES = 284, /* skipgen skipgen priority:high */ REMOTE_PROC_CONNECT_LIST_ALL_NODE_DEVICES = 285, /* skipgen skipgen priority:high */ - REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS = 286 /* skipgen skipgen priority:high */ + REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS = 286, /* skipgen skipgen priority:high */ + REMOTE_PROC_CONNECT_LIST_ALL_SECRETS = 287 /* skipgen skipgen priority:high */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index a465cf3..86416ad 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2078,6 +2078,17 @@ struct remote_connect_list_all_nwfilters_ret { } filters; u_int ret; }; +struct remote_list_all_secrets_args { + int need_results; + u_int flags; +}; +struct remote_list_all_secrets_ret { + struct { + u_int secrets_len; + remote_nonnull_network * secrets_val; + } secrets; + u_int ret; +}; enum remote_procedure { REMOTE_PROC_OPEN = 1, REMOTE_PROC_CLOSE = 2, @@ -2365,4 +2376,5 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES = 284, REMOTE_PROC_CONNECT_LIST_ALL_NODE_DEVICES = 285, REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS = 286, + REMOTE_PROC_CONNECT_LIST_ALL_SECRETS = 287, }; -- 1.7.7.3
On 09/14/12 10:38, Osier Yang wrote:
The RPC generator doesn't support returning list of object yet, this patch do the work manually.
s/do/does/
* daemon/remote.c: Implemente the server side handler remoteDispatchConnectListAllSecrets.
s/Implemente/Implement/
* src/remote/remote_driver.c: Add remote driver handler remoteConnectListAllSecrets.
* src/remote/remote_protocol.x: New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_SECRETS and structs to represent the args and ret for it.
* src/remote_protocol-structs: Likewise. --- daemon/remote.c | 54 +++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 64 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 13 ++++++++- src/remote_protocol-structs | 12 ++++++++ 4 files changed, 142 insertions(+), 1 deletions(-)
...
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 46269c8..1fc7f25 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2619,6 +2619,16 @@ struct remote_connect_list_all_nwfilters_ret { unsigned int ret; };
+struct remote_connect_list_all_secrets_args { + int need_results; + unsigned int flags; +}; + +struct remote_connect_list_all_secrets_ret { + remote_nonnull_secret secrets<>; + unsigned int ret; +}; + /*----- Protocol. -----*/
/* Define the program number, protocol version and procedure numbers here. */ @@ -2955,7 +2965,8 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_NETWORKS = 283, /* skipgen skipgen priority:high */ REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES = 284, /* skipgen skipgen priority:high */ REMOTE_PROC_CONNECT_LIST_ALL_NODE_DEVICES = 285, /* skipgen skipgen priority:high */ - REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS = 286 /* skipgen skipgen priority:high */ + REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS = 286, /* skipgen skipgen priority:high */ + REMOTE_PROC_CONNECT_LIST_ALL_SECRETS = 287 /* skipgen skipgen priority:high */
/* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index a465cf3..86416ad 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2078,6 +2078,17 @@ struct remote_connect_list_all_nwfilters_ret { } filters; u_int ret; }; +struct remote_list_all_secrets_args { + int need_results; + u_int flags; +}; +struct remote_list_all_secrets_ret { + struct { + u_int secrets_len; + remote_nonnull_network * secrets_val;
I think I raised this in the last version too. This should be remote_nonnull_secret instead of remote_nonnull_network.
+ } secrets; + u_int ret; +}; enum remote_procedure { REMOTE_PROC_OPEN = 1, REMOTE_PROC_CLOSE = 2, @@ -2365,4 +2376,5 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES = 284, REMOTE_PROC_CONNECT_LIST_ALL_NODE_DEVICES = 285, REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS = 286, + REMOTE_PROC_CONNECT_LIST_ALL_SECRETS = 287, };
ACK if you fix the commit message and change the struct type in src/remote_protocol-structs. Peter
Simply returns the object list. No filtering. src/secret/secret_driver.c: Implement listAllSecrets --- src/conf/secret_conf.h | 12 ++++++ src/secret/secret_driver.c | 82 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 1 deletions(-) diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h index 511c52d..02e1bf9 100644 --- a/src/conf/secret_conf.h +++ b/src/conf/secret_conf.h @@ -47,4 +47,16 @@ virSecretDefPtr virSecretDefParseString(const char *xml); virSecretDefPtr virSecretDefParseFile(const char *filename); char *virSecretDefFormat(const virSecretDefPtr def); +# define VIR_CONNECT_LIST_SECRETS_FILTERS_EPHEMERAL \ + (VIR_CONNECT_LIST_SECRETS_EPHEMERAL | \ + VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL) + +# define VIR_CONNECT_LIST_SECRETS_FILTERS_PRIVATE \ + (VIR_CONNECT_LIST_SECRETS_PRIVATE | \ + VIR_CONNECT_LIST_SECRETS_NO_PRIVATE) + +# define VIR_CONNECT_LIST_SECRETS_FILTERS_ALL \ + (VIR_CONNECT_LIST_SECRETS_FILTERS_EPHEMERAL | \ + VIR_CONNECT_LIST_SECRETS_FILTERS_PRIVATE) + #endif diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 7f92776..2ad66dd 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -601,7 +601,6 @@ cleanup: return -1; } - static const char * secretUsageIDForDef(virSecretDefPtr def) { @@ -620,6 +619,86 @@ secretUsageIDForDef(virSecretDefPtr def) } } +#define MATCH(FLAG) (flags & (FLAG)) +static int +secretListAllSecrets(virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags) { + virSecretDriverStatePtr driver = conn->secretPrivateData; + virSecretPtr *tmp_secrets = NULL; + int nsecrets = 0; + int ret_nsecrets = 0; + virSecretPtr secret = NULL; + virSecretEntryPtr entry = NULL; + int i = 0; + int ret = -1; + + virCheckFlags(VIR_CONNECT_LIST_SECRETS_FILTERS_ALL, -1); + + secretDriverLock(driver); + + for (entry = driver->secrets; entry != NULL; entry = entry->next) + nsecrets++; + + if (secrets) { + if (VIR_ALLOC_N(tmp_secrets, nsecrets + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + } + + for (entry = driver->secrets; entry != NULL; entry = entry->next) { + /* filter by whether it's ephemeral */ + if (MATCH(VIR_CONNECT_LIST_SECRETS_FILTERS_EPHEMERAL) && + !((MATCH(VIR_CONNECT_LIST_SECRETS_EPHEMERAL) && + entry->def->ephemeral) || + (MATCH(VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL) && + !entry->def->ephemeral))) + continue; + + /* filter by whether it's private */ + if (MATCH(VIR_CONNECT_LIST_SECRETS_FILTERS_PRIVATE) && + !((MATCH(VIR_CONNECT_LIST_SECRETS_PRIVATE) && + entry->def->private) || + (MATCH(VIR_CONNECT_LIST_SECRETS_NO_PRIVATE) && + !entry->def->private))) + continue; + + if (secrets) { + if (!(secret = virGetSecret(conn, + entry->def->uuid, + entry->def->usage_type, + secretUsageIDForDef(entry->def)))) + goto cleanup; + tmp_secrets[ret_nsecrets] = secret; + } + ret_nsecrets++; + } + + if (tmp_secrets) { + /* trim the array to the final size */ + ignore_value(VIR_REALLOC_N(tmp_secrets, ret_nsecrets + 1)); + *secrets = tmp_secrets; + tmp_secrets = NULL; + } + + ret = ret_nsecrets; + + cleanup: + secretDriverUnlock(driver); + if (tmp_secrets) { + for (i = 0; i < ret_nsecrets; i ++) { + if (tmp_secrets[i]) + virSecretFree(tmp_secrets[i]); + } + } + VIR_FREE(tmp_secrets); + + return ret; +} +#undef MATCH + + static virSecretPtr secretLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { @@ -1072,6 +1151,7 @@ static virSecretDriver secretDriver = { .close = secretClose, /* 0.7.1 */ .numOfSecrets = secretNumOfSecrets, /* 0.7.1 */ .listSecrets = secretListSecrets, /* 0.7.1 */ + .listAllSecrets = secretListAllSecrets, /* 0.10.2 */ .lookupByUUID = secretLookupByUUID, /* 0.7.1 */ .lookupByUsage = secretLookupByUsage, /* 0.7.1 */ .defineXML = secretDefineXML, /* 0.7.1 */ -- 1.7.7.3
On 09/14/12 10:38, Osier Yang wrote:
Simply returns the object list. No filtering.
Again, this statement isn't true any more.
src/secret/secret_driver.c: Implement listAllSecrets --- src/conf/secret_conf.h | 12 ++++++ src/secret/secret_driver.c | 82 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 1 deletions(-)
diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h index 511c52d..02e1bf9 100644 --- a/src/conf/secret_conf.h +++ b/src/conf/secret_conf.h @@ -47,4 +47,16 @@ virSecretDefPtr virSecretDefParseString(const char *xml); virSecretDefPtr virSecretDefParseFile(const char *filename); char *virSecretDefFormat(const virSecretDefPtr def);
+# define VIR_CONNECT_LIST_SECRETS_FILTERS_EPHEMERAL \ + (VIR_CONNECT_LIST_SECRETS_EPHEMERAL | \ + VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL) + +# define VIR_CONNECT_LIST_SECRETS_FILTERS_PRIVATE \ + (VIR_CONNECT_LIST_SECRETS_PRIVATE | \ + VIR_CONNECT_LIST_SECRETS_NO_PRIVATE) + +# define VIR_CONNECT_LIST_SECRETS_FILTERS_ALL \ + (VIR_CONNECT_LIST_SECRETS_FILTERS_EPHEMERAL | \ + VIR_CONNECT_LIST_SECRETS_FILTERS_PRIVATE) + #endif diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 7f92776..2ad66dd 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -601,7 +601,6 @@ cleanup: return -1; }
- static const char * secretUsageIDForDef(virSecretDefPtr def) { @@ -620,6 +619,86 @@ secretUsageIDForDef(virSecretDefPtr def) } }
+#define MATCH(FLAG) (flags & (FLAG)) +static int +secretListAllSecrets(virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags) {
Style nit: put the opening curly bracket on a new line.
+ virSecretDriverStatePtr driver = conn->secretPrivateData; + virSecretPtr *tmp_secrets = NULL; + int nsecrets = 0; + int ret_nsecrets = 0; + virSecretPtr secret = NULL; + virSecretEntryPtr entry = NULL; + int i = 0; + int ret = -1; + + virCheckFlags(VIR_CONNECT_LIST_SECRETS_FILTERS_ALL, -1); + + secretDriverLock(driver); + + for (entry = driver->secrets; entry != NULL; entry = entry->next) + nsecrets++; + + if (secrets) { + if (VIR_ALLOC_N(tmp_secrets, nsecrets + 1) < 0) {
Those two conditions can be merged into one easily.
+ virReportOOMError(); + goto cleanup; + } + } + + for (entry = driver->secrets; entry != NULL; entry = entry->next) { + /* filter by whether it's ephemeral */ + if (MATCH(VIR_CONNECT_LIST_SECRETS_FILTERS_EPHEMERAL) && + !((MATCH(VIR_CONNECT_LIST_SECRETS_EPHEMERAL) && + entry->def->ephemeral) || + (MATCH(VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL) && + !entry->def->ephemeral))) + continue; + + /* filter by whether it's private */ + if (MATCH(VIR_CONNECT_LIST_SECRETS_FILTERS_PRIVATE) && + !((MATCH(VIR_CONNECT_LIST_SECRETS_PRIVATE) && + entry->def->private) || + (MATCH(VIR_CONNECT_LIST_SECRETS_NO_PRIVATE) && + !entry->def->private))) + continue; + + if (secrets) { + if (!(secret = virGetSecret(conn, + entry->def->uuid, + entry->def->usage_type, + secretUsageIDForDef(entry->def)))) + goto cleanup; + tmp_secrets[ret_nsecrets] = secret; + } + ret_nsecrets++; + } + + if (tmp_secrets) { + /* trim the array to the final size */ + ignore_value(VIR_REALLOC_N(tmp_secrets, ret_nsecrets + 1)); + *secrets = tmp_secrets; + tmp_secrets = NULL; + } + + ret = ret_nsecrets; + + cleanup: + secretDriverUnlock(driver); + if (tmp_secrets) { + for (i = 0; i < ret_nsecrets; i ++) { + if (tmp_secrets[i]) + virSecretFree(tmp_secrets[i]); + } + } + VIR_FREE(tmp_secrets); + + return ret; +} +#undef MATCH + + static virSecretPtr secretLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { @@ -1072,6 +1151,7 @@ static virSecretDriver secretDriver = { .close = secretClose, /* 0.7.1 */ .numOfSecrets = secretNumOfSecrets, /* 0.7.1 */ .listSecrets = secretListSecrets, /* 0.7.1 */ + .listAllSecrets = secretListAllSecrets, /* 0.10.2 */ .lookupByUUID = secretLookupByUUID, /* 0.7.1 */ .lookupByUsage = secretLookupByUsage, /* 0.7.1 */ .defineXML = secretDefineXML, /* 0.7.1 */
ACK with the commit message fixed. The code is correct and you don't need to adress those style nits strictly. Peter
The implementation is done manually as the generator does not support wrapping lists of C pointers into Python objects. python/libvirt-override-api.xml: Document python/libvirt-override-virConnect.py: Implementation for listAllSecrets. python/libvirt-override.c: Implementation for the wrapper. --- python/libvirt-override-api.xml | 6 ++++ python/libvirt-override-virConnect.py | 12 ++++++++ python/libvirt-override.c | 48 +++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 0 deletions(-) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index b8ab823..4f609ee 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -368,6 +368,12 @@ <arg name='conn' type='virConnectPtr' info='virConnect connection'/> <return type='str *' info='the list of secret IDs or None in case of error'/> </function> + <function name='virConnectListAllSecrets' file='python'> + <info>returns list of all interfaces</info> + <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> + <arg name='flags' type='unsigned int' info='optional flags'/> + <return type='secret *' info='the list of secrets or None in case of error'/> + </function> <function name='virSecretSetValue' file='libvirt' module='libvirt'> <info>Associates a value with a secret.</info> <return type='int' info='0 on success, -1 on failure.'/> diff --git a/python/libvirt-override-virConnect.py b/python/libvirt-override-virConnect.py index caca982..6bec66d 100644 --- a/python/libvirt-override-virConnect.py +++ b/python/libvirt-override-virConnect.py @@ -266,3 +266,15 @@ retlist.append(virNWFilter(self, _obj=filter_ptr)) return retlist + + def listAllSecrets(self, flags): + """Returns a list of secret objects""" + ret = libvirtmod.virConnectListAllSecrets(self._o, flags) + if ret is None: + raise libvirtError("virConnectListAllSecrets() failed", conn=self) + + retlist = list() + for secret_ptr in ret: + retlist.append(virSecret(self, _obj=secret_ptr)) + + return retlist diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 0f8e4af..57f2c45 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3596,6 +3596,53 @@ libvirt_virConnectListSecrets(PyObject *self ATTRIBUTE_UNUSED, } static PyObject * +libvirt_virConnectListAllSecrets(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *pyobj_conn; + PyObject *py_retval = NULL; + PyObject *tmp = NULL; + virConnectPtr conn; + virSecretPtr *secrets = NULL; + int c_retval = 0; + int i; + unsigned int flags; + + if (!PyArg_ParseTuple(args, (char *)"Oi:virConnectListAllSecrets", + &pyobj_conn, &flags)) + return NULL; + conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virConnectListAllSecrets(conn, &secrets, flags); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) + return VIR_PY_NONE; + + if (!(py_retval = PyList_New(c_retval))) + goto cleanup; + + for (i = 0; i < c_retval; i++) { + if (!(tmp = libvirt_virSecretPtrWrap(secrets[i])) || + PyList_SetItem(py_retval, i, tmp) < 0) { + Py_XDECREF(tmp); + Py_DECREF(py_retval); + py_retval = NULL; + goto cleanup; + } + /* python steals the pointer */ + secrets[i] = NULL; + } + +cleanup: + for (i = 0; i < c_retval; i++) + if (secrets[i]) + virSecretFree(secrets[i]); + VIR_FREE(secrets); + return py_retval; +} + +static PyObject * libvirt_virSecretGetValue(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; @@ -6187,6 +6234,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virSecretGetUUIDString", libvirt_virSecretGetUUIDString, METH_VARARGS, NULL}, {(char *) "virSecretLookupByUUID", libvirt_virSecretLookupByUUID, METH_VARARGS, NULL}, {(char *) "virConnectListSecrets", libvirt_virConnectListSecrets, METH_VARARGS, NULL}, + {(char *) "virConnectListAllSecrets", libvirt_virConnectListAllSecrets, METH_VARARGS, NULL}, {(char *) "virSecretGetValue", libvirt_virSecretGetValue, METH_VARARGS, NULL}, {(char *) "virSecretSetValue", libvirt_virSecretSetValue, METH_VARARGS, NULL}, {(char *) "virNWFilterGetUUID", libvirt_virNWFilterGetUUID, METH_VARARGS, NULL}, -- 1.7.7.3
On 09/14/12 10:38, Osier Yang wrote:
The implementation is done manually as the generator does not support wrapping lists of C pointers into Python objects.
python/libvirt-override-api.xml: Document
python/libvirt-override-virConnect.py: Implementation for listAllSecrets.
python/libvirt-override.c: Implementation for the wrapper. --- python/libvirt-override-api.xml | 6 ++++ python/libvirt-override-virConnect.py | 12 ++++++++ python/libvirt-override.c | 48 +++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 0 deletions(-)
ACK. Peter
This introduce four new options for secret-list, to filter the 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 (list->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")}, + {"private", VSH_OT_BOOL, 0, N_("list private secrets")}, + {"no-private", VSH_OT_BOOL, 0, N_("list secrets not private")}, + {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; - 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 +I<--no-private> to list the ones not private. =back -- 1.7.7.3
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
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
participants (2)
-
Osier Yang -
Peter Krempa