On Fri, Sep 11, 2009 at 03:19:18PM +0100, Daniel P. Berrange wrote:
> * include/libvirt/libvirt.h, include/libvirt/libvirt.h.in: Add
> virSecretGetUsageType, virSecretGetUsageID and virLookupSecretByUsage
> * python/generator.py: Mark virSecretGetUsageType, virSecretGetUsageID
> as not throwing exceptions
> * qemud/remote.c: Implement dispatch for virLookupSecretByUsage
> * qemud/remote_protocol.x: Add usage type & ID as attributes of
> remote_nonnull_secret. Add RPC calls for new public APIs
> * qemud/remote_dispatch_args.h, qemud/remote_dispatch_prototypes.h,
> qemud/remote_dispatch_ret.h, qemud/remote_dispatch_table.h,
> qemud/remote_protocol.c, qemud/remote_protocol.h: Re-generate
> * src/datatypes.c, src/datatypes.h: Add usageType and usageID as
> properties of virSecretPtr
> * src/driver.h: Add virLookupSecretByUsage driver entry point
> * src/libvirt.c: Implement virSecretGetUsageType, virSecretGetUsageID
> and virLookupSecretByUsage
> * src/libvirt_public.syms: Export virSecretGetUsageType, virSecretGetUsageID
> and virLookupSecretByUsage
> * src/remote_internal.c: Implement virLookupSecretByUsage entry
> * src/secret_conf.c, src/secret_conf.h: Remove the
> virSecretUsageType enum, now in public API. Make volume
> path mandatory when parsing XML
> * src/secret_driver.c: Enforce usage uniqueness when defining secrets.
> Implement virSecretLookupByUsage api method
> * src/virsh.c: Include usage for secret-list command
[...]
> /**
> + * virSecretLookupByUsage:
> + * @conn: pointer to the hypervisor connection
> + * @usageType: the type of secret usage
> + * @usageID: identifier of the object using the secret
> + *
> + * Try to lookup a secret on the given hypervisor based on its usage
> + *
> + * Returns a new secret object or NULL in case of failure. If the
> + * secret cannot be found, then VIR_ERR_NO_SECRET error is raised.
> + */
> +virSecretPtr
> +virSecretLookupByUsage(virConnectPtr conn,
> + int usageType,
> + const char *usageID)
> +{
> + DEBUG("conn=%p, usageType=%d usageID=%s", conn, usageType, NULLSTR(usageID));
> +
> + virResetLastError();
> +
> + if (!VIR_IS_CONNECT(conn)) {
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + return (NULL);
> + }
> + if (usageID == NULL) {
> + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> + if (conn->secretDriver &&
> + conn->secretDriver->lookupByUsage) {
> + virSecretPtr ret;
> + ret = conn->secretDriver->lookupByUsage (conn, usageType, usageID);
> + if (!ret)
> + goto error;
> + return ret;
> + }
> +
> + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> + /* Copy to connection error object for back compatability */
> + virSetConnError(conn);
> + return NULL;
> +}
> +
> +
> +/**
> * virSecretDefineXML:
> * @conn: virConnect connection
> * @xml: XML describing the secret.
> @@ -9036,6 +9083,53 @@ error:
> return -1;
> }
>
> +/**
> + * virSecretGetUsageType:
> + * @secret: a secret object
> + *
> + * Get the type of object which uses this secret
> + *
> + * Returns a positive integer identifying the type of object,
> + * or -1 upon error.
> + */
> +int
> +virSecretGetUsageType(virSecretPtr secret)
> +{
> + DEBUG("secret=%p", secret);
> +
> + virResetLastError();
> +
> + if (!VIR_IS_SECRET(secret)) {
> + virLibSecretError(NULL, VIR_ERR_INVALID_SECRET, __FUNCTION__);
> + return (-1);
> + }
> + return (secret->usageType);
> +}
> +
> +/**
> + * virSecretGetUsageID:
> + * @secret: a secret object
> + *
> + * Get the unique identifier of the object with which this
> + * secret is to be used
> + *
> + * Returns a string identifying the object using the secret,
> + * or NULL upon error
> + */
> +const char *
> +virSecretGetUsageID(virSecretPtr secret)
> +{
> + DEBUG("secret=%p", secret);
> +
> + virResetLastError();
> +
> + if (!VIR_IS_SECRET(secret)) {
> + virLibSecretError(NULL, VIR_ERR_INVALID_SECRET, __FUNCTION__);
> + return (NULL);
> + }
> + return (secret->usageID);
> +}
> +
Looking from the outside I find that hard to graps especially the last
one. virSecretGetUsageID return value supposed to be an UUID , the
comment let the user expect this but it seems to be actually free form.
I'm not against the patch but I think this needs some example or
improved comments to really set properly the mechanism and expectations
for the user. This may come after as a set of documentations, but if
the function description could be clarified a bit this would be nice.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list