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

[Daniel P. Berrangé]

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(-)
> 
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 55db7a96f5..5fae9b3a41 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -124,6 +124,9 @@ typedef virStoragePoolOptions *virStoragePoolOptionsPtr;
>  struct _virStoragePoolOptions {
>      unsigned int flags;
>      int defaultFormat;
> +
> +    virStoragePoolXMLNamespace ns;
> +
>      virStoragePoolFormatToString formatToString;
>      virStoragePoolFormatFromString formatFromString;
>  };
> @@ -318,6 +321,34 @@ virStoragePoolOptionsForPoolType(int type)
>  }
>  
>  
> +/* virStoragePoolOptionsPoolTypeSetXMLNamespace:
> + * @type: virStoragePoolType
> + * @ns: xmlopt namespace pointer
> + *
> + * Store the @ns in the pool options for the particular backend.
> + * This allows the parse/format code to then directly call the Namespace
> + * method space (parse, format, href, free) as needed during processing.
> + *
> + * Returns: 0 on success, -1 on failure.
> + */
> +int
> +virStoragePoolOptionsPoolTypeSetXMLNamespace(int type,
> +                                             virStoragePoolXMLNamespacePtr ns)
> +{
> +    int ret = -1;
> +    virStoragePoolTypeInfoPtr backend = virStoragePoolTypeInfoLookup(type);
> +
> +    if (!backend)
> +        goto cleanup;
> +
> +    backend->poolOptions.ns = *ns;
> +    ret = 0;
> +
> + cleanup:
> +    return ret;
> +}
> +
> +
>  static virStorageVolOptionsPtr
>  virStorageVolOptionsForPoolType(int type)
>  {
> @@ -378,6 +409,9 @@ virStoragePoolSourceClear(virStoragePoolSourcePtr source)
>      virStorageAuthDefFree(source->auth);
>      VIR_FREE(source->vendor);
>      VIR_FREE(source->product);
> +
> +    if (source->namespaceData && source->ns.free)
> +        (source->ns.free)(source->namespaceData);
>  }
>  
>  
> @@ -549,6 +583,13 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
>      source->vendor = virXPathString("string(./vendor/@name)", ctxt);
>      source->product = virXPathString("string(./product/@name)", ctxt);
>  
> +    /* Make a copy of all the callback pointers here for easier use,
> +     * especially during the virStoragePoolSourceClear method */
> +    source->ns = options->ns;
> +    if (source->ns.parse &&
> +        (source->ns.parse)(ctxt, &source->namespaceData) < 0)
> +        goto cleanup;
> +
>      ret = 0;
>   cleanup:
>      ctxt->node = relnode;
> @@ -965,6 +1006,11 @@ virStoragePoolSourceFormat(virBufferPtr buf,
>      virBufferEscapeString(buf, "<vendor name='%s'/>\n", src->vendor);
>      virBufferEscapeString(buf, "<product name='%s'/>\n", src->product);
>  
> +    if (src->namespaceData && src->ns.format) {
> +        if ((src->ns.format)(buf, src->namespaceData) < 0)
> +            return -1;
> +    }
> +
>      virBufferAdjustIndent(buf, -2);
>      virBufferAddLit(buf, "</source>\n");
>      return 0;
> @@ -989,7 +1035,10 @@ virStoragePoolDefFormatBuf(virBufferPtr buf,
>                         _("unexpected pool type"));
>          return -1;
>      }
> -    virBufferAsprintf(buf, "<pool type='%s'>\n", type);
> +    virBufferAsprintf(buf, "<pool type='%s'", type);
> +    if (def->source.namespaceData && def->source.ns.href)
> +        virBufferAsprintf(buf, " %s", (def->source.ns.href)());
> +    virBufferAddLit(buf, ">\n");
>      virBufferAdjustIndent(buf, 2);
>      virBufferEscapeString(buf, "<name>%s</name>\n", def->name);
>  
> diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
> index dc0aa2ab29..f388596f7c 100644
> --- a/src/conf/storage_conf.h
> +++ b/src/conf/storage_conf.h
> @@ -33,6 +33,26 @@
>  
>  # include <libxml/tree.h>
>  
> +/* Various callbacks needed to parse/create Storage Pool XML's using
> + * a private namespace */
> +typedef int (*virStoragePoolDefNamespaceParse)(xmlXPathContextPtr, void **);
> +typedef void (*virStoragePoolDefNamespaceFree)(void *);
> +typedef int (*virStoragePoolDefNamespaceXMLFormat)(virBufferPtr, void *);
> +typedef const char *(*virStoragePoolDefNamespaceHref)(void);
> +
> +typedef struct _virStoragePoolXMLNamespace virStoragePoolXMLNamespace;
> +typedef virStoragePoolXMLNamespace *virStoragePoolXMLNamespacePtr;
> +struct _virStoragePoolXMLNamespace {
> +    virStoragePoolDefNamespaceParse parse;
> +    virStoragePoolDefNamespaceFree free;
> +    virStoragePoolDefNamespaceXMLFormat format;
> +    virStoragePoolDefNamespaceHref href;
> +};
> +
> +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.

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