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

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index dba5db68ecd60455024d7b680ca1c00a12326cb6..a95c32d32826a5642eb1d4fbbc92da2e86442932 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/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);
 }
 
 

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 94ff6e7bd3bd99c1cef091f03fdb70ace9bcd7ba..2ba51929db9018ebfd39cb704ce6579f7cdbd308 100644 (file)
--- a/src/qemu/qemu_domain.h
+++ b/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,

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4d6a9332436a011d84f19b7ed0b1d1342a47d543..75d6b3a9527f25c87e8060f673e882ae0e0e7e90 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/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 |

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ec1cae8beca39d26aaf19fe1a80b3d9774089d39..39e574ee3031e73bd25db2417a3bc146a0293943 100644 (file)
--- a/src/qemu/qemu_migration.c
+++ b/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;

diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c
index 74b8575a91007891a7e8bfd73adfb28b5eaaad64..74a12d1b0353dfa648ff4715ee42085ac9108dcc 100644 (file)
--- a/src/qemu/qemu_migration_cookie.c
+++ b/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;

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1ed56457b186f1a2522d492f2c29e9536b72f1af..9345ecf1beeb94902fd20a916a4f37418f1bf4b0 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/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,