qemu: Pass qemuCaps to qemuDomainDefFormatBufInternal

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.

This patch fixes all paths leading to qemuDomainDefFormatBufInternal.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

src/qemu/qemu_domain.c

@@ -8388,7 +8388,7 @@ qemuDomainDefCopy(virQEMUDriverPtr driver,
     virDomainDefPtr ret = NULL;
     char *xml;
 
-    if (!(xml = qemuDomainDefFormatXML(driver, src, flags)))
+    if (!(xml = qemuDomainDefFormatXML(driver, qemuCaps, src, flags)))
         return NULL;
 
     ret = qemuDomainDefFromXML(driver, qemuCaps, xml);
@@ -8400,6 +8400,7 @@ qemuDomainDefCopy(virQEMUDriverPtr driver,
 
 static int
 qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
+                               virQEMUCapsPtr qemuCaps,
                                virDomainDefPtr def,
                                virCPUDefPtr origCPU,
                                unsigned int flags,
@@ -8408,7 +8409,6 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
     int ret = -1;
     virDomainDefPtr copy = NULL;
     virCapsPtr caps = NULL;
-    virQEMUCapsPtr qemuCaps = NULL;
 
     virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS | VIR_DOMAIN_XML_UPDATE_CPU, -1);
 
@@ -8418,7 +8418,7 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
     if (!(flags & (VIR_DOMAIN_XML_UPDATE_CPU | VIR_DOMAIN_XML_MIGRATABLE)))
         goto format;
 
-    if (!(copy = virDomainDefCopy(def, caps, driver->xmlopt, NULL,
+    if (!(copy = virDomainDefCopy(def, caps, driver->xmlopt, qemuCaps,
                                   flags & VIR_DOMAIN_XML_MIGRATABLE)))
         goto cleanup;
 
@@ -8429,13 +8429,19 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
         def->cpu &&
         (def->cpu->mode != VIR_CPU_MODE_CUSTOM ||
          def->cpu->model)) {
-        if (!(qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache,
-                                                    def->emulator,
-                                                    def->os.machine)))
-            goto cleanup;
+        VIR_AUTOUNREF(virQEMUCapsPtr) qCaps = NULL;
+
+        if (qemuCaps) {
+            qCaps = virObjectRef(qemuCaps);
+        } else {
+            if (!(qCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache,
+                                                     def->emulator,
+                                                     def->os.machine)))
+                goto cleanup;
+        }
 
         if (virCPUUpdate(def->os.arch, def->cpu,
-                         virQEMUCapsGetHostModel(qemuCaps, def->virtType,
+                         virQEMUCapsGetHostModel(qCaps, def->virtType,
                                                  VIR_QEMU_CAPS_HOST_CPU_MIGRATABLE)) < 0)
             goto cleanup;
     }
@@ -8584,30 +8590,31 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
  cleanup:
     virDomainDefFree(copy);
     virObjectUnref(caps);
-    virObjectUnref(qemuCaps);
     return ret;
 }
 
 
 int
 qemuDomainDefFormatBuf(virQEMUDriverPtr driver,
+                       virQEMUCapsPtr qemuCaps,
                        virDomainDefPtr def,
                        unsigned int flags,
                        virBufferPtr buf)
 {
-    return qemuDomainDefFormatBufInternal(driver, def, NULL, flags, buf);
+    return qemuDomainDefFormatBufInternal(driver, qemuCaps, def, NULL, flags, buf);
 }
 
 
 static char *
 qemuDomainDefFormatXMLInternal(virQEMUDriverPtr driver,
+                               virQEMUCapsPtr qemuCaps,
                                virDomainDefPtr def,
                                virCPUDefPtr origCPU,
                                unsigned int flags)
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;
 
-    if (qemuDomainDefFormatBufInternal(driver, def, origCPU, flags, &buf) < 0)
+    if (qemuDomainDefFormatBufInternal(driver, qemuCaps, def, origCPU, flags, &buf) < 0)
         return NULL;
 
     return virBufferContentAndReset(&buf);
@@ -8616,10 +8623,11 @@ qemuDomainDefFormatXMLInternal(virQEMUDriverPtr driver,
 
 char *
 qemuDomainDefFormatXML(virQEMUDriverPtr driver,
+                       virQEMUCapsPtr qemuCaps,
                        virDomainDefPtr def,
                        unsigned int flags)
 {
-    return qemuDomainDefFormatXMLInternal(driver, def, NULL, flags);
+    return qemuDomainDefFormatXMLInternal(driver, qemuCaps, def, NULL, flags);
 }
 
 
@@ -8638,11 +8646,12 @@ char *qemuDomainFormatXML(virQEMUDriverPtr driver,
         origCPU = priv->origCPU;
     }
 
-    return qemuDomainDefFormatXMLInternal(driver, def, origCPU, flags);
+    return qemuDomainDefFormatXMLInternal(driver, priv->qemuCaps, def, origCPU, flags);
 }
 
 char *
 qemuDomainDefFormatLive(virQEMUDriverPtr driver,
+                        virQEMUCapsPtr qemuCaps,
                         virDomainDefPtr def,
                         virCPUDefPtr origCPU,
                         bool inactive,
@@ -8655,7 +8664,7 @@ qemuDomainDefFormatLive(virQEMUDriverPtr driver,
     if (compatible)
         flags |= VIR_DOMAIN_XML_MIGRATABLE;
 
-    return qemuDomainDefFormatXMLInternal(driver, def, origCPU, flags);
+    return qemuDomainDefFormatXMLInternal(driver, qemuCaps, def, origCPU, flags);
 }
 
 

src/qemu/qemu_domain.h

@@ -654,11 +654,13 @@ virDomainDefPtr qemuDomainDefCopy(virQEMUDriverPtr driver,
                                   unsigned int flags);
 
 int qemuDomainDefFormatBuf(virQEMUDriverPtr driver,
+                           virQEMUCapsPtr qemuCaps,
                            virDomainDefPtr vm,
                            unsigned int flags,
                            virBuffer *buf);
 
 char *qemuDomainDefFormatXML(virQEMUDriverPtr driver,
+                             virQEMUCapsPtr qemuCaps,
                              virDomainDefPtr vm,
                              unsigned int flags);
 
@@ -667,6 +669,7 @@ char *qemuDomainFormatXML(virQEMUDriverPtr driver,
                           unsigned int flags);
 
 char *qemuDomainDefFormatLive(virQEMUDriverPtr driver,
+                              virQEMUCapsPtr qemuCaps,
                               virDomainDefPtr def,
                               virCPUDefPtr origCPU,
                               bool inactive,

src/qemu/qemu_driver.c

@@ -3446,9 +3446,10 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver,
             virDomainDefFree(def);
             goto endjob;
         }
-        xml = qemuDomainDefFormatLive(driver, def, NULL, true, true);
+        xml = qemuDomainDefFormatLive(driver, priv->qemuCaps, def, NULL, true, true);
     } else {
-        xml = qemuDomainDefFormatLive(driver, vm->def, priv->origCPU, true, true);
+        xml = qemuDomainDefFormatLive(driver, priv->qemuCaps, vm->def,
+                                      priv->origCPU, true, true);
     }
     if (!xml) {
         virReportError(VIR_ERR_OPERATION_FAILED,
@@ -7231,7 +7232,7 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path,
     if (virDomainSaveImageGetXMLDescEnsureACL(conn, def) < 0)
         goto cleanup;
 
-    ret = qemuDomainDefFormatXML(driver, def, flags);
+    ret = qemuDomainDefFormatXML(driver, NULL, def, flags);
 
  cleanup:
     virQEMUSaveDataFree(data);
@@ -7284,7 +7285,7 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path,
 
     VIR_FREE(data->xml);
 
-    if (!(data->xml = qemuDomainDefFormatXML(driver, newdef,
+    if (!(data->xml = qemuDomainDefFormatXML(driver, NULL, newdef,
                                              VIR_DOMAIN_XML_INACTIVE |
                                              VIR_DOMAIN_XML_SECURE |
                                              VIR_DOMAIN_XML_MIGRATABLE)))
@@ -7323,12 +7324,15 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags)
     virDomainDefPtr def = NULL;
     int fd = -1;
     virQEMUSaveDataPtr data = NULL;
+    qemuDomainObjPrivatePtr priv;
 
     virCheckFlags(VIR_DOMAIN_SAVE_IMAGE_XML_SECURE, NULL);
 
     if (!(vm = qemuDomObjFromDomain(dom)))
         return ret;
 
+    priv = vm->privateData;
+
     if (virDomainManagedSaveGetXMLDescEnsureACL(dom->conn, vm->def, flags) < 0)
         goto cleanup;
 
@@ -7345,7 +7349,7 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags)
                                       false, NULL, false, false)) < 0)
         goto cleanup;
 
-    ret = qemuDomainDefFormatXML(driver, def, flags);
+    ret = qemuDomainDefFormatXML(driver, priv->qemuCaps, def, flags);
 
  cleanup:
     virQEMUSaveDataFree(data);
@@ -15639,7 +15643,8 @@ qemuDomainSnapshotCreateActiveExternal(virQEMUDriverPtr driver,
                                                     "snapshot", false)) < 0)
             goto cleanup;
 
-        if (!(xml = qemuDomainDefFormatLive(driver, vm->def, priv->origCPU,
+        if (!(xml = qemuDomainDefFormatLive(driver, priv->qemuCaps,
+                                            vm->def, priv->origCPU,
                                             true, true)) ||
             !(snapdef->cookie = (virObjectPtr) qemuDomainSaveCookieNew(vm)))
             goto cleanup;
@@ -15897,7 +15902,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
     } else {
         /* Easiest way to clone inactive portion of vm->def is via
          * conversion in and back out of xml.  */
-        if (!(xml = qemuDomainDefFormatLive(driver, vm->def, priv->origCPU,
+        if (!(xml = qemuDomainDefFormatLive(driver, priv->qemuCaps,
+                                            vm->def, priv->origCPU,
                                             true, true)) ||
             !(def->parent.dom = virDomainDefParseString(xml, caps, driver->xmlopt, NULL,
                                                         VIR_DOMAIN_DEF_PARSE_INACTIVE |
@@ -16997,7 +17003,8 @@ qemuDomainCheckpointPrepare(virQEMUDriverPtr driver, virCapsPtr caps,
 
     /* Easiest way to clone inactive portion of vm->def is via
      * conversion in and back out of xml.  */
-    if (!(xml = qemuDomainDefFormatLive(driver, vm->def, priv->origCPU,
+    if (!(xml = qemuDomainDefFormatLive(driver, priv->qemuCaps,
+                                        vm->def, priv->origCPU,
                                         true, true)) ||
         !(def->parent.dom = virDomainDefParseString(xml, caps, driver->xmlopt, NULL,
                                                     VIR_DOMAIN_DEF_PARSE_INACTIVE |

src/qemu/qemu_migration.c

@@ -2083,9 +2083,9 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver,
         if (!qemuDomainCheckABIStability(driver, vm, def))
             goto cleanup;
 
-        rv = qemuDomainDefFormatLive(driver, def, NULL, false, true);
+        rv = qemuDomainDefFormatLive(driver, priv->qemuCaps, def, NULL, false, true);
     } else {
-        rv = qemuDomainDefFormatLive(driver, vm->def, priv->origCPU,
+        rv = qemuDomainDefFormatLive(driver, priv->qemuCaps, vm->def, priv->origCPU,
                                      false, true);
     }
 
@@ -2355,7 +2355,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver,
         char *xml;
         int hookret;
 
-        if (!(xml = qemuDomainDefFormatXML(driver, *def,
+        if (!(xml = qemuDomainDefFormatXML(driver, NULL, *def,
                                            VIR_DOMAIN_XML_SECURE |
                                            VIR_DOMAIN_XML_MIGRATABLE)))
             goto cleanup;

src/qemu/qemu_migration_cookie.c

@@ -781,6 +781,7 @@ qemuMigrationCookieCapsXMLFormat(virBufferPtr buf,
 
 static int
 qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver,
+                             virQEMUCapsPtr qemuCaps,
                              virBufferPtr buf,
                              qemuMigrationCookiePtr mig)
 {
@@ -822,6 +823,7 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver,
     if ((mig->flags & QEMU_MIGRATION_COOKIE_PERSISTENT) &&
         mig->persistent) {
         if (qemuDomainDefFormatBuf(driver,
+                                   qemuCaps,
                                    mig->persistent,
                                    VIR_DOMAIN_XML_INACTIVE |
                                    VIR_DOMAIN_XML_SECURE |
@@ -873,11 +875,12 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver,
 
 static char *
 qemuMigrationCookieXMLFormatStr(virQEMUDriverPtr driver,
+                                virQEMUCapsPtr qemuCaps,
                                 qemuMigrationCookiePtr mig)
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;
 
-    if (qemuMigrationCookieXMLFormat(driver, &buf, mig) < 0) {
+    if (qemuMigrationCookieXMLFormat(driver, qemuCaps, &buf, mig) < 0) {
         virBufferFreeAndReset(&buf);
         return NULL;
     }
@@ -1419,6 +1422,8 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig,
                         int *cookieoutlen,
                         unsigned int flags)
 {
+    qemuDomainObjPrivatePtr priv = dom->privateData;
+
     if (!cookieout || !cookieoutlen)
         return 0;
 
@@ -1462,7 +1467,7 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig,
         qemuMigrationCookieAddCaps(mig, dom, party) < 0)
         return -1;
 
-    if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig)))
+    if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, priv->qemuCaps, mig)))
         return -1;
 
     *cookieoutlen = strlen(*cookieout) + 1;

src/qemu/qemu_process.c

@@ -4644,13 +4644,14 @@ qemuProcessStartHook(virQEMUDriverPtr driver,
                      virHookQemuOpType op,
                      virHookSubopType subop)
 {
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     char *xml;
     int ret;
 
     if (!virHookPresent(VIR_HOOK_DRIVER_QEMU))
         return 0;
 
-    if (!(xml = qemuDomainDefFormatXML(driver, vm->def, 0)))
+    if (!(xml = qemuDomainDefFormatXML(driver, priv->qemuCaps, vm->def, 0)))
         return -1;
 
     ret = virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name, op, subop,
@@ -7482,7 +7483,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 
     /* now that we know it's stopped call the hook if present */
     if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
-        char *xml = qemuDomainDefFormatXML(driver, vm->def, 0);
+        char *xml = qemuDomainDefFormatXML(driver, NULL, vm->def, 0);
 
         /* we can't stop the operation even if the script raised an error */
         ignore_value(virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
@@ -7658,7 +7659,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 
     /* The "release" hook cleans up additional resources */
     if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
-        char *xml = qemuDomainDefFormatXML(driver, vm->def, 0);
+        char *xml = qemuDomainDefFormatXML(driver, NULL, vm->def, 0);
 
         /* we can't stop the operation even if the script raised an error */
         virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
@@ -8220,7 +8221,7 @@ qemuProcessReconnect(void *opaque)
 
     /* Run an hook to allow admins to do some magic */
     if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
-        char *xml = qemuDomainDefFormatXML(driver, obj->def, 0);
+        char *xml = qemuDomainDefFormatXML(driver, priv->qemuCaps, obj->def, 0);
         int hookret;
 
         hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, obj->def->name,