qemu: Avoid deadlock in autodestroy

Since closeCallbacks were turned into virObjectLockable, we can no
longer call virQEMUCloseCallbacks APIs from within a registered close
callback.

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 82245e27de5d2a65b7a2e16c03fb8a8fd71c8f70..822075aeb960b3bd9de69b0231c203baf19caf6d 100644 (file)
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -810,33 +810,22 @@ struct virQEMUCloseCallbacksData {
 
 static void
 virQEMUCloseCallbacksRunOne(void *payload,
-                            const void *name,
+                            const void *uuid,
                             void *opaque)
 {
     struct virQEMUCloseCallbacksData *data = opaque;
     qemuDriverCloseDefPtr closeDef = payload;
-    unsigned char uuid[VIR_UUID_BUFLEN];
-    char uuidstr[VIR_UUID_STRING_BUFLEN];
     virDomainObjPtr dom;
 
     VIR_DEBUG("conn=%p, thisconn=%p, uuid=%s, cb=%p",
-              closeDef->conn, data->conn, (const char *)name, closeDef->cb);
+              closeDef->conn, data->conn, (const char *) uuid, closeDef->cb);
 
     if (data->conn != closeDef->conn || !closeDef->cb)
         return;
 
-    if (virUUIDParse(name, uuid) < 0) {
-        VIR_WARN("Failed to parse %s", (const char *)name);
-        return;
-    }
-    /* We need to reformat uuidstr, because closeDef->cb
-     * might cause the current hash entry to be removed,
-     * which means 'name' will have been free()d
-     */
-    virUUIDFormat(uuid, uuidstr);
-
     if (!(dom = virDomainObjListFindByUUID(data->driver->domains, uuid))) {
-        VIR_DEBUG("No domain object with UUID %s", uuidstr);
+        VIR_DEBUG("No domain object with UUID %s",
+                  (const char *) uuid);
         return;
     }
 
@@ -844,7 +833,7 @@ virQEMUCloseCallbacksRunOne(void *payload,
     if (dom)
         virObjectUnlock(dom);
 
-    virHashRemoveEntry(data->list, uuidstr);
+    virHashRemoveEntry(data->list, uuid);
 }
 
 void

diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 6bb3dee8c3e84ce0ac96a44134b6dfe6d66424e1..d547d978d28a59435ed9150d4159779c39c4a833 100644 (file)
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -254,6 +254,10 @@ struct qemuDomainDiskInfo {
     int io_status;
 };
 
+/*
+ * To avoid a certain deadlock this callback must never call any
+ * virQEMUCloseCallbacks* API.
+ */
 typedef virDomainObjPtr (*virQEMUCloseCallback)(virQEMUDriverPtr driver,
                                                 virDomainObjPtr vm,
                                                 virConnectPtr conn);

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 905b09951201ae56b8f4a8f234e0680914be5102..c9d5f8bff879f79def2eb6f7f85a2804d312096b 100644 (file)
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -137,6 +137,7 @@ struct _qemuDomainObjPrivate {
 
     bool gotShutdown;
     bool beingDestroyed;
+    bool autoDestroyed;
     char *pidfile;
 
     int nvcpupids;

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 994417bb7ede9d35b184cfcec13980c321fe90b2..aade68268904ac8afb00ea9c541ccee33511fbce 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4223,7 +4223,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
     qemuDomainCleanupRun(driver, vm);
 
     /* Stop autodestroy in case guest is restarted */
-    qemuProcessAutoDestroyRemove(driver, vm);
+    if (!priv->autoDestroyed)
+        qemuProcessAutoDestroyRemove(driver, vm);
 
     /* now that we know it's stopped call the hook if present */
     if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
@@ -4603,8 +4604,15 @@ qemuProcessAutoDestroy(virQEMUDriverPtr driver,
         goto cleanup;
 
     VIR_DEBUG("Killing domain");
+
+    /* We need to prevent qemuProcessStop from removing this function from
+     * closeCallbacks since that would cause a deadlock.
+     */
+    priv->autoDestroyed = true;
     qemuProcessStop(driver, dom, VIR_DOMAIN_SHUTOFF_DESTROYED,
                     VIR_QEMU_PROCESS_STOP_MIGRATED);
+    priv->autoDestroyed = false;
+
     virDomainAuditStop(dom, "destroyed");
     event = virDomainEventNewFromObj(dom,
                                      VIR_DOMAIN_EVENT_STOPPED,