[libvirt] [PATCH 2/4] Add usage type/id as a public API property of virSecret

[Daniel Veillard]

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 at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/