qemu: Turn closeCallbacks into virObjectLockable

To avoid having to hold the qemu driver lock while iterating through
close callbacks and calling them. This fixes a real deadlock when a
domain which is being migrated from another host gets autodestoyed as a
result of broken connection to the other host.
This commit is contained in:
Jiri Denemark 2013-02-15 13:05:12 +01:00
parent 091831633f
commit 3898ba7f2c
5 changed files with 140 additions and 93 deletions

View File

@ -57,28 +57,47 @@
#define VIR_FROM_THIS VIR_FROM_QEMU
typedef struct _qemuDriverCloseDef qemuDriverCloseDef;
typedef qemuDriverCloseDef *qemuDriverCloseDefPtr;
struct _qemuDriverCloseDef {
virConnectPtr conn;
virQEMUCloseCallback cb;
};
struct _virQEMUCloseCallbacks {
virObjectLockable parent;
/* UUID string to qemuDriverCloseDef mapping */
virHashTablePtr list;
};
static virClassPtr virQEMUDriverConfigClass;
static virClassPtr virQEMUCloseCallbacksClass;
static void virQEMUDriverConfigDispose(void *obj);
static void virQEMUCloseCallbacksDispose(void *obj);
static int virQEMUConfigOnceInit(void)
{
if (!(virQEMUDriverConfigClass = virClassNew(virClassForObject(),
"virQEMUDriverConfig",
sizeof(virQEMUDriverConfig),
virQEMUDriverConfigDispose)))
return -1;
virQEMUDriverConfigClass = virClassNew(virClassForObject(),
"virQEMUDriverConfig",
sizeof(virQEMUDriverConfig),
virQEMUDriverConfigDispose);
return 0;
virQEMUCloseCallbacksClass = virClassNew(virClassForObjectLockable(),
"virQEMUCloseCallbacks",
sizeof(virQEMUCloseCallbacks),
virQEMUCloseCallbacksDispose);
if (!virQEMUDriverConfigClass || !virQEMUCloseCallbacksClass)
return -1;
else
return 0;
}
VIR_ONCE_GLOBAL_INIT(virQEMUConfig)
struct _qemuDriverCloseDef {
virConnectPtr conn;
qemuDriverCloseCallback cb;
};
static void
qemuDriverLock(virQEMUDriverPtr driver)
{
@ -639,44 +658,57 @@ virCapsPtr virQEMUDriverGetCapabilities(virQEMUDriverPtr driver,
static void
qemuDriverCloseCallbackFree(void *payload,
const void *name ATTRIBUTE_UNUSED)
virQEMUCloseCallbacksFreeData(void *payload,
const void *name ATTRIBUTE_UNUSED)
{
VIR_FREE(payload);
}
int
qemuDriverCloseCallbackInit(virQEMUDriverPtr driver)
virQEMUCloseCallbacksPtr
virQEMUCloseCallbacksNew(void)
{
driver->closeCallbacks = virHashCreate(5, qemuDriverCloseCallbackFree);
if (!driver->closeCallbacks)
return -1;
virQEMUCloseCallbacksPtr closeCallbacks;
return 0;
if (virQEMUConfigInitialize() < 0)
return NULL;
if (!(closeCallbacks = virObjectLockableNew(virQEMUCloseCallbacksClass)))
return NULL;
closeCallbacks->list = virHashCreate(5, virQEMUCloseCallbacksFreeData);
if (!closeCallbacks->list) {
virObjectUnref(closeCallbacks);
return NULL;
}
return closeCallbacks;
}
void
qemuDriverCloseCallbackShutdown(virQEMUDriverPtr driver)
static void
virQEMUCloseCallbacksDispose(void *obj)
{
virHashFree(driver->closeCallbacks);
virQEMUCloseCallbacksPtr closeCallbacks = obj;
virHashFree(closeCallbacks->list);
}
int
qemuDriverCloseCallbackSet(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virConnectPtr conn,
qemuDriverCloseCallback cb)
virQEMUCloseCallbacksSet(virQEMUCloseCallbacksPtr closeCallbacks,
virDomainObjPtr vm,
virConnectPtr conn,
virQEMUCloseCallback cb)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
qemuDriverCloseDefPtr closeDef;
int ret = -1;
qemuDriverLock(driver);
virUUIDFormat(vm->def->uuid, uuidstr);
VIR_DEBUG("vm=%s, uuid=%s, conn=%p, cb=%p",
vm->def->name, uuidstr, conn, cb);
closeDef = virHashLookup(driver->closeCallbacks, uuidstr);
virObjectLock(closeCallbacks);
closeDef = virHashLookup(closeCallbacks->list, uuidstr);
if (closeDef) {
if (closeDef->conn != conn) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@ -701,7 +733,7 @@ qemuDriverCloseCallbackSet(virQEMUDriverPtr driver,
closeDef->conn = conn;
closeDef->cb = cb;
if (virHashAddEntry(driver->closeCallbacks, uuidstr, closeDef) < 0) {
if (virHashAddEntry(closeCallbacks->list, uuidstr, closeDef) < 0) {
VIR_FREE(closeDef);
goto cleanup;
}
@ -709,25 +741,26 @@ qemuDriverCloseCallbackSet(virQEMUDriverPtr driver,
ret = 0;
cleanup:
qemuDriverUnlock(driver);
virObjectUnlock(closeCallbacks);
return ret;
}
int
qemuDriverCloseCallbackUnset(virQEMUDriverPtr driver,
virDomainObjPtr vm,
qemuDriverCloseCallback cb)
virQEMUCloseCallbacksUnset(virQEMUCloseCallbacksPtr closeCallbacks,
virDomainObjPtr vm,
virQEMUCloseCallback cb)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
qemuDriverCloseDefPtr closeDef;
int ret = -1;
qemuDriverLock(driver);
virUUIDFormat(vm->def->uuid, uuidstr);
VIR_DEBUG("vm=%s, uuid=%s, cb=%p",
vm->def->name, uuidstr, cb);
closeDef = virHashLookup(driver->closeCallbacks, uuidstr);
virObjectLock(closeCallbacks);
closeDef = virHashLookup(closeCallbacks->list, uuidstr);
if (!closeDef)
goto cleanup;
@ -738,46 +771,49 @@ qemuDriverCloseCallbackUnset(virQEMUDriverPtr driver,
goto cleanup;
}
ret = virHashRemoveEntry(driver->closeCallbacks, uuidstr);
ret = virHashRemoveEntry(closeCallbacks->list, uuidstr);
cleanup:
qemuDriverUnlock(driver);
virObjectUnlock(closeCallbacks);
return ret;
}
qemuDriverCloseCallback
qemuDriverCloseCallbackGet(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virConnectPtr conn)
virQEMUCloseCallback
virQEMUCloseCallbacksGet(virQEMUCloseCallbacksPtr closeCallbacks,
virDomainObjPtr vm,
virConnectPtr conn)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
qemuDriverCloseDefPtr closeDef;
qemuDriverCloseCallback cb = NULL;
virQEMUCloseCallback cb = NULL;
qemuDriverLock(driver);
virUUIDFormat(vm->def->uuid, uuidstr);
VIR_DEBUG("vm=%s, uuid=%s, conn=%p",
vm->def->name, uuidstr, conn);
closeDef = virHashLookup(driver->closeCallbacks, uuidstr);
virObjectLock(closeCallbacks);
closeDef = virHashLookup(closeCallbacks->list, uuidstr);
if (closeDef && (!conn || closeDef->conn == conn))
cb = closeDef->cb;
virObjectUnlock(closeCallbacks);
VIR_DEBUG("cb=%p", cb);
qemuDriverUnlock(driver);
return cb;
}
struct qemuDriverCloseCallbackData {
struct virQEMUCloseCallbacksData {
virHashTablePtr list;
virQEMUDriverPtr driver;
virConnectPtr conn;
};
static void
qemuDriverCloseCallbackRun(void *payload,
const void *name,
void *opaque)
virQEMUCloseCallbacksRunOne(void *payload,
const void *name,
void *opaque)
{
struct qemuDriverCloseCallbackData *data = opaque;
struct virQEMUCloseCallbacksData *data = opaque;
qemuDriverCloseDefPtr closeDef = payload;
unsigned char uuid[VIR_UUID_BUFLEN];
char uuidstr[VIR_UUID_STRING_BUFLEN];
@ -808,20 +844,26 @@ qemuDriverCloseCallbackRun(void *payload,
if (dom)
virObjectUnlock(dom);
virHashRemoveEntry(data->driver->closeCallbacks, uuidstr);
virHashRemoveEntry(data->list, uuidstr);
}
void
qemuDriverCloseCallbackRunAll(virQEMUDriverPtr driver,
virConnectPtr conn)
virQEMUCloseCallbacksRun(virQEMUCloseCallbacksPtr closeCallbacks,
virConnectPtr conn,
virQEMUDriverPtr driver)
{
struct qemuDriverCloseCallbackData data = {
driver, conn
struct virQEMUCloseCallbacksData data = {
.driver = driver,
.conn = conn
};
VIR_DEBUG("conn=%p", conn);
qemuDriverLock(driver);
virHashForEach(driver->closeCallbacks, qemuDriverCloseCallbackRun, &data);
qemuDriverUnlock(driver);
virObjectLock(closeCallbacks);
data.list = closeCallbacks->list;
virHashForEach(data.list, virQEMUCloseCallbacksRunOne, &data);
virObjectUnlock(closeCallbacks);
}
struct _qemuSharedDiskEntry {

View File

@ -47,8 +47,8 @@
# define QEMUD_CPUMASK_LEN CPU_SETSIZE
typedef struct _qemuDriverCloseDef qemuDriverCloseDef;
typedef qemuDriverCloseDef *qemuDriverCloseDefPtr;
typedef struct _virQEMUCloseCallbacks virQEMUCloseCallbacks;
typedef virQEMUCloseCallbacks *virQEMUCloseCallbacksPtr;
typedef struct _virQEMUDriver virQEMUDriver;
typedef virQEMUDriver *virQEMUDriverPtr;
@ -216,8 +216,8 @@ struct _virQEMUDriver {
/* Immutable pointer. lockless access */
virLockManagerPluginPtr lockManager;
/* Immutable pointer. Unsafe APIs. XXX */
virHashTablePtr closeCallbacks;
/* Immutable pointer, self-clocking APIs */
virQEMUCloseCallbacksPtr closeCallbacks;
};
typedef struct _qemuDomainCmdlineDef qemuDomainCmdlineDef;
@ -254,23 +254,24 @@ struct qemuDomainDiskInfo {
int io_status;
};
typedef virDomainObjPtr (*qemuDriverCloseCallback)(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virConnectPtr conn);
int qemuDriverCloseCallbackInit(virQEMUDriverPtr driver);
void qemuDriverCloseCallbackShutdown(virQEMUDriverPtr driver);
int qemuDriverCloseCallbackSet(virQEMUDriverPtr driver,
typedef virDomainObjPtr (*virQEMUCloseCallback)(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virConnectPtr conn);
virQEMUCloseCallbacksPtr virQEMUCloseCallbacksNew(void);
int virQEMUCloseCallbacksSet(virQEMUCloseCallbacksPtr closeCallbacks,
virDomainObjPtr vm,
virConnectPtr conn,
virQEMUCloseCallback cb);
int virQEMUCloseCallbacksUnset(virQEMUCloseCallbacksPtr closeCallbacks,
virDomainObjPtr vm,
virConnectPtr conn,
qemuDriverCloseCallback cb);
int qemuDriverCloseCallbackUnset(virQEMUDriverPtr driver,
virDomainObjPtr vm,
qemuDriverCloseCallback cb);
qemuDriverCloseCallback qemuDriverCloseCallbackGet(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virConnectPtr conn);
void qemuDriverCloseCallbackRunAll(virQEMUDriverPtr driver,
virConnectPtr conn);
virQEMUCloseCallback cb);
virQEMUCloseCallback
virQEMUCloseCallbacksGet(virQEMUCloseCallbacksPtr closeCallbacks,
virDomainObjPtr vm,
virConnectPtr conn);
void virQEMUCloseCallbacksRun(virQEMUCloseCallbacksPtr closeCallbacks,
virConnectPtr conn,
virQEMUDriverPtr driver);
typedef struct _qemuSharedDiskEntry qemuSharedDiskEntry;
typedef qemuSharedDiskEntry *qemuSharedDiskEntryPtr;

View File

@ -757,7 +757,7 @@ qemuStartup(bool privileged,
cfg->hugepagePath = mempath;
}
if (qemuDriverCloseCallbackInit(qemu_driver) < 0)
if (!(qemu_driver->closeCallbacks = virQEMUCloseCallbacksNew()))
goto error;
/* Get all the running persistent or transient configs first */
@ -955,7 +955,7 @@ qemuShutdown(void) {
virSysinfoDefFree(qemu_driver->hostsysinfo);
qemuDriverCloseCallbackShutdown(qemu_driver);
virObjectUnref(qemu_driver->closeCallbacks);
VIR_FREE(qemu_driver->qemuImgBinary);
@ -1049,11 +1049,12 @@ cleanup:
return ret;
}
static int qemuClose(virConnectPtr conn) {
static int qemuClose(virConnectPtr conn)
{
virQEMUDriverPtr driver = conn->privateData;
/* Get rid of callbacks registered for this conn */
qemuDriverCloseCallbackRunAll(driver, conn);
virQEMUCloseCallbacksRun(driver->closeCallbacks, conn, driver);
conn->privateData = NULL;
@ -9702,8 +9703,8 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
* This prevents any other APIs being invoked while migration is taking
* place.
*/
if (qemuDriverCloseCallbackSet(driver, vm, domain->conn,
qemuMigrationCleanup) < 0)
if (virQEMUCloseCallbacksSet(driver->closeCallbacks, vm, domain->conn,
qemuMigrationCleanup) < 0)
goto endjob;
if (qemuMigrationJobContinue(vm) == 0) {
vm = NULL;
@ -9929,7 +9930,8 @@ qemuDomainMigrateConfirm3(virDomainPtr domain,
phase = QEMU_MIGRATION_PHASE_CONFIRM3;
qemuMigrationJobStartPhase(driver, vm, phase);
qemuDriverCloseCallbackUnset(driver, vm, qemuMigrationCleanup);
virQEMUCloseCallbacksUnset(driver->closeCallbacks, vm,
qemuMigrationCleanup);
ret = qemuMigrationConfirm(driver, domain->conn, vm,
cookiein, cookieinlen,

View File

@ -3155,7 +3155,8 @@ qemuMigrationPerformPhase(virQEMUDriverPtr driver,
}
qemuMigrationJobStartPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM3);
qemuDriverCloseCallbackUnset(driver, vm, qemuMigrationCleanup);
virQEMUCloseCallbacksUnset(driver->closeCallbacks, vm,
qemuMigrationCleanup);
resume = virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING;
ret = doNativeMigrate(driver, vm, uri, cookiein, cookieinlen,
@ -3186,8 +3187,8 @@ qemuMigrationPerformPhase(virQEMUDriverPtr driver,
qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE);
if (qemuDriverCloseCallbackSet(driver, vm, conn,
qemuMigrationCleanup) < 0)
if (virQEMUCloseCallbacksSet(driver->closeCallbacks, vm, conn,
qemuMigrationCleanup) < 0)
goto endjob;
endjob:

View File

@ -4626,22 +4626,23 @@ int qemuProcessAutoDestroyAdd(virQEMUDriverPtr driver,
virConnectPtr conn)
{
VIR_DEBUG("vm=%s, conn=%p", vm->def->name, conn);
return qemuDriverCloseCallbackSet(driver, vm, conn,
qemuProcessAutoDestroy);
return virQEMUCloseCallbacksSet(driver->closeCallbacks, vm, conn,
qemuProcessAutoDestroy);
}
int qemuProcessAutoDestroyRemove(virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
VIR_DEBUG("vm=%s", vm->def->name);
return qemuDriverCloseCallbackUnset(driver, vm, qemuProcessAutoDestroy);
return virQEMUCloseCallbacksUnset(driver->closeCallbacks, vm,
qemuProcessAutoDestroy);
}
bool qemuProcessAutoDestroyActive(virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
qemuDriverCloseCallback cb;
virQEMUCloseCallback cb;
VIR_DEBUG("vm=%s", vm->def->name);
cb = qemuDriverCloseCallbackGet(driver, vm, NULL);
cb = virQEMUCloseCallbacksGet(driver->closeCallbacks, vm, NULL);
return cb == qemuProcessAutoDestroy;
}