[libvirt] [PATCH 01/20] Secret manipulation step 1: Public API

[Daniel P. Berrange]

On Tue, Aug 04, 2009 at 10:28:16PM +0200, Miloslav Trma?? wrote:
> This patch adds a "secret" as a separately managed object, using a
> special-purpose API to transfer the secret values between nodes and
> libvirt users.
> 
> Rather than add explicit accessors for attributes of secrets, and
> hard-code the "secrets are related to storage volumes" association in
> the API, the API uses XML to manipulate the association as well as
> other attributes, similarly to other areas of libvirt.
> 
> The user can allocate an ID for the secret using virSecretAllocateID()
> (or use some other way to allocate secret IDs, e.g. reuse IDs used by
> the user in other databases), set attributes of the secret using XML, e.g.
>   <secret ephemeral='no' private='yes'>
>     <volume>/var/lib/libvirt/images/mail.img</volume>
>     <description>LUKS passphrase for our mail server</description>
>   </secret>
> The secret value can be either generated and stored by libvirt during
> volume creation, or supplied by the user using virSecretSetValue().
> 
> A simple API is provided for enumeration of all secrets.  Very large
> deployments will manage secret IDs automatically, so it is probably not
> necessary to provide a specialized lookup function (allowing the volume
> key -> secret ID lookup in less than O(number of secrets)).  These
> functions can eventually be added later.

> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index e6536c7..093c89e 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1448,6 +1448,30 @@ void virEventRegisterImpl(virEventAddHandleFunc addHandle,
>                            virEventAddTimeoutFunc addTimeout,
>                            virEventUpdateTimeoutFunc updateTimeout,
>                            virEventRemoveTimeoutFunc removeTimeout);
> +
> +/*
> + * Secret manipulation API
> + */
> +char *                  virSecretAllocateID     (virConnectPtr conn);
> +int                     virSecretSetXML         (virConnectPtr conn,
> +                                                 const char *secret_id,
> +                                                 const char *xml);
> +char *                  virSecretGetXML         (virConnectPtr conn,
> +                                                 const char *secret_id);
> +int                     virSecretSetValue       (virConnectPtr conn,
> +                                                 const char *secret_id,
> +                                                 const void *secret,
> +                                                 size_t secret_size);
> +void *                  virSecretGetValue       (virConnectPtr conn,
> +                                                 const char *secret_id,
> +                                                 size_t *secret_size);
> +int                     virSecretDelete         (virConnectPtr conn,
> +                                                 const char *secret_id);
> +int                     virSecretNumOfSecrets   (virConnectPtr conn);
> +int                     virSecretListSecrets    (virConnectPtr conn,
> +                                                 char **ids,
> +                                                 int maxids);
> +

This looks like a reasonable set of operations to start with, just
have a couple of minor tweaks to suggest.

 - Instead of using "const char *secret_id" as the entity to
   pass around the APIs, introduce a opaque struct type.
   eg in libvirt.h.in

      typedef struct _virSecret virSecret;
      typedef virSecret *virSecretPtr;

    and internally in datatypes.h, have that struct be

    struct _virSecret {
       unsigned int magic;                  /* specific value to check */
       int refs;                            /* reference count */
       virConnectPtr conn;                  /* pointer back to the connection */
       unsigned char uuid[VIR_UUID_BUFLEN]; /* the unique secret identifier */
    };

    this just gives us a little more flexibility in the API for dealing with
    the unexpected in the future, and follows the paradigm of the other APIs.

 -  Rename virSecretSetXML() to virSecretDefineXML() and
    rename virSecretDelete() to virSecretUndefine()

    Again this is simply for consistency with API naming scheme used by
    other objects in libvirt.

    virSecretAllocateID() would then be unneccessary, since virSecretDefineXML
    would simply return an instance of the virSecretPtr object. We would 
    probably want a virSecretGetUUID() method to retrieve the id for an
    object, and a virSecretLookupByID() method to fetch an existing secret 
    based off its identifier - that would allow O(1) lookup from the ID
    in a volume XML doc as you suggested.


In the SetValue/GetValue fields you have a 

       const void *secret
       size_t secret_size

This implies an arbitrary data type being passed in or out. I'm guessing
that you're really meaning a opaque byte array, in which case I think it
would be preferrable to use  'unsigned char *' for the data instead of
'void *'

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|