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@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 :|