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

On Fri, Sep 11, 2009 at 06:17:40PM +0200, Daniel Veillard wrote: > On Fri, Sep 11, 2009 at 03:19:18PM +0100, Daniel P. Berrange wrote: > > > > +/** > > + * 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. I'll expand the API docs for this method. It is not returning a UUID. I named it 'ID' to imply a arbitrary type of unique identifier since the format of the identifier returned by 'virSecretGetUsageID' will vary depending on what 'virSecretGetUsageType' shows. For a usage type of VIR_SECRET_USAGE_TYPE_VOLUME, the identifier will be the full volume file path, eg /var/lib/libvirt/images/encrypted1.img As and when we add new usage types (eg perhaps for VNC passwords, or SPICE credentials), then we'll define appropriate formats for their identifiers. As an example, with virsh, using the virSecretGetUsageID method, it will now show $ virsh secret-list UUID Usage ----------------------------------------------------------- 0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f Volume /home/berrange/VirtualMachines/encrypted2.img 80cfa88e-baa3-3e6f-a2aa-12d0576bc7c5 Volume /foo/bar e3a9758f-b0c6-7a3a-ebb9-71a69c930289 Volume /home/berrange/VirtualMachines/encrypted1.img e3b61a0f-714a-0b62-78c2-81eeb1d7d7a5 Unused The path bit of the usage there comes directly from the virSecretGetUsageType method, and also happens to match te <usage><volume>..</volume></usage> XML element contents.