[libvirt] [PATCH v2 20/29] conf: Allow convenient lookup of <source> in virDomainStorageSourceParse

[Ján Tomko]

On Fri, Mar 22, 2019 at 07:00:56PM +0100, Peter Krempa wrote:
>If NULL is passed, the function will lookup <source> in current context.
>
>Signed-off-by: Peter Krempa <pkrempa at redhat.com>
>---
> src/conf/domain_conf.c | 12 +++++++++++-
> src/conf/domain_conf.h |  2 +-
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 5773d07474..852489e185 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -9080,7 +9080,7 @@ virDomainStorageSourceParseBase(const char *type,
>
> /**
>  * virDomainStorageSourceParse:
>- * @node: XML node pointing to the source element to parse
>+ * @node: XML node pointing to the source element to parse (see below)
>  * @ctxt: XPath context
>  * @src: filled with parsed data
>  * @flags: XML parser flags
>@@ -9089,6 +9089,9 @@ virDomainStorageSourceParseBase(const char *type,
>  * Parses @src definition from element pointed to by @node. Note that this
>  * does not parse the 'type' and 'format' attributes of @src and 'type' needs
>  * to be set correctly prior to calling this function.
>+ *
>+ * If @node is NULL a <source> subelement is looked up in @ctxt to be used as
>+ * source. Error is reported if the source is not found.
>  */
> int
> virDomainStorageSourceParse(xmlNodePtr node,
>@@ -9100,6 +9103,13 @@ virDomainStorageSourceParse(xmlNodePtr node,
>     VIR_XPATH_NODE_AUTORESTORE(ctxt);
>     xmlNodePtr tmp;
>
>+    if (!node &&
>+        !(node = virXPathNode("./source", ctxt))) {
>+        virReportError(VIR_ERR_XML_ERROR, "%s",
>+                       _("missing <source> element for storage source"));
>+        return -1;
>+    }
>+

This feels odd to me and there is only one caller using this at the end
of the series. Most of the callers already have a node handy and there's
the migrationSource vs. source where the 'source' node is preferred and
optional.

I'd rather open-code the lookup in DiskDefMirrorParse

Jano

>     ctxt->node = node;
>
>     switch ((virStorageType)src->type) {
>diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>index ce6e5b4748..6fb73bdaf7 100644
>--- a/src/conf/domain_conf.h
>+++ b/src/conf/domain_conf.h
>@@ -3463,7 +3463,7 @@ int virDomainStorageSourceParse(xmlNodePtr node,
>                                 virStorageSourcePtr src,
>                                 unsigned int flags,
>                                 virDomainXMLOptionPtr xmlopt)
>-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>+    ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>
> int virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
>                                      int maplen,
>-- 
>2.20.1
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190402/ea844a84/attachment-0001.sig>