[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Fri, Sep 11, 2009 at 06:31:02PM +0100, Daniel P. Berrange wrote:
> 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.

  Okay, that clears things up ! ACK,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]