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

----- "Daniel Veillard" <veillard(a)redhat.com&gt; 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.

> > - 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.

> > - 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.

> > 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.

> > +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.