7:40 a.m.
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
7:40 a.m.
New subject: [libvirt] [PATCH 01/10] util: introduce virXMLPropStringLimit
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(a)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
9:55 a.m.
New subject: [libvirt] [PATCH 01/10] util: introduce virXMLPropStringLimit
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(a)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
7:40 a.m.
New subject: [libvirt] [PATCH 02/10] util: introduce virXMLNodeContentString
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(a)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
9:56 a.m.
New subject: [libvirt] [PATCH 02/10] util: introduce virXMLNodeContentString
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(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virxml.c | 16 ++++++++++++++++
src/util/virxml.h | 1 +
3 files changed, 18 insertions(+)
AKC
Jan
7:40 a.m.
New subject: [libvirt] [PATCH 03/10] conf: use virXMLPropString for virDomainVirtioOptionsParseXML
XPath is good for random search of elements, not for accessing
attributes of one node.
Signed-off-by: Pavel Hrdina <phrdina(a)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
9:59 a.m.
New subject: [libvirt] [PATCH 03/10] conf: use virXMLPropString for virDomainVirtioOptionsParseXML
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(a)redhat.com>
---
src/conf/domain_conf.c | 49 ++++++++++++++++++++++++++++---------------------
1 file changed, 28 insertions(+), 21 deletions(-)
ACK
Jan
7:40 a.m.
New subject: [libvirt] [PATCH 04/10] conf: use virXMLPropString for IOMMU def parsing
XPath is good for random search of elements, not for accessing
attributes of one node.
Signed-off-by: Pavel Hrdina <phrdina(a)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
10:02 a.m.
New subject: [libvirt] [PATCH 04/10] conf: use virXMLPropString for IOMMU def parsing
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(a)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
7:40 a.m.
New subject: [libvirt] [PATCH 05/10] conf: use virXMLPropString for network parsing
XPath is good for random search of elements, not for accessing
attributes of one node.
Signed-off-by: Pavel Hrdina <phrdina(a)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
6:29 a.m.
New subject: [libvirt] [PATCH 05/10] conf: use virXMLPropString for network parsing
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(a)redhat.com>
---
Notes:
hint: review with -b
src/conf/domain_conf.c | 223 +++++++++++++++++++++++++------------------------
1 file changed, 116 insertions(+), 107 deletions(-)
ACK
Jan
7:40 a.m.
New subject: [libvirt] [PATCH 06/10] conf: use virXMLPropString for boot parsing
XPath is good for random search of elements, not for accessing
attributes of one node.
Signed-off-by: Pavel Hrdina <phrdina(a)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
6:30 a.m.
New subject: [libvirt] [PATCH 06/10] conf: use virXMLPropString for boot parsing
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(a)redhat.com>
---
Notes:
hint: review with -b
src/conf/domain_conf.c | 95 ++++++++++++++++++++++++++------------------------
1 file changed, 50 insertions(+), 45 deletions(-)
ACK
Jan
7:40 a.m.
New subject: [libvirt] [PATCH 07/10] conf: use virXMLPropString for actual network parsing
XPath is good for random search of elements, not for accessing
attributes of one node.
Signed-off-by: Pavel Hrdina <phrdina(a)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
6:30 a.m.
New subject: [libvirt] [PATCH 07/10] conf: use virXMLPropString for actual network parsing
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(a)redhat.com>
---
Notes:
hint: review with -b
src/conf/domain_conf.c | 61 ++++++++++++++++++++++++++++----------------------
1 file changed, 34 insertions(+), 27 deletions(-)
ACK
Jan
7:40 a.m.
New subject: [libvirt] [PATCH 08/10] conf: use virXMLPropStringLimit where it makes sense
The XPath call for these cases is more expensive than accessing the
XML dom node directly.
Signed-off-by: Pavel Hrdina <phrdina(a)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
6:31 a.m.
New subject: [libvirt] [PATCH 08/10] conf: use virXMLPropStringLimit where it makes sense
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(a)redhat.com>
---
src/conf/domain_conf.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
ACK
Jan
7:40 a.m.
New subject: [libvirt] [PATCH 09/10] conf: use virXMLNodeContentString for boot options parsing
Using XPath here doesn't add any benefit.
Signed-off-by: Pavel Hrdina <phrdina(a)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
6:33 a.m.
New subject: [libvirt] [PATCH 09/10] conf: use virXMLNodeContentString for boot options parsing
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(a)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
7:40 a.m.
New subject: [libvirt] [PATCH 10/10] conf: use virXMLPropString and virXMLNodeContentString for vcpu parsing
XPath is good for random search of elements, not for accessing
attributes of one node.
Signed-off-by: Pavel Hrdina <phrdina(a)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
6:36 a.m.
New subject: [libvirt] [PATCH 10/10] conf: use virXMLPropString and virXMLNodeContentString for vcpu parsing
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(a)redhat.com>
---
Notes:
hint: review with -b
src/conf/domain_conf.c | 98 ++++++++++++++++++++++++++------------------------
1 file changed, 51 insertions(+), 47 deletions(-)
ACK
Jan
2654
days inactive
2655
days old
20 comments
2 participants
participants (2)
-
Ján Tomko -
Pavel Hrdina