99cbc7
From 317ea40b7e1bedecfad0fca4d9d314e41d1f2819 Mon Sep 17 00:00:00 2001
99cbc7
Message-Id: <317ea40b7e1bedecfad0fca4d9d314e41d1f2819@dist-git>
99cbc7
From: Jiri Denemark <jdenemar@redhat.com>
99cbc7
Date: Fri, 16 Aug 2019 14:52:27 +0200
99cbc7
Subject: [PATCH] qemu: Pass qemuCaps to qemuDomainDefCopy
99cbc7
MIME-Version: 1.0
99cbc7
Content-Type: text/plain; charset=UTF-8
99cbc7
Content-Transfer-Encoding: 8bit
99cbc7
99cbc7
Since qemuDomainDefPostParse callback requires qemuCaps, we need to make
99cbc7
sure it gets the capabilities stored in the domain's private data if the
99cbc7
domain is running. Passing NULL may cause QEMU capabilities probing to
99cbc7
be triggered in case QEMU binary changed in the meantime. When this
99cbc7
happens while a running domain object is locked, QMP event delivered to
99cbc7
the domain before QEMU capabilities probing finishes will deadlock the
99cbc7
event loop.
99cbc7
99cbc7
This patch fixes all paths leading to qemuDomainDefCopy.
99cbc7
99cbc7
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
99cbc7
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
99cbc7
(cherry picked from commit a42f889591c16235e0fe349e509af896fa1ea5ff)
99cbc7
99cbc7
Conflicts:
99cbc7
	src/qemu/qemu_driver.c
99cbc7
            - context
99cbc7
99cbc7
https://bugzilla.redhat.com/show_bug.cgi?id=1731783
99cbc7
99cbc7
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
99cbc7
Message-Id: <6ecee413894eadadaf37884be7dd4136ad2de403.1565959866.git.jdenemar@redhat.com>
99cbc7
Reviewed-by: Ján Tomko <jtomko@redhat.com>
99cbc7
---
99cbc7
 src/qemu/qemu_domain.c    | 16 ++++++++++------
99cbc7
 src/qemu/qemu_domain.h    |  2 ++
99cbc7
 src/qemu/qemu_driver.c    |  9 +++++----
99cbc7
 src/qemu/qemu_migration.c |  4 ++--
99cbc7
 4 files changed, 19 insertions(+), 12 deletions(-)
99cbc7
99cbc7
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
99cbc7
index 81699110fc..bf6013d42a 100644
99cbc7
--- a/src/qemu/qemu_domain.c
99cbc7
+++ b/src/qemu/qemu_domain.c
99cbc7
@@ -7203,6 +7203,7 @@ qemuDomainObjExitRemote(virDomainObjPtr obj,
99cbc7
 
99cbc7
 static virDomainDefPtr
99cbc7
 qemuDomainDefFromXML(virQEMUDriverPtr driver,
99cbc7
+                     virQEMUCapsPtr qemuCaps,
99cbc7
                      const char *xml)
99cbc7
 {
99cbc7
     virCapsPtr caps;
99cbc7
@@ -7211,7 +7212,7 @@ qemuDomainDefFromXML(virQEMUDriverPtr driver,
99cbc7
     if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
99cbc7
         return NULL;
99cbc7
 
99cbc7
-    def = virDomainDefParseString(xml, caps, driver->xmlopt, NULL,
99cbc7
+    def = virDomainDefParseString(xml, caps, driver->xmlopt, qemuCaps,
99cbc7
                                   VIR_DOMAIN_DEF_PARSE_INACTIVE |
99cbc7
                                   VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE);
99cbc7
 
99cbc7
@@ -7222,6 +7223,7 @@ qemuDomainDefFromXML(virQEMUDriverPtr driver,
99cbc7
 
99cbc7
 virDomainDefPtr
99cbc7
 qemuDomainDefCopy(virQEMUDriverPtr driver,
99cbc7
+                  virQEMUCapsPtr qemuCaps,
99cbc7
                   virDomainDefPtr src,
99cbc7
                   unsigned int flags)
99cbc7
 {
99cbc7
@@ -7231,7 +7233,7 @@ qemuDomainDefCopy(virQEMUDriverPtr driver,
99cbc7
     if (!(xml = qemuDomainDefFormatXML(driver, src, flags)))
99cbc7
         return NULL;
99cbc7
 
99cbc7
-    ret = qemuDomainDefFromXML(driver, xml);
99cbc7
+    ret = qemuDomainDefFromXML(driver, qemuCaps, xml);
99cbc7
 
99cbc7
     VIR_FREE(xml);
99cbc7
     return ret;
99cbc7
@@ -9052,6 +9054,7 @@ qemuDomainMigratableDefCheckABIStability(virQEMUDriverPtr driver,
99cbc7
 
99cbc7
 bool
99cbc7
 qemuDomainDefCheckABIStability(virQEMUDriverPtr driver,
99cbc7
+                               virQEMUCapsPtr qemuCaps,
99cbc7
                                virDomainDefPtr src,
99cbc7
                                virDomainDefPtr dst)
99cbc7
 {
99cbc7
@@ -9059,8 +9062,8 @@ qemuDomainDefCheckABIStability(virQEMUDriverPtr driver,
99cbc7
     virDomainDefPtr migratableDefDst = NULL;
99cbc7
     bool ret = false;
99cbc7
 
99cbc7
-    if (!(migratableDefSrc = qemuDomainDefCopy(driver, src, COPY_FLAGS)) ||
99cbc7
-        !(migratableDefDst = qemuDomainDefCopy(driver, dst, COPY_FLAGS)))
99cbc7
+    if (!(migratableDefSrc = qemuDomainDefCopy(driver, qemuCaps, src, COPY_FLAGS)) ||
99cbc7
+        !(migratableDefDst = qemuDomainDefCopy(driver, qemuCaps, dst, COPY_FLAGS)))
99cbc7
         goto cleanup;
99cbc7
 
99cbc7
     ret = qemuDomainMigratableDefCheckABIStability(driver,
99cbc7
@@ -9079,14 +9082,15 @@ qemuDomainCheckABIStability(virQEMUDriverPtr driver,
99cbc7
                             virDomainObjPtr vm,
99cbc7
                             virDomainDefPtr dst)
99cbc7
 {
99cbc7
+    qemuDomainObjPrivatePtr priv = vm->privateData;
99cbc7
     virDomainDefPtr migratableSrc = NULL;
99cbc7
     virDomainDefPtr migratableDst = NULL;
99cbc7
     char *xml = NULL;
99cbc7
     bool ret = false;
99cbc7
 
99cbc7
     if (!(xml = qemuDomainFormatXML(driver, vm, COPY_FLAGS)) ||
99cbc7
-        !(migratableSrc = qemuDomainDefFromXML(driver, xml)) ||
99cbc7
-        !(migratableDst = qemuDomainDefCopy(driver, dst, COPY_FLAGS)))
99cbc7
+        !(migratableSrc = qemuDomainDefFromXML(driver, priv->qemuCaps, xml)) ||
99cbc7
+        !(migratableDst = qemuDomainDefCopy(driver, priv->qemuCaps, dst, COPY_FLAGS)))
99cbc7
         goto cleanup;
99cbc7
 
99cbc7
     ret = qemuDomainMigratableDefCheckABIStability(driver,
99cbc7
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
99cbc7
index cc406e3ca0..29283105cb 100644
99cbc7
--- a/src/qemu/qemu_domain.h
99cbc7
+++ b/src/qemu/qemu_domain.h
99cbc7
@@ -594,6 +594,7 @@ int qemuDomainObjExitRemote(virDomainObjPtr obj,
99cbc7
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
99cbc7
 
99cbc7
 virDomainDefPtr qemuDomainDefCopy(virQEMUDriverPtr driver,
99cbc7
+                                  virQEMUCapsPtr qemuCaps,
99cbc7
                                   virDomainDefPtr src,
99cbc7
                                   unsigned int flags);
99cbc7
 
99cbc7
@@ -769,6 +770,7 @@ int qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver,
99cbc7
                                      int asyncJob);
99cbc7
 
99cbc7
 bool qemuDomainDefCheckABIStability(virQEMUDriverPtr driver,
99cbc7
+                                    virQEMUCapsPtr qemuCaps,
99cbc7
                                     virDomainDefPtr src,
99cbc7
                                     virDomainDefPtr dst);
99cbc7
 
99cbc7
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
99cbc7
index 25818f5d8c..a486c66a5f 100644
99cbc7
--- a/src/qemu/qemu_driver.c
99cbc7
+++ b/src/qemu/qemu_driver.c
99cbc7
@@ -6325,7 +6325,7 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver,
99cbc7
                                            VIR_DOMAIN_DEF_PARSE_INACTIVE)))
99cbc7
         goto cleanup;
99cbc7
 
99cbc7
-    if (!(newdef_migr = qemuDomainDefCopy(driver,
99cbc7
+    if (!(newdef_migr = qemuDomainDefCopy(driver, NULL,
99cbc7
                                           newdef,
99cbc7
                                           QEMU_DOMAIN_FORMAT_LIVE_FLAGS |
99cbc7
                                           VIR_DOMAIN_XML_MIGRATABLE)))
99cbc7
@@ -16052,7 +16052,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
99cbc7
     switch ((virDomainState) snap->def->state) {
99cbc7
     case VIR_DOMAIN_RUNNING:
99cbc7
     case VIR_DOMAIN_PAUSED:
99cbc7
-
99cbc7
+        priv = vm->privateData;
99cbc7
         start_flags |= VIR_QEMU_PROCESS_START_PAUSED;
99cbc7
 
99cbc7
         /* Transitions 2, 3, 5, 6, 8, 9 */
99cbc7
@@ -16079,7 +16079,9 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
99cbc7
                     if (!(config->cpu = virCPUDefCopy(cookie->cpu)))
99cbc7
                         goto endjob;
99cbc7
 
99cbc7
-                    compatible = qemuDomainDefCheckABIStability(driver, vm->def,
99cbc7
+                    compatible = qemuDomainDefCheckABIStability(driver,
99cbc7
+                                                                priv->qemuCaps,
99cbc7
+                                                                vm->def,
99cbc7
                                                                 config);
99cbc7
                 } else {
99cbc7
                     compatible = qemuDomainCheckABIStability(driver, vm, config);
99cbc7
@@ -16123,7 +16125,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
99cbc7
                 }
99cbc7
             }
99cbc7
 
99cbc7
-            priv = vm->privateData;
99cbc7
             if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
99cbc7
                 /* Transitions 5, 6 */
99cbc7
                 if (qemuProcessStopCPUs(driver, vm,
99cbc7
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
99cbc7
index 67940330aa..4af105b997 100644
99cbc7
--- a/src/qemu/qemu_migration.c
99cbc7
+++ b/src/qemu/qemu_migration.c
99cbc7
@@ -2378,7 +2378,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver,
99cbc7
                 if (!newdef)
99cbc7
                     goto cleanup;
99cbc7
 
99cbc7
-                if (!qemuDomainDefCheckABIStability(driver, *def, newdef)) {
99cbc7
+                if (!qemuDomainDefCheckABIStability(driver, NULL, *def, newdef)) {
99cbc7
                     virDomainDefFree(newdef);
99cbc7
                     goto cleanup;
99cbc7
                 }
99cbc7
@@ -3417,7 +3417,7 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver,
99cbc7
                 goto error;
99cbc7
         } else {
99cbc7
             virDomainDefPtr def = vm->newDef ? vm->newDef : vm->def;
99cbc7
-            if (!(persistDef = qemuDomainDefCopy(driver, def,
99cbc7
+            if (!(persistDef = qemuDomainDefCopy(driver, priv->qemuCaps, def,
99cbc7
                                                  VIR_DOMAIN_XML_SECURE |
99cbc7
                                                  VIR_DOMAIN_XML_MIGRATABLE)))
99cbc7
                 goto error;
99cbc7
-- 
99cbc7
2.22.1
99cbc7