9:26 a.m.
Since qemuDomainDefPostParse callback requires qemuCaps, we need to make
sure it gets the capabilities stored in the domain's private data if the
domain is running. Passing NULL may cause QEMU capabilities probing to
be triggered in case QEMU binary changed in the meantime. When this
happens while a running domain object is locked, QMP event delivered to
the domain before QEMU capabilities probing finishes will deadlock the
event loop.
Several general snapshot and checkpoint APIs were lazily passing NULL as
the parseOpaque pointer instead of letting their callers pass the right
data. This patch fixes all paths leading to virDomainDefParseNode.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/conf/checkpoint_conf.c | 9 ++++++---
src/conf/checkpoint_conf.h | 1 +
src/conf/snapshot_conf.c | 11 ++++++++---
src/conf/snapshot_conf.h | 2 ++
src/esx/esx_driver.c | 2 +-
src/qemu/qemu_driver.c | 18 +++++++++++++-----
src/test/test_driver.c | 5 +++--
src/vbox/vbox_common.c | 4 ++--
tests/qemudomaincheckpointxml2xmltest.c | 2 +-
tests/qemudomainsnapshotxml2xmltest.c | 2 +-
10 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c
index 5ce4cc4853..b744e2b363 100644
--- a/src/conf/checkpoint_conf.c
+++ b/src/conf/checkpoint_conf.c
@@ -128,6 +128,7 @@ static virDomainCheckpointDefPtr
virDomainCheckpointDefParse(xmlXPathContextPtr ctxt,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ void *parseOpaque,
unsigned int flags)
{
virDomainCheckpointDefPtr ret = NULL;
@@ -174,7 +175,7 @@ virDomainCheckpointDefParse(xmlXPathContextPtr ctxt,
return NULL;
}
def->parent.dom = virDomainDefParseNode(ctxt->node->doc,
domainNode,
- caps, xmlopt, NULL,
+ caps, xmlopt, parseOpaque,
domainflags);
if (!def->parent.dom)
return NULL;
@@ -207,6 +208,7 @@ virDomainCheckpointDefParseNode(xmlDocPtr xml,
xmlNodePtr root,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ void *parseOpaque,
unsigned int flags)
{
xmlXPathContextPtr ctxt = NULL;
@@ -234,7 +236,7 @@ virDomainCheckpointDefParseNode(xmlDocPtr xml,
}
ctxt->node = root;
- def = virDomainCheckpointDefParse(ctxt, caps, xmlopt, flags);
+ def = virDomainCheckpointDefParse(ctxt, caps, xmlopt, parseOpaque, flags);
cleanup:
xmlXPathFreeContext(ctxt);
return def;
@@ -244,6 +246,7 @@ virDomainCheckpointDefPtr
virDomainCheckpointDefParseString(const char *xmlStr,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ void *parseOpaque,
unsigned int flags)
{
virDomainCheckpointDefPtr ret = NULL;
@@ -253,7 +256,7 @@ virDomainCheckpointDefParseString(const char *xmlStr,
if ((xml = virXMLParse(NULL, xmlStr, _("(domain_checkpoint)")))) {
xmlKeepBlanksDefault(keepBlanksDefault);
ret = virDomainCheckpointDefParseNode(xml, xmlDocGetRootElement(xml),
- caps, xmlopt, flags);
+ caps, xmlopt, parseOpaque, flags);
xmlFreeDoc(xml);
}
xmlKeepBlanksDefault(keepBlanksDefault);
diff --git a/src/conf/checkpoint_conf.h b/src/conf/checkpoint_conf.h
index 03dd6b7bde..47ff69eb4d 100644
--- a/src/conf/checkpoint_conf.h
+++ b/src/conf/checkpoint_conf.h
@@ -75,6 +75,7 @@ virDomainCheckpointDefPtr
virDomainCheckpointDefParseString(const char *xmlStr,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ void *parseOpaque,
unsigned int flags);
virDomainCheckpointDefPtr
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 324901a560..7996589ad2 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -228,6 +228,7 @@ static virDomainSnapshotDefPtr
virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ void *parseOpaque,
bool *current,
unsigned int flags)
{
@@ -303,7 +304,8 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
goto cleanup;
}
def->parent.dom = virDomainDefParseNode(ctxt->node->doc,
domainNode,
- caps, xmlopt, NULL, domainflags);
+ caps, xmlopt, parseOpaque,
+ domainflags);
if (!def->parent.dom)
goto cleanup;
} else {
@@ -413,6 +415,7 @@ virDomainSnapshotDefParseNode(xmlDocPtr xml,
xmlNodePtr root,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ void *parseOpaque,
bool *current,
unsigned int flags)
{
@@ -443,7 +446,7 @@ virDomainSnapshotDefParseNode(xmlDocPtr xml,
}
ctxt->node = root;
- def = virDomainSnapshotDefParse(ctxt, caps, xmlopt, current, flags);
+ def = virDomainSnapshotDefParse(ctxt, caps, xmlopt, parseOpaque, current, flags);
cleanup:
xmlXPathFreeContext(ctxt);
return def;
@@ -453,6 +456,7 @@ virDomainSnapshotDefPtr
virDomainSnapshotDefParseString(const char *xmlStr,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ void *parseOpaque,
bool *current,
unsigned int flags)
{
@@ -463,7 +467,8 @@ virDomainSnapshotDefParseString(const char *xmlStr,
if ((xml = virXMLParse(NULL, xmlStr, _("(domain_snapshot)")))) {
xmlKeepBlanksDefault(keepBlanksDefault);
ret = virDomainSnapshotDefParseNode(xml, xmlDocGetRootElement(xml),
- caps, xmlopt, current, flags);
+ caps, xmlopt, parseOpaque,
+ current, flags);
xmlFreeDoc(xml);
}
xmlKeepBlanksDefault(keepBlanksDefault);
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index a0c87e7ade..216726fc16 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -106,12 +106,14 @@ unsigned int virDomainSnapshotFormatConvertXMLFlags(unsigned int
flags);
virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ void *parseOpaque,
bool *current,
unsigned int flags);
virDomainSnapshotDefPtr virDomainSnapshotDefParseNode(xmlDocPtr xml,
xmlNodePtr root,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ void *parseOpaque,
bool *current,
unsigned int flags);
virDomainSnapshotDefPtr virDomainSnapshotDefNew(void);
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index b98c72dc3f..d6d219c101 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -4117,7 +4117,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char
*xmlDesc,
return NULL;
def = virDomainSnapshotDefParseString(xmlDesc, priv->caps,
- priv->xmlopt, NULL, parse_flags);
+ priv->xmlopt, NULL, NULL, parse_flags);
if (!def)
return NULL;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4da8b0e623..db6b00852f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -464,8 +464,12 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
int ret = -1;
virCapsPtr caps = NULL;
int direrr;
+ qemuDomainObjPrivatePtr priv;
virObjectLock(vm);
+
+ priv = vm->privateData;
+
if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) <
0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to allocate memory for "
@@ -504,7 +508,8 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
}
def = virDomainSnapshotDefParseString(xmlStr, caps,
- qemu_driver->xmlopt, &cur,
+ qemu_driver->xmlopt,
+ priv->qemuCaps, &cur,
flags);
if (def == NULL) {
/* Nothing we can do here, skip this one */
@@ -579,8 +584,11 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm,
int ret = -1;
virCapsPtr caps = NULL;
int direrr;
+ qemuDomainObjPrivatePtr priv;
virObjectLock(vm);
+ priv = vm->privateData;
+
if (virAsprintf(&chkDir, "%s/%s", baseDir, vm->def->name) < 0)
{
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to allocate memory for "
@@ -620,6 +628,7 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm,
def = virDomainCheckpointDefParseString(xmlStr, caps,
qemu_driver->xmlopt,
+ priv->qemuCaps,
flags);
if (!def || virDomainCheckpointAlignDisks(def) < 0) {
/* Nothing we can do here, skip this one */
@@ -15804,6 +15813,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
goto cleanup;
}
+ priv = vm->privateData;
cfg = virQEMUDriverGetConfig(driver);
if (virDomainSnapshotCreateXMLEnsureACL(domain->conn, vm->def, flags) < 0)
@@ -15831,7 +15841,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE;
if (!(def = virDomainSnapshotDefParseString(xmlDesc, caps, driver->xmlopt,
- NULL, parse_flags)))
+ priv->qemuCaps, NULL, parse_flags)))
goto cleanup;
/* reject snapshot names containing slashes or starting with dot as
@@ -15908,8 +15918,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
qemuDomainObjSetAsyncJobMask(vm, QEMU_JOB_NONE);
- priv = vm->privateData;
-
if (redefine) {
if (virDomainSnapshotRedefinePrep(domain, vm, &def, &snap,
driver->xmlopt,
@@ -17182,7 +17190,7 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain,
}
if (!(def = virDomainCheckpointDefParseString(xmlDesc, caps, driver->xmlopt,
- parse_flags)))
+ priv->qemuCaps, parse_flags)))
goto cleanup;
/* Unlike snapshots, the RNG schema already ensured a sane filename. */
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 85cfcce79f..7e87772434 100755
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -902,6 +902,7 @@ testParseDomainSnapshots(testDriverPtr privconn,
def = virDomainSnapshotDefParseNode(ctxt->doc, node,
privconn->caps,
privconn->xmlopt,
+ NULL,
&cur,
VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL |
@@ -8168,7 +8169,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
if (!(def = virDomainSnapshotDefParseString(xmlDesc,
privconn->caps,
privconn->xmlopt,
- NULL,
+ NULL, NULL,
parse_flags)))
goto cleanup;
@@ -8629,7 +8630,7 @@ testDomainCheckpointCreateXML(virDomainPtr domain,
}
if (!(def = virDomainCheckpointDefParseString(xmlDesc, privconn->caps,
- privconn->xmlopt,
+ privconn->xmlopt, NULL,
parse_flags)))
goto cleanup;
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 6a4ef01e64..db3d940f85 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -5505,7 +5505,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE;
if (!(def = virDomainSnapshotDefParseString(xmlDesc, data->caps,
- data->xmlopt, NULL,
+ data->xmlopt, NULL, NULL,
parse_flags)))
goto cleanup;
@@ -6949,7 +6949,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
}
def = virDomainSnapshotDefParseString(defXml,
data->caps,
- data->xmlopt, NULL,
+ data->xmlopt, NULL, NULL,
VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE);
if (!def) {
diff --git a/tests/qemudomaincheckpointxml2xmltest.c
b/tests/qemudomaincheckpointxml2xmltest.c
index 8a7c0922c7..9dabe92ab9 100644
--- a/tests/qemudomaincheckpointxml2xmltest.c
+++ b/tests/qemudomaincheckpointxml2xmltest.c
@@ -54,7 +54,7 @@ testCompareXMLToXMLFiles(const char *inxml,
return -1;
if (!(def = virDomainCheckpointDefParseString(inXmlData, driver.caps,
- driver.xmlopt,
+ driver.xmlopt, NULL,
parseflags))) {
if (flags & TEST_INVALID)
return 0;
diff --git a/tests/qemudomainsnapshotxml2xmltest.c
b/tests/qemudomainsnapshotxml2xmltest.c
index 10ea9cf8d2..c84ee0bf7d 100644
--- a/tests/qemudomainsnapshotxml2xmltest.c
+++ b/tests/qemudomainsnapshotxml2xmltest.c
@@ -55,7 +55,7 @@ testCompareXMLToXMLFiles(const char *inxml,
goto cleanup;
if (!(def = virDomainSnapshotDefParseString(inXmlData, driver.caps,
- driver.xmlopt, &cur,
+ driver.xmlopt, NULL, &cur,
parseflags)))
goto cleanup;
if (cur) {
--
2.22.0