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>

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);

src/conf/checkpoint_conf.h

@@ -75,6 +75,7 @@ virDomainCheckpointDefPtr
 virDomainCheckpointDefParseString(const char *xmlStr,
                                   virCapsPtr caps,
                                   virDomainXMLOptionPtr xmlopt,
+                                  void *parseOpaque,
                                   unsigned int flags);
 
 virDomainCheckpointDefPtr

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);

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);

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;

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. */
 

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;
 

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) {

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;

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) {