From 8f8cc927a643f9bd02ff37f0fe6167e44359fb12 Mon Sep 17 00:00:00 2001 Message-Id: <8f8cc927a643f9bd02ff37f0fe6167e44359fb12@dist-git> From: Jiri Denemark Date: Fri, 16 Aug 2019 14:52:35 +0200 Subject: [PATCH] qemu: Pass correct qemuCaps to virDomainDefParseNode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Michal Privoznik (cherry picked from commit 577a1f98fc84e4152c246695942502ef9a45c7f7) Conflicts: src/conf/checkpoint_conf.c src/conf/checkpoint_conf.h src/conf/snapshot_conf.c src/conf/snapshot_conf.h src/esx/esx_driver.c src/qemu/qemu_driver.c src/test/test_driver.c src/vbox/vbox_common.c tests/qemudomaincheckpointxml2xmltest.c - no checkpoint APIs - snapshot parsing APIs do not have bool *current parameter tests/qemudomainsnapshotxml2xmltest.c - this is called domainsnapshotxml2xmltest.c downstream - snapshot parsing APIs do not have bool *current parameter https://bugzilla.redhat.com/show_bug.cgi?id=1731783 Signed-off-by: Jiri Denemark Message-Id: Reviewed-by: Ján Tomko --- src/conf/snapshot_conf.c | 9 ++++++--- src/conf/snapshot_conf.h | 2 ++ src/esx/esx_driver.c | 2 +- src/qemu/qemu_driver.c | 10 +++++++--- src/test/test_driver.c | 3 ++- src/vbox/vbox_common.c | 4 ++-- tests/domainsnapshotxml2xmltest.c | 2 +- 7 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 9c537ac7d1..75df496201 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -203,6 +203,7 @@ static virDomainSnapshotDefPtr virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, + void *parseOpaque, unsigned int flags) { virDomainSnapshotDefPtr def = NULL; @@ -284,7 +285,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, goto cleanup; } def->dom = virDomainDefParseNode(ctxt->node->doc, domainNode, - caps, xmlopt, NULL, domainflags); + caps, xmlopt, parseOpaque, domainflags); if (!def->dom) goto cleanup; } else { @@ -389,6 +390,7 @@ virDomainSnapshotDefParseNode(xmlDocPtr xml, xmlNodePtr root, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, + void *parseOpaque, unsigned int flags) { xmlXPathContextPtr ctxt = NULL; @@ -406,7 +408,7 @@ virDomainSnapshotDefParseNode(xmlDocPtr xml, } ctxt->node = root; - def = virDomainSnapshotDefParse(ctxt, caps, xmlopt, flags); + def = virDomainSnapshotDefParse(ctxt, caps, xmlopt, parseOpaque, flags); cleanup: xmlXPathFreeContext(ctxt); return def; @@ -416,6 +418,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, + void *parseOpaque, unsigned int flags) { virDomainSnapshotDefPtr ret = NULL; @@ -425,7 +428,7 @@ virDomainSnapshotDefParseString(const char *xmlStr, if ((xml = virXMLParse(NULL, xmlStr, _("(domain_snapshot)")))) { xmlKeepBlanksDefault(keepBlanksDefault); ret = virDomainSnapshotDefParseNode(xml, xmlDocGetRootElement(xml), - caps, xmlopt, flags); + caps, xmlopt, parseOpaque, flags); xmlFreeDoc(xml); } xmlKeepBlanksDefault(keepBlanksDefault); diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index 20a42bd572..733f1dd071 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -106,11 +106,13 @@ typedef enum { virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, + void *parseOpaque, unsigned int flags); virDomainSnapshotDefPtr virDomainSnapshotDefParseNode(xmlDocPtr xml, xmlNodePtr root, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, + void *parseOpaque, unsigned int flags); void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def); char *virDomainSnapshotDefFormat(const char *domain_uuid, diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 60aa5fc252..4ef856d71c 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4156,7 +4156,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, return NULL; def = virDomainSnapshotDefParseString(xmlDesc, priv->caps, - priv->xmlopt, 0); + priv->xmlopt, NULL, 0); if (!def) return NULL; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cc7210c6f4..00213a5f80 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -431,8 +431,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 " @@ -472,6 +476,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, def = virDomainSnapshotDefParseString(xmlStr, caps, qemu_driver->xmlopt, + priv->qemuCaps, flags); if (def == NULL) { /* Nothing we can do here, skip this one */ @@ -15312,6 +15317,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, if (!(vm = qemuDomObjFromDomain(domain))) goto cleanup; + priv = vm->privateData; cfg = virQEMUDriverGetConfig(driver); if (virDomainSnapshotCreateXMLEnsureACL(domain->conn, vm->def, flags) < 0) @@ -15336,7 +15342,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE; if (!(def = virDomainSnapshotDefParseString(xmlDesc, caps, driver->xmlopt, - parse_flags))) + priv->qemuCaps, parse_flags))) goto cleanup; /* reject snapshot names containing slashes or starting with dot as @@ -15405,8 +15411,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, qemuDomainObjSetAsyncJobMask(vm, QEMU_JOB_NONE); - priv = vm->privateData; - if (redefine) { if (virDomainSnapshotRedefinePrep(domain, vm, &def, &snap, driver->xmlopt, diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 1a25b37729..1c7e4f4982 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -849,6 +849,7 @@ testParseDomainSnapshots(testDriverPtr privconn, def = virDomainSnapshotDefParseNode(ctxt->doc, node, privconn->caps, privconn->xmlopt, + NULL, VIR_DOMAIN_SNAPSHOT_PARSE_DISKS | VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL | VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE); @@ -6403,6 +6404,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, if (!(def = virDomainSnapshotDefParseString(xmlDesc, privconn->caps, privconn->xmlopt, + NULL, parse_flags))) goto cleanup; @@ -6808,7 +6810,6 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } - static virHypervisorDriver testHypervisorDriver = { .name = "Test", .connectOpen = testConnectOpen, /* 0.1.1 */ diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 664650f217..46d2b7afa3 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -5496,7 +5496,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT, NULL); if (!(def = virDomainSnapshotDefParseString(xmlDesc, data->caps, - data->xmlopt, + data->xmlopt, NULL, VIR_DOMAIN_SNAPSHOT_PARSE_DISKS | VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE))) goto cleanup; @@ -6941,7 +6941,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) } def = virDomainSnapshotDefParseString(defXml, data->caps, - data->xmlopt, + data->xmlopt, NULL, VIR_DOMAIN_SNAPSHOT_PARSE_DISKS | VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE); if (!def) { diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c index 2b594e626c..a4e31de811 100644 --- a/tests/domainsnapshotxml2xmltest.c +++ b/tests/domainsnapshotxml2xmltest.c @@ -96,7 +96,7 @@ testCompareXMLToXMLFiles(const char *inxml, goto cleanup; if (!(def = virDomainSnapshotDefParseString(inXmlData, driver.caps, - driver.xmlopt, + driver.xmlopt, NULL, flags))) goto cleanup; -- 2.22.1