[libvirt] [PATCH 2/2] qemu: Avoid deadlock in autodestroy
Since closeCallbacks were turned into virObjectLockable, we can no
longer call virQEMUCloseCallbacks APIs from within a registered close
callback.
---
src/qemu/qemu_conf.c | 19 ++++---------------
src/qemu/qemu_conf.h | 4 ++++
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_process.c | 10 +++++++++-
4 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index edadf36..fc8e152 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -825,8 +825,6 @@ virQEMUCloseCallbacksRunOne(void *payload,
{
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",
@@ -835,18 +833,9 @@ virQEMUCloseCallbacksRunOne(void *payload,
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);
+ if (!(dom = virDomainObjListFindByUUID(data->driver->domains, name))) {
+ VIR_DEBUG("No domain object with UUID %s",
+ (const char *) name);
return;
}
@@ -854,7 +843,7 @@ virQEMUCloseCallbacksRunOne(void *payload,
if (dom)
virObjectUnlock(dom);
- virHashRemoveEntry(data->list, uuidstr);
+ virHashRemoveEntry(data->list, name);
}
void
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 72380dc..b5a3281 100644
--- 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 905b099..c9d5f8b 100644
--- 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 f987618..74406a6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4263,7 +4263,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)) {
@@ -4647,8 +4648,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,
--
1.8.1.2