[libvirt] [PATCH v2 1/6] conf: Introduce virStoragePoolXMLNamespace

[Daniel P. Berrangé]

On Wed, Jan 09, 2019 at 12:41:27PM -0500, John Ferlan wrote:
> 
> 
> On 1/9/19 12:06 PM, Daniel P. Berrangé wrote:
> > On Tue, Jan 08, 2019 at 12:52:21PM -0500, John Ferlan wrote:
> >> Introduce the infrastructure necessary to manage a Storage Pool XML
> >> Namespace. The general concept is similar to virDomainXMLNamespace,
> >> except that for Storage Pools the storage backend specific details
> >> can be stored within the _virStoragePoolOptions unlike the domain
> >> processing code which manages its xmlopt's via the virDomainXMLOption
> >> which is allocated/passed around for each domain.
> >>
> >> This patch defines the add the parse, format, free, and href methods
> >> required to process the XML and callout from the Storage Pool Source
> >> parse, format, and clear/free API's to perform the action on the
> >> XML data for/from the backend.
> >>
> >> Signed-off-by: John Ferlan <jferlan at redhat.com>
> >> ---
> >>  src/conf/storage_conf.c  | 51 +++++++++++++++++++++++++++++++++++++++-
> >>  src/conf/storage_conf.h  | 24 +++++++++++++++++++
> >>  src/libvirt_private.syms |  1 +
> >>  3 files changed, 75 insertions(+), 1 deletion(-)
> >>
> 
> [...]
> 
> >> +
> >> +int
> >> +virStoragePoolOptionsPoolTypeSetXMLNamespace(int type,
> >> +                                             virStoragePoolXMLNamespacePtr ns);
> >> +
> >>  /*
> >>   * How the volume's data is stored on underlying
> >>   * physical devices - can potentially span many
> >> @@ -197,6 +217,10 @@ struct _virStoragePoolSource {
> >>       * or lvm version, etc.
> >>       */
> >>      int format;
> >> +
> >> +    /* Pool backend specific XML namespace data */
> >> +    void *namespaceData;
> >> +    virStoragePoolXMLNamespace ns;
> >>  };
> > 
> > I don't like this placement.  For the domain and network XML
> > we put namespace data at the top level of the XML document.
> > This puts it one level further down, which makes it unusable
> > if we need it for parts of the storage pool which are not
> > related to the <source>.  I think consistency across all our
> > XML documents is important here.
> > 
> 
> Understood... Still does it make sense at the <pool> level to have
> something that only matters to the <source>? That's what I struggled
> with for placement. What if someone wanted something <target> specific
> or <pool> specific.
> 
> In the long run I don't care where it's placed - it's just moving code
> around. It just doesn't seem logical elsewhere though.
> 
> For the domain, it's "logically" part of the "qemu", "lxc", or "vmx"
> domains in some manner when building the command line or setting
> environment variables for a command to be run. I didn't look at the
> network example.
> 
> For storage, while it is backend/pool specific, it's even more specific
> to the source and some command to run or API to call to ensure the
> source is configured in some specific manner.

It will vary depending on the pool backend. With NFS there's only a
single mount command, so a single  <netfs:mountopts> is relevant.
If other backends run many commands I can imagine seeing multiple
distinct XML elements - one for each conceptual command that needs
extensions. It can still be at the top level IMHO.

Even for domain XML the BHyve driver has multiple commands, so if it
had custom XML namespace it would need to allow options for each of
these commands.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|