Fix deadlock in QEMU close callback APIs

There is a lock ordering problem in the QEMU close callback
APIs.

When starting a guest we have a lock on the VM. We then
set a autodestroy callback, which acquires a lock on the
close callbacks.

When running auto-destroy, we obtain a lock on the close
callbacks, then run each callbacks - which obtains a lock
on the VM.

This causes deadlock if anyone tries to start a VM, while
autodestroy is taking place.

The fix is to do autodestroy in 2 phases. First obtain
all the callbacks and remove them from the list under
the close callback lock. Then invoke each callback
from outside the close callback lock.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
上级 7ccad0b1
......@@ -797,20 +797,37 @@ virQEMUCloseCallbacksGet(virQEMUCloseCallbacksPtr closeCallbacks,
return cb;
}
typedef struct _virQEMUCloseCallbacksListEntry virQEMUCloseCallbacksListEntry;
typedef virQEMUCloseCallbacksListEntry *virQEMUCloseCallbacksListEntryPtr;
struct _virQEMUCloseCallbacksListEntry {
unsigned char uuid[VIR_UUID_BUFLEN];
virQEMUCloseCallback callback;
};
typedef struct _virQEMUCloseCallbacksList virQEMUCloseCallbacksList;
typedef virQEMUCloseCallbacksList *virQEMUCloseCallbacksListPtr;
struct _virQEMUCloseCallbacksList {
size_t nentries;
virQEMUCloseCallbacksListEntryPtr entries;
};
struct virQEMUCloseCallbacksData {
virHashTablePtr list;
virQEMUDriverPtr driver;
virConnectPtr conn;
virQEMUCloseCallbacksListPtr list;
bool oom;
};
static void
virQEMUCloseCallbacksRunOne(void *payload,
virQEMUCloseCallbacksGetOne(void *payload,
const void *key,
void *opaque)
{
struct virQEMUCloseCallbacksData *data = opaque;
qemuDriverCloseDefPtr closeDef = payload;
virDomainObjPtr dom;
const char *uuidstr = key;
unsigned char uuid[VIR_UUID_BUFLEN];
......@@ -823,35 +840,90 @@ virQEMUCloseCallbacksRunOne(void *payload,
if (data->conn != closeDef->conn || !closeDef->cb)
return;
if (!(dom = virDomainObjListFindByUUID(data->driver->domains, uuid))) {
VIR_DEBUG("No domain object with UUID %s", uuidstr);
if (VIR_EXPAND_N(data->list->entries,
data->list->nentries, 1) < 0) {
data->oom = true;
return;
}
dom = closeDef->cb(data->driver, dom, data->conn);
if (dom)
virObjectUnlock(dom);
memcpy(data->list->entries[data->list->nentries - 1].uuid,
uuid, VIR_UUID_BUFLEN);
data->list->entries[data->list->nentries - 1].callback = closeDef->cb;
}
static virQEMUCloseCallbacksListPtr
virQEMUCloseCallbacksGetForConn(virQEMUCloseCallbacksPtr closeCallbacks,
virConnectPtr conn)
{
virQEMUCloseCallbacksListPtr list = NULL;
struct virQEMUCloseCallbacksData data;
if (VIR_ALLOC(list) < 0) {
virReportOOMError();
return NULL;
}
data.conn = conn;
data.list = list;
data.oom = false;
virHashForEach(closeCallbacks->list, virQEMUCloseCallbacksGetOne, &data);
virHashRemoveEntry(data->list, uuid);
if (data.oom) {
VIR_FREE(list->entries);
VIR_FREE(list);
virReportOOMError();
return NULL;
}
return list;
}
void
virQEMUCloseCallbacksRun(virQEMUCloseCallbacksPtr closeCallbacks,
virConnectPtr conn,
virQEMUDriverPtr driver)
{
struct virQEMUCloseCallbacksData data = {
.driver = driver,
.conn = conn
};
virQEMUCloseCallbacksListPtr list;
size_t i;
VIR_DEBUG("conn=%p", conn);
virObjectLock(closeCallbacks);
data.list = closeCallbacks->list;
virHashForEach(data.list, virQEMUCloseCallbacksRunOne, &data);
/* We must not hold the lock while running the callbacks,
* so first we obtain the list of callbacks, then remove
* them all from the hash. At that point we can release
* the lock and run the callbacks safely. */
virObjectLock(closeCallbacks);
list = virQEMUCloseCallbacksGetForConn(closeCallbacks, conn);
if (!list)
return;
for (i = 0 ; i < list->nentries ; i++) {
virHashRemoveEntry(closeCallbacks->list,
list->entries[i].uuid);
}
virObjectUnlock(closeCallbacks);
for (i = 0 ; i < list->nentries ; i++) {
virDomainObjPtr vm;
if (!(vm = virDomainObjListFindByUUID(driver->domains,
list->entries[i].uuid))) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(list->entries[i].uuid, uuidstr);
VIR_DEBUG("No domain object with UUID %s", uuidstr);
continue;
}
vm = list->entries[i].callback(driver, vm, conn);
if (vm)
virObjectUnlock(vm);
}
VIR_FREE(list->entries);
VIR_FREE(list);
}
struct _qemuSharedDiskEntry {
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册
反馈
建议
客服 返回
顶部