[libvirt] [PATCH 00/10] another set of parsing improvements

Pavel Hrdina (10): util: introduce virXMLPropStringLimit util: introduce virXMLNodeContentString conf: use virXMLPropString for virDomainVirtioOptionsParseXML conf: use virXMLPropString for IOMMU def parsing conf: use virXMLPropString for network parsing conf: use virXMLPropString for boot parsing conf: use virXMLPropString for actual network parsing conf: use virXMLPropStringLimit where it makes sense conf: use virXMLNodeContentString for boot options parsing conf: use virXMLPropString and virXMLNodeContentString for vcpu parsing src/conf/domain_conf.c | 601 +++++++++++++++++++++++++---------------------- src/libvirt_private.syms | 2 + src/util/virxml.c | 68 +++++- src/util/virxml.h | 4 + 4 files changed, 381 insertions(+), 294 deletions(-) -- 2.13.5

The virXMLPropStringLimit is an equivalent of virXPathStringLimit which should be preferred if you already have a XML dom node or if you need to parse more than one property. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 52 +++++++++++++++++++++++++++++++++++++++--------- src/util/virxml.h | 3 +++ 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6e4c3e83b9..7646998aef 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2939,6 +2939,7 @@ virXMLNodeToString; virXMLParseHelper; virXMLPickShellSafeComment; virXMLPropString; +virXMLPropStringLimit; virXMLSaveFile; virXMLValidateAgainstSchema; virXMLValidatorFree; diff --git a/src/util/virxml.c b/src/util/virxml.c index b42358a08c..2904fc16c9 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -92,6 +92,24 @@ virXPathString(const char *xpath, return ret; } + +static char * +virXMLStringLimitInternal(char *value, + size_t maxlen, + const char *name) +{ + if (value != NULL && strlen(value) >= maxlen) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("'%s' value longer than '%zu' bytes"), + name, maxlen); + VIR_FREE(value); + return NULL; + } + + return value; +} + + /** * virXPathStringLimit: * @xpath: the XPath string to evaluate @@ -111,15 +129,7 @@ virXPathStringLimit(const char *xpath, { char *tmp = virXPathString(xpath, ctxt); - if (tmp != NULL && strlen(tmp) >= maxlen) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("\'%s\' value longer than %zu bytes"), - xpath, maxlen); - VIR_FREE(tmp); - return NULL; - } - - return tmp; + return virXMLStringLimitInternal(tmp, maxlen, xpath); } /** @@ -506,6 +516,30 @@ virXMLPropString(xmlNodePtr node, return (char *)xmlGetProp(node, BAD_CAST name); } + +/** + * virXMLPropStringLimit: + * @node: XML dom node pointer + * @name: Name of the property (attribute) to get + * @maxlen: maximum length permitted string + * + * Wrapper for virXMLPropString, which validates the length of the returned + * string. + * + * Returns a new string which must be deallocated by the caller or NULL if + * the evaluation failed. + */ +char * +virXMLPropStringLimit(xmlNodePtr node, + const char *name, + size_t maxlen) +{ + char *tmp = (char *)xmlGetProp(node, BAD_CAST name); + + return virXMLStringLimitInternal(tmp, maxlen, name); +} + + /** * virXPathBoolean: * @xpath: the XPath string to evaluate diff --git a/src/util/virxml.h b/src/util/virxml.h index 2f953a6d44..1ecc6b0a61 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -73,6 +73,9 @@ int virXPathNodeSet(const char *xpath, xmlNodePtr **list); char * virXMLPropString(xmlNodePtr node, const char *name); +char * virXMLPropStringLimit(xmlNodePtr node, + const char *name, + size_t maxlen); long virXMLChildElementCount(xmlNodePtr node); /* Internal function; prefer the macros below. */ -- 2.13.5

On Wed, Aug 16, 2017 at 02:40:38PM +0200, Pavel Hrdina wrote:
The virXMLPropStringLimit is an equivalent of virXPathStringLimit which should be preferred if you already have a XML dom node or if you need to parse more than one property.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 52 +++++++++++++++++++++++++++++++++++++++--------- src/util/virxml.h | 3 +++ 3 files changed, 47 insertions(+), 9 deletions(-)
ACK with one nit below
@@ -506,6 +516,30 @@ virXMLPropString(xmlNodePtr node, return (char *)xmlGetProp(node, BAD_CAST name); }
+ +/** + * virXMLPropStringLimit: + * @node: XML dom node pointer + * @name: Name of the property (attribute) to get + * @maxlen: maximum length permitted string
How about: maximum permitted length of the string Jan
+ * + * Wrapper for virXMLPropString, which validates the length of the returned + * string. + * + * Returns a new string which must be deallocated by the caller or NULL if + * the evaluation failed. + */ +char * +virXMLPropStringLimit(xmlNodePtr node, + const char *name, + size_t maxlen) +{ + char *tmp = (char *)xmlGetProp(node, BAD_CAST name); + + return virXMLStringLimitInternal(tmp, maxlen, name); +} + + /** * virXPathBoolean: * @xpath: the XPath string to evaluate

It's equivalent of calling virXPahtString("string(.)", ctxt) but it doesn't have to use the XPath resolving and parsing. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 16 ++++++++++++++++ src/util/virxml.h | 1 + 3 files changed, 18 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7646998aef..f0e1e3eb6e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2933,6 +2933,7 @@ virVHBAPathExists; virXMLCheckIllegalChars; virXMLChildElementCount; virXMLExtractNamespaceXML; +virXMLNodeContentString; virXMLNodeNameEqual; virXMLNodeSanitizeNamespaces; virXMLNodeToString; diff --git a/src/util/virxml.c b/src/util/virxml.c index 2904fc16c9..cf9122f261 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -541,6 +541,22 @@ virXMLPropStringLimit(xmlNodePtr node, /** + * virXMLNodeContentString: + * @node: XML dom node pointer + * + * Convenience function to return copy of content of an XML node. + * + * Returns the content value as string or NULL in case of failure. + * The caller is responsible for freeing the returned buffer. + */ +char * +virXMLNodeContentString(xmlNodePtr node) +{ + return (char *)xmlNodeGetContent(node); +} + + +/** * virXPathBoolean: * @xpath: the XPath string to evaluate * @ctxt: an XPath context diff --git a/src/util/virxml.h b/src/util/virxml.h index 1ecc6b0a61..86baeb37a7 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -76,6 +76,7 @@ char * virXMLPropString(xmlNodePtr node, char * virXMLPropStringLimit(xmlNodePtr node, const char *name, size_t maxlen); +char * virXMLNodeContentString(xmlNodePtr node); long virXMLChildElementCount(xmlNodePtr node); /* Internal function; prefer the macros below. */ -- 2.13.5

On Wed, Aug 16, 2017 at 02:40:39PM +0200, Pavel Hrdina wrote:
It's equivalent of calling virXPahtString("string(.)", ctxt) but it
s/Paht/Path/
doesn't have to use the XPath resolving and parsing.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 16 ++++++++++++++++ src/util/virxml.h | 1 + 3 files changed, 18 insertions(+)
AKC Jan

XPath is good for random search of elements, not for accessing attributes of one node. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 49 ++++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 281dc68f0e..ec4fbf36b3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1114,7 +1114,7 @@ virDomainXMLOptionGetNamespace(virDomainXMLOptionPtr xmlopt) } static int -virDomainVirtioOptionsParseXML(xmlXPathContextPtr ctxt, +virDomainVirtioOptionsParseXML(xmlNodePtr driver, virDomainVirtioOptionsPtr *virtio) { char *str = NULL; @@ -1122,12 +1122,15 @@ virDomainVirtioOptionsParseXML(xmlXPathContextPtr ctxt, int val; virDomainVirtioOptionsPtr res; + if (*virtio || !driver) + return 0; + if (VIR_ALLOC(*virtio) < 0) return -1; res = *virtio; - if ((str = virXPathString("string(./driver/@iommu)", ctxt))) { + if ((str = virXMLPropString(driver, "iommu"))) { if ((val = virTristateSwitchTypeFromString(str)) <= 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid iommu value")); @@ -1137,7 +1140,7 @@ virDomainVirtioOptionsParseXML(xmlXPathContextPtr ctxt, } VIR_FREE(str); - if ((str = virXPathString("string(./driver/@ats)", ctxt))) { + if ((str = virXMLPropString(driver, "ats"))) { if ((val = virTristateSwitchTypeFromString(str)) <= 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid ats value")); @@ -8406,6 +8409,9 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } } else if (!def->src->driverName && virXMLNodeNameEqual(cur, "driver")) { + if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0) + goto error; + if (virDomainDiskDefDriverParseXML(def, cur) < 0) goto error; } else if (!def->mirror && @@ -8491,9 +8497,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } } - if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) - goto error; - /* Disk volume types will have authentication information handled in * virStorageTranslateDiskSourcePool */ @@ -9078,6 +9081,9 @@ virDomainControllerDefParseXML(xmlNodePtr node, max_sectors = virXMLPropString(cur, "max_sectors"); ioeventfd = virXMLPropString(cur, "ioeventfd"); iothread = virXMLPropString(cur, "iothread"); + + if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0) + goto error; } else if (virXMLNodeNameEqual(cur, "model")) { if (processedModel) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -9105,9 +9111,6 @@ virDomainControllerDefParseXML(xmlNodePtr node, cur = cur->next; } - if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) - goto error; - /* node is parsed differently from target attributes because * someone thought it should be a subelement instead... */ @@ -9483,6 +9486,9 @@ virDomainFSDefParseXML(xmlNodePtr node, wrpolicy = virXMLPropString(cur, "wrpolicy"); if (!format) format = virXMLPropString(cur, "format"); + + if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0) + goto error; } } cur = cur->next; @@ -9544,9 +9550,6 @@ virDomainFSDefParseXML(xmlNodePtr node, goto error; } - if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) - goto error; - def->src->path = source; source = NULL; def->dst = target; @@ -9986,6 +9989,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, queues = virXMLPropString(cur, "queues"); rx_queue_size = virXMLPropString(cur, "rx_queue_size"); tx_queue_size = virXMLPropString(cur, "tx_queue_size"); + + if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0) + goto error; } else if (virXMLNodeNameEqual(cur, "filterref")) { if (filter) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -10563,9 +10569,6 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } - if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) - goto error; - cleanup: ctxt->node = oldnode; VIR_FREE(macaddr); @@ -11689,7 +11692,8 @@ virDomainInputDefParseXML(const virDomainDef *dom, goto error; } - if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + if (virDomainVirtioOptionsParseXML(virXPathNode("./driver", ctxt), + &def->virtio) < 0) goto error; cleanup: @@ -13028,7 +13032,8 @@ virDomainRNGDefParseXML(virDomainXMLOptionPtr xmlopt, if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; - if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + if (virDomainVirtioOptionsParseXML(virXPathNode("./driver", ctxt), + &def->virtio) < 0) goto error; cleanup: @@ -13096,7 +13101,8 @@ virDomainMemballoonDefParseXML(xmlNodePtr node, else if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; - if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + if (virDomainVirtioOptionsParseXML(virXPathNode("./driver", ctxt), + &def->virtio) < 0) goto error; cleanup: @@ -13681,6 +13687,10 @@ virDomainVideoDefParseXML(xmlNodePtr node, def->accel = virDomainVideoAccelDefParseXML(cur); } + if (virXMLNodeNameEqual(cur, "driver")) { + if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0) + goto error; + } } cur = cur->next; } @@ -13759,9 +13769,6 @@ virDomainVideoDefParseXML(xmlNodePtr node, if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; - if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) - goto error; - def->driver = virDomainVideoDriverDefParseXML(node); cleanup: -- 2.13.5

On Wed, Aug 16, 2017 at 02:40:40PM +0200, Pavel Hrdina wrote:
XPath is good for random search of elements, not for accessing attributes of one node.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 49 ++++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 21 deletions(-)
ACK Jan

XPath is good for random search of elements, not for accessing attributes of one node. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- Notes: hint: review with -b src/conf/domain_conf.c | 57 ++++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ec4fbf36b3..4138a87f8b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14429,6 +14429,7 @@ virDomainIOMMUDefParseXML(xmlNodePtr node, { virDomainIOMMUDefPtr iommu = NULL, ret = NULL; xmlNodePtr save = ctxt->node; + xmlNodePtr driver; char *tmp = NULL; int val; @@ -14450,39 +14451,41 @@ virDomainIOMMUDefParseXML(xmlNodePtr node, iommu->model = val; - VIR_FREE(tmp); - if ((tmp = virXPathString("string(./driver/@intremap)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, _("unknown intremap value: %s"), tmp); - goto cleanup; + if ((driver = virXPathNode("./driver", ctxt))) { + VIR_FREE(tmp); + if ((tmp = virXMLPropString(driver, "intremap"))) { + if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_XML_ERROR, _("unknown intremap value: %s"), tmp); + goto cleanup; + } + iommu->intremap = val; } - iommu->intremap = val; - } - VIR_FREE(tmp); - if ((tmp = virXPathString("string(./driver/@caching_mode)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, _("unknown caching_mode value: %s"), tmp); - goto cleanup; + VIR_FREE(tmp); + if ((tmp = virXMLPropString(driver, "caching_mode"))) { + if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_XML_ERROR, _("unknown caching_mode value: %s"), tmp); + goto cleanup; + } + iommu->caching_mode = val; } - iommu->caching_mode = val; - } - VIR_FREE(tmp); - if ((tmp = virXPathString("string(./driver/@iotlb)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, _("unknown iotlb value: %s"), tmp); - goto cleanup; + VIR_FREE(tmp); + if ((tmp = virXMLPropString(driver, "iotlb"))) { + if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_XML_ERROR, _("unknown iotlb value: %s"), tmp); + goto cleanup; + } + iommu->iotlb = val; } - iommu->iotlb = val; - } - VIR_FREE(tmp); - if ((tmp = virXPathString("string(./driver/@eim)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, _("unknown eim value: %s"), tmp); - goto cleanup; + VIR_FREE(tmp); + if ((tmp = virXMLPropString(driver, "eim"))) { + if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_XML_ERROR, _("unknown eim value: %s"), tmp); + goto cleanup; + } + iommu->eim = val; } - iommu->eim = val; } ret = iommu; -- 2.13.5

On Wed, Aug 16, 2017 at 02:40:41PM +0200, Pavel Hrdina wrote:
XPath is good for random search of elements, not for accessing attributes of one node.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
Notes: hint: review with -b
src/conf/domain_conf.c | 57 ++++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 27 deletions(-)
The STRDUP in virXPathString is still redundant, but not sure if it's worth over-optimizning further. ACK Jan

XPath is good for random search of elements, not for accessing attributes of one node. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- Notes: hint: review with -b src/conf/domain_conf.c | 223 +++++++++++++++++++++++++------------------------ 1 file changed, 116 insertions(+), 107 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4138a87f8b..90f3f55f25 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9793,6 +9793,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, virDomainNetDefPtr def; virDomainHostdevDefPtr hostdev; xmlNodePtr cur; + xmlNodePtr tmpNode; char *macaddr = NULL; char *type = NULL; char *network = NULL; @@ -9952,8 +9953,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, if (!localaddr && def->type == VIR_DOMAIN_NET_TYPE_UDP) { xmlNodePtr tmpnode = ctxt->node; ctxt->node = cur; - localaddr = virXPathString("string(./local/@address)", ctxt); - localport = virXPathString("string(./local/@port)", ctxt); + if ((tmpNode = virXPathNode("./local", ctxt))) { + localaddr = virXMLPropString(tmpNode, "address"); + localport = virXMLPropString(tmpNode, "port"); + } ctxt->node = tmpnode; } } else if (!ifname && @@ -10399,124 +10402,130 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } def->driver.virtio.tx_queue_size = q; } - if ((str = virXPathString("string(./driver/host/@csum)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown host csum mode '%s'"), - str); - goto error; + + if ((tmpNode = virXPathNode("./driver/host", ctxt))) { + if ((str = virXMLPropString(tmpNode, "csum"))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown host csum mode '%s'"), + str); + goto error; + } + def->driver.virtio.host.csum = val; } - def->driver.virtio.host.csum = val; - } - VIR_FREE(str); - if ((str = virXPathString("string(./driver/host/@gso)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown host gso mode '%s'"), - str); - goto error; + VIR_FREE(str); + if ((str = virXMLPropString(tmpNode, "gso"))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown host gso mode '%s'"), + str); + goto error; + } + def->driver.virtio.host.gso = val; } - def->driver.virtio.host.gso = val; - } - VIR_FREE(str); - if ((str = virXPathString("string(./driver/host/@tso4)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown host tso4 mode '%s'"), - str); - goto error; + VIR_FREE(str); + if ((str = virXMLPropString(tmpNode, "tso4"))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown host tso4 mode '%s'"), + str); + goto error; + } + def->driver.virtio.host.tso4 = val; } - def->driver.virtio.host.tso4 = val; - } - VIR_FREE(str); - if ((str = virXPathString("string(./driver/host/@tso6)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown host tso6 mode '%s'"), - str); - goto error; + VIR_FREE(str); + if ((str = virXMLPropString(tmpNode, "tso6"))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown host tso6 mode '%s'"), + str); + goto error; + } + def->driver.virtio.host.tso6 = val; } - def->driver.virtio.host.tso6 = val; - } - VIR_FREE(str); - if ((str = virXPathString("string(./driver/host/@ecn)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown host ecn mode '%s'"), - str); - goto error; + VIR_FREE(str); + if ((str = virXMLPropString(tmpNode, "ecn"))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown host ecn mode '%s'"), + str); + goto error; + } + def->driver.virtio.host.ecn = val; } - def->driver.virtio.host.ecn = val; - } - VIR_FREE(str); - if ((str = virXPathString("string(./driver/host/@ufo)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown host ufo mode '%s'"), - str); - goto error; + VIR_FREE(str); + if ((str = virXMLPropString(tmpNode, "ufo"))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown host ufo mode '%s'"), + str); + goto error; + } + def->driver.virtio.host.ufo = val; } - def->driver.virtio.host.ufo = val; - } - VIR_FREE(str); - if ((str = virXPathString("string(./driver/host/@mrg_rxbuf)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown host mrg_rxbuf mode '%s'"), - str); - goto error; + VIR_FREE(str); + if ((str = virXMLPropString(tmpNode, "mrg_rxbuf"))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown host mrg_rxbuf mode '%s'"), + str); + goto error; + } + def->driver.virtio.host.mrg_rxbuf = val; } - def->driver.virtio.host.mrg_rxbuf = val; + VIR_FREE(str); } - VIR_FREE(str); - if ((str = virXPathString("string(./driver/guest/@csum)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown guest csum mode '%s'"), - str); - goto error; + + if ((tmpNode = virXPathNode("./driver/guest", ctxt))) { + if ((str = virXMLPropString(tmpNode, "csum"))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown guest csum mode '%s'"), + str); + goto error; + } + def->driver.virtio.guest.csum = val; } - def->driver.virtio.guest.csum = val; - } - VIR_FREE(str); - if ((str = virXPathString("string(./driver/guest/@tso4)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown guest tso4 mode '%s'"), - str); - goto error; + VIR_FREE(str); + if ((str = virXMLPropString(tmpNode, "tso4"))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown guest tso4 mode '%s'"), + str); + goto error; + } + def->driver.virtio.guest.tso4 = val; } - def->driver.virtio.guest.tso4 = val; - } - VIR_FREE(str); - if ((str = virXPathString("string(./driver/guest/@tso6)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown guest tso6 mode '%s'"), - str); - goto error; + VIR_FREE(str); + if ((str = virXMLPropString(tmpNode, "tso6"))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown guest tso6 mode '%s'"), + str); + goto error; + } + def->driver.virtio.guest.tso6 = val; } - def->driver.virtio.guest.tso6 = val; - } - VIR_FREE(str); - if ((str = virXPathString("string(./driver/guest/@ecn)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown guest ecn mode '%s'"), - str); - goto error; + VIR_FREE(str); + if ((str = virXMLPropString(tmpNode, "ecn"))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown guest ecn mode '%s'"), + str); + goto error; + } + def->driver.virtio.guest.ecn = val; } - def->driver.virtio.guest.ecn = val; - } - VIR_FREE(str); - if ((str = virXPathString("string(./driver/guest/@ufo)", ctxt))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown guest ufo mode '%s'"), - str); - goto error; + VIR_FREE(str); + if ((str = virXMLPropString(tmpNode, "ufo"))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown guest ufo mode '%s'"), + str); + goto error; + } + def->driver.virtio.guest.ufo = val; } - def->driver.virtio.guest.ufo = val; } def->backend.vhost = vhost_path; vhost_path = NULL; -- 2.13.5

On Wed, Aug 16, 2017 at 02:40:42PM +0200, Pavel Hrdina wrote:
XPath is good for random search of elements, not for accessing attributes of one node.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
Notes: hint: review with -b
src/conf/domain_conf.c | 223 +++++++++++++++++++++++++------------------------ 1 file changed, 116 insertions(+), 107 deletions(-)
ACK Jan

XPath is good for random search of elements, not for accessing attributes of one node. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- Notes: hint: review with -b src/conf/domain_conf.c | 95 ++++++++++++++++++++++++++------------------------ 1 file changed, 50 insertions(+), 45 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 90f3f55f25..917ea004e5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16039,6 +16039,7 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, virDomainDefPtr def) { xmlNodePtr *nodes = NULL; + xmlNodePtr node; size_t i; int n; char *tmp = NULL; @@ -16088,62 +16089,66 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK; } - tmp = virXPathString("string(./os/bootmenu[1]/@enable)", ctxt); - if (tmp) { - def->os.bootmenu = virTristateBoolTypeFromString(tmp); - if (def->os.bootmenu <= 0) { - /* In order not to break misconfigured machines, this - * should not emit an error, but rather set the bootmenu - * to disabled */ - VIR_WARN("disabling bootmenu due to unknown option '%s'", - tmp); - def->os.bootmenu = VIR_TRISTATE_BOOL_NO; - } - VIR_FREE(tmp); - } - - tmp = virXPathString("string(./os/bootmenu[1]/@timeout)", ctxt); - if (tmp && def->os.bootmenu == VIR_TRISTATE_BOOL_YES) { - if (virStrToLong_uip(tmp, NULL, 0, &def->os.bm_timeout) < 0 || - def->os.bm_timeout > 65535) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("invalid value for boot menu timeout, " - "must be in range [0,65535]")); - goto cleanup; + if ((node = virXPathNode("./os/bootmenu[1]", ctxt))) { + tmp = virXMLPropString(node, "enable"); + if (tmp) { + def->os.bootmenu = virTristateBoolTypeFromString(tmp); + if (def->os.bootmenu <= 0) { + /* In order not to break misconfigured machines, this + * should not emit an error, but rather set the bootmenu + * to disabled */ + VIR_WARN("disabling bootmenu due to unknown option '%s'", + tmp); + def->os.bootmenu = VIR_TRISTATE_BOOL_NO; + } + VIR_FREE(tmp); } - def->os.bm_timeout_set = true; - } - VIR_FREE(tmp); - tmp = virXPathString("string(./os/bios[1]/@useserial)", ctxt); - if (tmp) { - if (STREQ(tmp, "yes")) { - if (virXPathULong("count(./devices/serial)", - ctxt, &serialPorts) < 0) { + tmp = virXMLPropString(node, "timeout"); + if (tmp && def->os.bootmenu == VIR_TRISTATE_BOOL_YES) { + if (virStrToLong_uip(tmp, NULL, 0, &def->os.bm_timeout) < 0 || + def->os.bm_timeout > 65535) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("need at least one serial port " - "for useserial")); + _("invalid value for boot menu timeout, " + "must be in range [0,65535]")); goto cleanup; } - def->os.bios.useserial = VIR_TRISTATE_BOOL_YES; - } else { - def->os.bios.useserial = VIR_TRISTATE_BOOL_NO; + def->os.bm_timeout_set = true; } VIR_FREE(tmp); } - tmp = virXPathString("string(./os/bios[1]/@rebootTimeout)", ctxt); - if (tmp) { - /* that was really just for the check if it is there */ + if ((node = virXPathNode("./os/bios[1]", ctxt))) { + tmp = virXMLPropString(node, "useserial"); + if (tmp) { + if (STREQ(tmp, "yes")) { + if (virXPathULong("count(./devices/serial)", + ctxt, &serialPorts) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("need at least one serial port " + "for useserial")); + goto cleanup; + } + def->os.bios.useserial = VIR_TRISTATE_BOOL_YES; + } else { + def->os.bios.useserial = VIR_TRISTATE_BOOL_NO; + } + VIR_FREE(tmp); + } + + tmp = virXMLPropString(node, "rebootTimeout"); + if (tmp) { + /* that was really just for the check if it is there */ - if (virStrToLong_i(tmp, NULL, 0, &def->os.bios.rt_delay) < 0 || - def->os.bios.rt_delay < -1 || def->os.bios.rt_delay > 65535) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("invalid value for rebootTimeout, " - "must be in range [-1,65535]")); - goto cleanup; + if (virStrToLong_i(tmp, NULL, 0, &def->os.bios.rt_delay) < 0 || + def->os.bios.rt_delay < -1 || def->os.bios.rt_delay > 65535) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("invalid value for rebootTimeout, " + "must be in range [-1,65535]")); + goto cleanup; + } + def->os.bios.rt_set = true; } - def->os.bios.rt_set = true; } ret = 0; -- 2.13.5

On Wed, Aug 16, 2017 at 02:40:43PM +0200, Pavel Hrdina wrote:
XPath is good for random search of elements, not for accessing attributes of one node.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
Notes: hint: review with -b
src/conf/domain_conf.c | 95 ++++++++++++++++++++++++++------------------------ 1 file changed, 50 insertions(+), 45 deletions(-)
ACK Jan

XPath is good for random search of elements, not for accessing attributes of one node. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- Notes: hint: review with -b src/conf/domain_conf.c | 61 ++++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 917ea004e5..83432fa5b0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9656,18 +9656,22 @@ virDomainActualNetDefParseXML(xmlNodePtr node, } if (actual->type == VIR_DOMAIN_NET_TYPE_DIRECT) { - actual->data.direct.linkdev = virXPathString("string(./source[1]/@dev)", ctxt); + xmlNodePtr sourceNode = virXPathNode("./source[1]", ctxt); - mode = virXPathString("string(./source[1]/@mode)", ctxt); - if (mode) { - int m; - if ((m = virNetDevMacVLanModeTypeFromString(mode)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown mode '%s' in interface <actual> element"), - mode); - goto error; + if (sourceNode) { + actual->data.direct.linkdev = virXMLPropString(sourceNode, "dev"); + + mode = virXMLPropString(sourceNode, "mode"); + if (mode) { + int m; + if ((m = virNetDevMacVLanModeTypeFromString(mode)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown mode '%s' in interface <actual> element"), + mode); + goto error; + } + actual->data.direct.mode = m; } - actual->data.direct.mode = m; } } else if (actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) { virDomainHostdevDefPtr hostdev = &actual->data.hostdev.def; @@ -9703,24 +9707,27 @@ virDomainActualNetDefParseXML(xmlNodePtr node, } if (actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE || actual->type == VIR_DOMAIN_NET_TYPE_NETWORK) { - char *brname = virXPathString("string(./source/@bridge)", ctxt); + xmlNodePtr sourceNode = virXPathNode("./source", ctxt); + if (sourceNode) { + char *brname = virXMLPropString(sourceNode, "bridge"); - if (!brname && actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing <source> element with bridge name in " - "interface's <actual> element")); - goto error; - } - actual->data.bridge.brname = brname; - macTableManager = virXPathString("string(./source/@macTableManager)", ctxt); - if (macTableManager && - (actual->data.bridge.macTableManager - = virNetworkBridgeMACTableManagerTypeFromString(macTableManager)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid macTableManager setting '%s' " - "in domain interface's <actual> element"), - macTableManager); - goto error; + if (!brname && actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing <source> element with bridge name in " + "interface's <actual> element")); + goto error; + } + actual->data.bridge.brname = brname; + macTableManager = virXMLPropString(sourceNode, "macTableManager"); + if (macTableManager && + (actual->data.bridge.macTableManager + = virNetworkBridgeMACTableManagerTypeFromString(macTableManager)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid macTableManager setting '%s' " + "in domain interface's <actual> element"), + macTableManager); + goto error; + } } } -- 2.13.5

On Wed, Aug 16, 2017 at 02:40:44PM +0200, Pavel Hrdina wrote:
XPath is good for random search of elements, not for accessing attributes of one node.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
Notes: hint: review with -b
src/conf/domain_conf.c | 61 ++++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 27 deletions(-)
ACK Jan

The XPath call for these cases is more expensive than accessing the XML dom node directly. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 83432fa5b0..ea293b9f02 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7227,8 +7227,8 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt, char *p; virSecurityLabelDefPtr seclabel = NULL; - p = virXPathStringLimit("string(./@model)", - VIR_SECURITY_MODEL_BUFLEN - 1, ctxt); + p = virXMLPropStringLimit(ctxt->node, "model", + VIR_SECURITY_MODEL_BUFLEN - 1); if (!(seclabel = virSecurityLabelDefNew(p))) goto error; @@ -7237,8 +7237,8 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt, /* set default value */ seclabel->type = VIR_DOMAIN_SECLABEL_DYNAMIC; - p = virXPathStringLimit("string(./@type)", - VIR_SECURITY_LABEL_BUFLEN - 1, ctxt); + p = virXMLPropStringLimit(ctxt->node, "type", + VIR_SECURITY_LABEL_BUFLEN - 1); if (p) { seclabel->type = virDomainSeclabelTypeFromString(p); if (seclabel->type <= 0) { @@ -7253,8 +7253,8 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt, seclabel->relabel = false; VIR_FREE(p); - p = virXPathStringLimit("string(./@relabel)", - VIR_SECURITY_LABEL_BUFLEN-1, ctxt); + p = virXMLPropStringLimit(ctxt->node, "relabel", + VIR_SECURITY_LABEL_BUFLEN-1); if (p) { if (STREQ(p, "yes")) { seclabel->relabel = true; -- 2.13.5

On Wed, Aug 16, 2017 at 02:40:45PM +0200, Pavel Hrdina wrote:
The XPath call for these cases is more expensive than accessing the XML dom node directly.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
ACK Jan

Using XPath here doesn't add any benefit. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ea293b9f02..db042e5dc1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17062,7 +17062,6 @@ virDomainDefParseBootOptions(virDomainDefPtr def, virHashTablePtr *bootHash) { xmlNodePtr *nodes = NULL; - xmlNodePtr oldnode; char *tmp = NULL; char *name = NULL; int ret = -1; @@ -17169,8 +17168,6 @@ virDomainDefParseBootOptions(virDomainDefPtr def, } if (n == 1) { - oldnode = ctxt->node; - ctxt->node = nodes[0]; tmp = virXMLPropString(nodes[0], "type"); if (!tmp) { @@ -17181,7 +17178,7 @@ virDomainDefParseBootOptions(virDomainDefPtr def, if (STREQ_NULLABLE(tmp, "slic")) { VIR_FREE(tmp); - tmp = virXPathString("string(.)", ctxt); + tmp = virXMLNodeContentString(nodes[0]); def->os.slic_table = virFileSanitizePath(tmp); VIR_FREE(tmp); } else { @@ -17190,7 +17187,6 @@ virDomainDefParseBootOptions(virDomainDefPtr def, tmp); goto error; } - ctxt->node = oldnode; } if (virDomainDefParseBootXML(ctxt, def) < 0) -- 2.13.5

On Wed, Aug 16, 2017 at 02:40:46PM +0200, Pavel Hrdina wrote:
Using XPath here doesn't add any benefit.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
ACK, thank you for cleaning this up after me. Jan

XPath is good for random search of elements, not for accessing attributes of one node. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- Notes: hint: review with -b src/conf/domain_conf.c | 98 ++++++++++++++++++++++++++------------------------ 1 file changed, 51 insertions(+), 47 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index db042e5dc1..3db56ffb7a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16914,66 +16914,70 @@ virDomainVcpuParse(virDomainDefPtr def, { int n; xmlNodePtr *nodes = NULL; + xmlNodePtr vcpuNode; size_t i; char *tmp = NULL; unsigned int maxvcpus; unsigned int vcpus; int ret = -1; - if ((n = virXPathUInt("string(./vcpu[1])", ctxt, &maxvcpus)) < 0) { - if (n == -2) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("maximum vcpus count must be an integer")); - goto cleanup; + vcpus = maxvcpus = 1; + + if ((vcpuNode = virXPathNode("./vcpu[1]", ctxt))) { + if ((tmp = virXMLNodeContentString(vcpuNode))) { + if (virStrToLong_ui(tmp, NULL, 10, &maxvcpus) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("maximum vcpus count must be an integer")); + goto cleanup; + } + VIR_FREE(tmp); + } + + if ((tmp = virXMLPropString(vcpuNode, "current"))) { + if (virStrToLong_ui(tmp, NULL, 10, &vcpus) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("current vcpus count must be an integer")); + goto cleanup; + } + VIR_FREE(tmp); + } else { + vcpus = maxvcpus; + } + + tmp = virXMLPropString(vcpuNode, "placement"); + if (tmp) { + if ((def->placement_mode = + virDomainCpuPlacementModeTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported CPU placement mode '%s'"), + tmp); + goto cleanup; + } + VIR_FREE(tmp); + } else { + def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC; } - maxvcpus = 1; + if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { + tmp = virXMLPropString(vcpuNode, "cpuset"); + if (tmp) { + if (virBitmapParse(tmp, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) + goto cleanup; + + if (virBitmapIsAllClear(def->cpumask)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'cpuset': %s"), tmp); + goto cleanup; + } + + VIR_FREE(tmp); + } + } } if (virDomainDefSetVcpusMax(def, maxvcpus, xmlopt) < 0) goto cleanup; - if ((n = virXPathUInt("string(./vcpu[1]/@current)", ctxt, &vcpus)) < 0) { - if (n == -2) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("current vcpus count must be an integer")); - goto cleanup; - } - - vcpus = maxvcpus; - } - - - tmp = virXPathString("string(./vcpu[1]/@placement)", ctxt); - if (tmp) { - if ((def->placement_mode = - virDomainCpuPlacementModeTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unsupported CPU placement mode '%s'"), - tmp); - goto cleanup; - } - VIR_FREE(tmp); - } else { - def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC; - } - - if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { - tmp = virXPathString("string(./vcpu[1]/@cpuset)", ctxt); - if (tmp) { - if (virBitmapParse(tmp, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) - goto cleanup; - - if (virBitmapIsAllClear(def->cpumask)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Invalid value of 'cpuset': %s"), tmp); - goto cleanup; - } - - VIR_FREE(tmp); - } - } - if ((n = virXPathNodeSet("./vcpus/vcpu", ctxt, &nodes)) < 0) goto cleanup; -- 2.13.5

On Wed, Aug 16, 2017 at 02:40:47PM +0200, Pavel Hrdina wrote:
XPath is good for random search of elements, not for accessing attributes of one node.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
Notes: hint: review with -b
src/conf/domain_conf.c | 98 ++++++++++++++++++++++++++------------------------ 1 file changed, 51 insertions(+), 47 deletions(-)
ACK Jan
participants (2)
-
Ján Tomko
-
Pavel Hrdina