[libvirt] [PATCH] Fix virDomainObj ref handling in QEMU driver
Daniel P. Berrange
berrange at redhat.com
Tue Dec 8 14:46:28 UTC 2009
Since the monitor I/O is processed out of band from the main
thread(s) invoking monitor commands, the virDomainObj may be
deleted by the I/O thread. The qemuDomainObjBeginJob takes an
extra reference to protect against final deletion, but this
reference is released by the corresponding EndJob call. THus
after the EndJob call it may not be valid to reference the
virDomainObj any more. To allow callers to detect this, the
EndJob call is changed to return the remaining reference count.
* src/conf/domain_conf.c: Make virDomainObjUnref return the
remaining reference count
* src/qemu/qemu_driver.c: Avoid referencing virDomainObjPtr
after qemuDomainObjEndJob if it has been deleted.
---
src/conf/domain_conf.c | 6 +-
src/qemu/qemu_driver.c | 133 +++++++++++++++++++++++++++++-------------------
2 files changed, 84 insertions(+), 55 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 21d509d..dca2e49 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -231,7 +231,7 @@ static void virDomainObjListDeallocator(void *payload, const char *name ATTRIBUT
{
virDomainObjPtr obj = payload;
virDomainObjLock(obj);
- if (!virDomainObjUnref(obj))
+ if (virDomainObjUnref(obj) > 0)
virDomainObjUnlock(obj);
}
@@ -637,9 +637,9 @@ int virDomainObjUnref(virDomainObjPtr dom)
if (dom->refs == 0) {
virDomainObjUnlock(dom);
virDomainObjFree(dom);
- return 1;
+ return 0;
}
- return 0;
+ return dom->refs;
}
static virDomainObjPtr virDomainObjNew(virConnectPtr conn,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7dfd1e6..7e60d0e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -371,15 +371,18 @@ static int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver,
*
* To be called after completing the work associated with the
* earlier qemuDomainBeginJob() call
+ *
+ * Returns remaining refcount on 'obj', maybe 0 to indicated it
+ * was deleted
*/
-static void qemuDomainObjEndJob(virDomainObjPtr obj)
+static int ATTRIBUTE_RETURN_CHECK qemuDomainObjEndJob(virDomainObjPtr obj)
{
qemuDomainObjPrivatePtr priv = obj->privateData;
priv->jobActive = 0;
virCondSignal(&priv->jobCond);
- virDomainObjUnref(obj);
+ return virDomainObjUnref(obj);
}
@@ -3039,9 +3042,9 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml,
goto cleanup; /* XXXX free the 'vm' we created ? */
if (qemudStartVMDaemon(conn, driver, vm, NULL, -1) < 0) {
- qemuDomainObjEndJob(vm);
- virDomainRemoveInactive(&driver->domains,
- vm);
+ if (qemuDomainObjEndJob(vm) > 0)
+ virDomainRemoveInactive(&driver->domains,
+ vm);
vm = NULL;
goto endjob;
}
@@ -3054,8 +3057,9 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml,
if (dom) dom->id = vm->def->id;
endjob:
- if (vm)
- qemuDomainObjEndJob(vm);
+ if (vm &&
+ qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
virDomainDefFree(def);
@@ -3110,7 +3114,8 @@ static int qemudDomainSuspend(virDomainPtr dom) {
ret = 0;
endjob:
- qemuDomainObjEndJob(vm);
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
if (vm)
@@ -3169,7 +3174,8 @@ static int qemudDomainResume(virDomainPtr dom) {
ret = 0;
endjob:
- qemuDomainObjEndJob(vm);
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
if (vm)
@@ -3213,7 +3219,8 @@ static int qemudDomainShutdown(virDomainPtr dom) {
qemuDomainObjExitMonitor(vm);
endjob:
- qemuDomainObjEndJob(vm);
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
if (vm)
@@ -3252,16 +3259,17 @@ static int qemudDomainDestroy(virDomainPtr dom) {
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
if (!vm->persistent) {
- qemuDomainObjEndJob(vm);
- virDomainRemoveInactive(&driver->domains,
- vm);
+ if (qemuDomainObjEndJob(vm) > 0)
+ virDomainRemoveInactive(&driver->domains,
+ vm);
vm = NULL;
}
ret = 0;
endjob:
- if (vm)
- qemuDomainObjEndJob(vm);
+ if (vm &&
+ qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
if (vm)
@@ -3402,7 +3410,8 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) {
ret = 0;
endjob:
- qemuDomainObjEndJob(vm);
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
if (vm)
@@ -3452,7 +3461,8 @@ static int qemudDomainGetInfo(virDomainPtr dom,
err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
qemuDomainObjExitMonitor(vm);
if (err < 0) {
- qemuDomainObjEndJob(vm);
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
goto cleanup;
}
@@ -3462,7 +3472,10 @@ static int qemudDomainGetInfo(virDomainPtr dom,
else
info->memory = balloon;
- qemuDomainObjEndJob(vm);
+ if (qemuDomainObjEndJob(vm) == 0) {
+ vm = NULL;
+ goto cleanup;
+ }
} else {
info->memory = vm->def->memory;
}
@@ -3671,15 +3684,16 @@ static int qemudDomainSave(virDomainPtr dom,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_SAVED);
if (!vm->persistent) {
- qemuDomainObjEndJob(vm);
- virDomainRemoveInactive(&driver->domains,
- vm);
+ if (qemuDomainObjEndJob(vm) > 0)
+ virDomainRemoveInactive(&driver->domains,
+ vm);
vm = NULL;
}
endjob:
- if (vm)
- qemuDomainObjEndJob(vm);
+ if (vm &&
+ qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
if (fd != -1)
@@ -3766,7 +3780,8 @@ static int qemudDomainCoreDump(virDomainPtr dom,
}
endjob:
- qemuDomainObjEndJob(vm);
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
if (vm)
@@ -3827,7 +3842,8 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) {
ret = 0;
endjob:
- qemuDomainObjEndJob(vm);
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
if (vm)
@@ -4235,9 +4251,9 @@ static int qemudDomainRestore(virConnectPtr conn,
fd = -1;
if (ret < 0) {
if (!vm->persistent) {
- qemuDomainObjEndJob(vm);
- virDomainRemoveInactive(&driver->domains,
- vm);
+ if (qemuDomainObjEndJob(vm) > 0)
+ virDomainRemoveInactive(&driver->domains,
+ vm);
vm = NULL;
}
goto endjob;
@@ -4265,8 +4281,9 @@ static int qemudDomainRestore(virConnectPtr conn,
ret = 0;
endjob:
- if (vm)
- qemuDomainObjEndJob(vm);
+ if (vm &&
+ qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
virDomainDefFree(def);
@@ -4314,7 +4331,10 @@ static char *qemudDomainDumpXML(virDomainPtr dom,
qemuDomainObjEnterMonitor(vm);
err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
qemuDomainObjExitMonitor(vm);
- qemuDomainObjEndJob(vm);
+ if (qemuDomainObjEndJob(vm) == 0) {
+ vm = NULL;
+ goto cleanup;
+ }
if (err < 0)
goto cleanup;
if (err > 0)
@@ -4547,7 +4567,8 @@ static int qemudDomainStart(virDomainPtr dom) {
VIR_DOMAIN_EVENT_STARTED_BOOTED);
endjob:
- qemuDomainObjEndJob(vm);
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
if (vm)
@@ -5338,7 +5359,8 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
ret = -1;
endjob:
- qemuDomainObjEndJob(vm);
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
if (cgroup)
@@ -5672,7 +5694,8 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
ret = -1;
endjob:
- qemuDomainObjEndJob(vm);
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
virDomainDeviceDefFree(dev);
@@ -5996,7 +6019,8 @@ qemudDomainBlockStats (virDomainPtr dom,
qemuDomainObjExitMonitor(vm);
endjob:
- qemuDomainObjEndJob(vm);
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
VIR_FREE(qemu_dev_name);
@@ -6214,7 +6238,8 @@ qemudDomainMemoryPeek (virDomainPtr dom,
ret = 0;
endjob:
- qemuDomainObjEndJob(vm);
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
VIR_FREE(tmp);
@@ -6708,8 +6733,8 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn,
if (qemust == NULL) {
qemudShutdownVMDaemon(NULL, driver, vm);
if (!vm->persistent) {
- qemuDomainObjEndJob(vm);
- virDomainRemoveInactive(&driver->domains, vm);
+ if (qemuDomainObjEndJob(vm) > 0)
+ virDomainRemoveInactive(&driver->domains, vm);
vm = NULL;
}
virReportSystemError(dconn, errno,
@@ -6727,8 +6752,9 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn,
ret = 0;
endjob:
- if (vm)
- qemuDomainObjEndJob(vm);
+ if (vm &&
+ qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
virDomainDefFree(def);
@@ -6895,8 +6921,8 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
* should have already done that.
*/
if (!vm->persistent) {
- qemuDomainObjEndJob(vm);
- virDomainRemoveInactive(&driver->domains, vm);
+ if (qemuDomainObjEndJob(vm) > 0)
+ virDomainRemoveInactive(&driver->domains, vm);
vm = NULL;
}
goto endjob;
@@ -6908,8 +6934,9 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
ret = 0;
endjob:
- if (vm)
- qemuDomainObjEndJob(vm);
+ if (vm &&
+ qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
virDomainDefFree(def);
@@ -7399,8 +7426,8 @@ qemudDomainMigratePerform (virDomainPtr dom,
VIR_DOMAIN_EVENT_STOPPED_MIGRATED);
if (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE)) {
virDomainDeleteConfig(dom->conn, driver->configDir, driver->autostartDir, vm);
- qemuDomainObjEndJob(vm);
- virDomainRemoveInactive(&driver->domains, vm);
+ if (qemuDomainObjEndJob(vm) > 0)
+ virDomainRemoveInactive(&driver->domains, vm);
vm = NULL;
}
ret = 0;
@@ -7424,8 +7451,9 @@ endjob:
VIR_DOMAIN_EVENT_RESUMED,
VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
}
- if (vm)
- qemuDomainObjEndJob(vm);
+ if (vm &&
+ qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
if (vm)
@@ -7523,15 +7551,16 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_FAILED);
if (!vm->persistent) {
- qemuDomainObjEndJob(vm);
- virDomainRemoveInactive(&driver->domains, vm);
+ if (qemuDomainObjEndJob(vm) > 0)
+ virDomainRemoveInactive(&driver->domains, vm);
vm = NULL;
}
}
endjob:
- if (vm)
- qemuDomainObjEndJob(vm);
+ if (vm &&
+ qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
cleanup:
if (vm)
--
1.6.5.2
More information about the libvir-list
mailing list