[libvirt] [PATCH 0/2] Improve qemuDomainObjPrivateXMLParseJob

This is a follow-up to the Peter's work pushed yesterday. Jiri Denemark (2): qemu: Parse all job related private XML in dedicated function qemu: Switch context for job related private XML parsing src/qemu/qemu_domain.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) -- 2.16.2

Commit dc567cc22b introduced qemuDomainObjPrivateXMLParseJob, but forgot to move "./job[1]/@type" parsing into it. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2d108bec1b..a08a1825e9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2337,6 +2337,18 @@ qemuDomainObjPrivateXMLParseJob(virDomainObjPtr vm, int n; int ret = -1; + if ((tmp = virXPathString("string(./job[1]/@type)", ctxt))) { + int type; + + if ((type = qemuDomainJobTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown job type %s"), tmp); + goto cleanup; + } + VIR_FREE(tmp); + priv->job.active = type; + } + if ((tmp = virXPathString("string(./job[1]/@async)", ctxt))) { int async; @@ -2502,19 +2514,6 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, priv->lockState = virXPathString("string(./lockstate)", ctxt); - if ((tmp = virXPathString("string(./job[1]/@type)", ctxt))) { - int type; - - if ((type = qemuDomainJobTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown job type %s"), tmp); - VIR_FREE(tmp); - goto error; - } - VIR_FREE(tmp); - priv->job.active = type; - } - if (qemuDomainObjPrivateXMLParseJob(vm, priv, ctxt) < 0) goto error; -- 2.16.2

On Wed, Mar 14, 2018 at 12:25:13 +0100, Jiri Denemark wrote:
Commit dc567cc22b introduced qemuDomainObjPrivateXMLParseJob, but forgot to move "./job[1]/@type" parsing into it.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)
ACK

Having to repeat "./job[1]/" XPath prefix for every single element or attribute we want to parse is suboptimal. And even more so once we further extract code from qemuDomainObjPrivateXMLParseJob into separate functions. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a08a1825e9..570ad4ea8f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2332,12 +2332,18 @@ qemuDomainObjPrivateXMLParseJob(virDomainObjPtr vm, xmlXPathContextPtr ctxt) { xmlNodePtr *nodes = NULL; + xmlNodePtr node = ctxt->node; char *tmp = NULL; size_t i; int n; int ret = -1; - if ((tmp = virXPathString("string(./job[1]/@type)", ctxt))) { + if (!(ctxt->node = virXPathNode("./job[1]", ctxt))) { + ret = 0; + goto cleanup; + } + + if ((tmp = virXPathString("string(@type)", ctxt))) { int type; if ((type = qemuDomainJobTypeFromString(tmp)) < 0) { @@ -2349,7 +2355,7 @@ qemuDomainObjPrivateXMLParseJob(virDomainObjPtr vm, priv->job.active = type; } - if ((tmp = virXPathString("string(./job[1]/@async)", ctxt))) { + if ((tmp = virXPathString("string(@async)", ctxt))) { int async; if ((async = qemuDomainAsyncJobTypeFromString(tmp)) < 0) { @@ -2360,7 +2366,7 @@ qemuDomainObjPrivateXMLParseJob(virDomainObjPtr vm, VIR_FREE(tmp); priv->job.asyncJob = async; - if ((tmp = virXPathString("string(./job[1]/@phase)", ctxt))) { + if ((tmp = virXPathString("string(@phase)", ctxt))) { priv->job.phase = qemuDomainAsyncJobPhaseFromString(async, tmp); if (priv->job.phase < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2371,7 +2377,7 @@ qemuDomainObjPrivateXMLParseJob(virDomainObjPtr vm, } } - if ((n = virXPathNodeSet("./job[1]/disk[@migrating='yes']", ctxt, &nodes)) < 0) + if ((n = virXPathNodeSet("./disk[@migrating='yes']", ctxt, &nodes)) < 0) goto cleanup; if (n > 0) { @@ -2394,6 +2400,7 @@ qemuDomainObjPrivateXMLParseJob(virDomainObjPtr vm, ret = 0; cleanup: + ctxt->node = node; VIR_FREE(tmp); VIR_FREE(nodes); return ret; -- 2.16.2

On Wed, Mar 14, 2018 at 12:25:14 +0100, Jiri Denemark wrote:
Having to repeat "./job[1]/" XPath prefix for every single element or attribute we want to parse is suboptimal. And even more so once we further extract code from qemuDomainObjPrivateXMLParseJob into separate functions.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a08a1825e9..570ad4ea8f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2332,12 +2332,18 @@ qemuDomainObjPrivateXMLParseJob(virDomainObjPtr vm, xmlXPathContextPtr ctxt) { xmlNodePtr *nodes = NULL; + xmlNodePtr node = ctxt->node;
Rename this to saveNode. 'node' is usually used for a working pointer in our parser code. ACK
participants (2)
-
Jiri Denemark
-
Peter Krempa