qemu: Pass correct qemuCaps to virDomainDefParseNode

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@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c
index 5ce4cc485359379eae0c597a66ee05aac223e152..b744e2b36331b99a5f9198891b82b3bab7f83d34 100644 (file)
--- 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 03dd6b7bde250e890ceef3e71bdbbac102f72049..47ff69eb4d8e81cfeb5303a83af927f547999376 100644 (file)
--- 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 324901a56079bc8dba1d974b2912fa0a83877e8b..7996589ad2bda098490d727da06706df25332f57 100644 (file)
--- 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 a0c87e7ade2ea875cefa252da54a5c28b85c0c76..216726fc16a4989c4449b77f442fcd733a80413d 100644 (file)
--- 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 b98c72dc3f68cbd200561729ba245dbf59e430ce..d6d219c1013c8a426d3086e9b4df24e7174255e7 100644 (file)
--- 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 4da8b0e6236f812efe65e371658a0ea6378a0e5b..db6b00852fd46a3b63ad67b6096b8b005a7e21b6 100644 (file)
--- 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 043d63b12afbe320df688f1f2b2def4eb262ba79..2b5376ec283ba12e0ac25dc720eabda6b8cc7ba1 100755 (executable)
--- 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 |
@@ -8207,7 +8208,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
     if (!(def = virDomainSnapshotDefParseString(xmlDesc,
                                                 privconn->caps,
                                                 privconn->xmlopt,
-                                                NULL,
+                                                NULL, NULL,
                                                 parse_flags)))
         goto cleanup;
 
@@ -8668,7 +8669,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 6a4ef01e641085d70a265e76460d3bd4f193d4ee..db3d940f8532da2a858ab9f618a323201fb86fb3 100644 (file)
--- 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 8a7c0922c71d98487bc207ba582e5d19e9f7e7e1..9dabe92ab921c24d50c5a11a98fd23c53024b306 100644 (file)
--- 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 10ea9cf8d26dfa771658f626cb57c08e8dc02fb3..c84ee0bf7d1793f3261bb58c4e993bc9f530112c 100644 (file)
--- 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) {