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