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

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



On Wed, Aug 19, 2009 at 05:36:27AM -0400, Miloslav Trmac wrote:
> ----- "Daniel Veillard" <veillard redhat com> wrote:
> > On Sun, Aug 16, 2009 at 10:47:54PM +0200, Miloslav Trma─Ź wrote:
> > > 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 set attributes of the secret using XML, e.g.
> > >   <secret ephemeral='no' private='yes'>
> > >     <uuid>b8eecf55-798e-4db7-b2dd-025b0cf08a36</uuid>
> > >     <volume>/var/lib/libvirt/images/mail.img</volume>
> > >     <description>LUKS passphrase for our mail server</description>
> > >   </secret>
> > > If <uuid/> is not specified, it is chosen automatically.
> > 
> >   Should secret always be tied to volumes. The API is generic enough
> > that we should make sure we can use this later to get priviledged
> > access to other resources, though right now I don't have a good
> > example in mind.
> The secret is not "tied" to a volume - the volume is an optional attribute.  A libvirt user can ignore the volume field completely, and manage the secrets separately - but other clients, e.g. virt-manager, can use the volume attribute to locate a secret for a given volume without storing any data outside of libvirt.

  Okay, it may later be tied to something else but then we can expand
  the rng, fine !

> > > - s/secret_id/uuid/g
> > 
> >   Not sure I catch that, 
> An earlier patch was calling the ID of a secret object "secret_id" everywhere, leading to constructs like <secret secret_id=.../>  In an earlier patch review I was asked to change that.

  Ah, right !

> > > - use "unsigned char *" for secret value
> > 
> >   fine, assuming 0 terninated. though this may not work with everything
> > but since we need to import/export to XML, unless uuencoding the value
> > we won't be able to embed something which is not part of XML character
> > range (#x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] |
> > [#x10000-#x10FFFF]) unicode code points.
> No, the value is an arbitrary set of bytes that is never passed through XML.  Originally it was (void *value, size_t value_size), the change to (unsigned char *value, size_t value_size) was requested in an earlier review.

  yes Dan pointed I was wrong too :-)

> > 
> >   it's good to have a document from the start :-) but this lacks a bit
> >   of the intended use for the API, i.e. where this may be required to
> >   use it.
> Those parts are documented in later patches that add references to secrets to the <volume> and <domain> XML formats.

  okay

> > > +virSecretPtr            virSecretDefineXML      (virConnectPtr conn,
> > > +                                                 const char *xml);
> > 
> >  Let's add an "unsigned int flags" to virSecretDefineXML() especially
> > as we don't know yet the scope of future usages.
> >
> > > +char *                  virSecretGetXMLDesc     (virSecretPtr secret);
> > 
> >   And also add an "unsigned int flags" to virSecretGetXMLDesc
> > 
> Will do.

  thanks !

  looks good then,

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]