External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-24 22:55:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix virDomainObj ref handling in QEMU driver

Browse Source 
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.
This commit is contained in:
Daniel P. Berrange 2009-12-08 14:42:43 +00:00
parent 604c70fd5d
commit 8e7d14953c
2 changed files with 84 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/conf/domain_conf.c
View File

@ -231,7 +231,7 @@ static void virDomainObjListDeallocator(void *payload, const char *name ATTRIBUT
{ {
virDomainObjPtr obj = payload; virDomainObjPtr obj = payload;
virDomainObjLock(obj); virDomainObjLock(obj);
if (!virDomainObjUnref(obj)) if (virDomainObjUnref(obj) > 0)
virDomainObjUnlock(obj); virDomainObjUnlock(obj);
} }
@ -637,9 +637,9 @@ int virDomainObjUnref(virDomainObjPtr dom)
if (dom->refs == 0) { if (dom->refs == 0) {
virDomainObjUnlock(dom); virDomainObjUnlock(dom);
virDomainObjFree(dom); virDomainObjFree(dom);
return 1;
}
return 0; return 0;
}
return dom->refs;
} }
static virDomainObjPtr virDomainObjNew(virConnectPtr conn, static virDomainObjPtr virDomainObjNew(virConnectPtr conn,

109
src/qemu/qemu_driver.c
View File

@ -371,15 +371,18 @@ static int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver,
* *
* To be called after completing the work associated with the * To be called after completing the work associated with the
* earlier qemuDomainBeginJob() call * 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; qemuDomainObjPrivatePtr priv = obj->privateData;
priv->jobActive = 0; priv->jobActive = 0;
virCondSignal(&priv->jobCond); virCondSignal(&priv->jobCond);
virDomainObjUnref(obj); return virDomainObjUnref(obj);
} }
@ -3039,7 +3042,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml,
goto cleanup; /* XXXX free the 'vm' we created ? */ goto cleanup; /* XXXX free the 'vm' we created ? */
if (qemudStartVMDaemon(conn, driver, vm, NULL, -1) < 0) { if (qemudStartVMDaemon(conn, driver, vm, NULL, -1) < 0) {
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) > 0)
virDomainRemoveInactive(&driver->domains, virDomainRemoveInactive(&driver->domains,
vm); vm);
vm = NULL; vm = NULL;
@ -3054,8 +3057,9 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml,
if (dom) dom->id = vm->def->id; if (dom) dom->id = vm->def->id;
endjob: endjob:
if (vm) if (vm &&
qemuDomainObjEndJob(vm); qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
virDomainDefFree(def); virDomainDefFree(def);
@ -3110,7 +3114,8 @@ static int qemudDomainSuspend(virDomainPtr dom) {
ret = 0; ret = 0;
endjob: endjob:
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
if (vm) if (vm)
@ -3169,7 +3174,8 @@ static int qemudDomainResume(virDomainPtr dom) {
ret = 0; ret = 0;
endjob: endjob:
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
if (vm) if (vm)
@ -3213,7 +3219,8 @@ static int qemudDomainShutdown(virDomainPtr dom) {
qemuDomainObjExitMonitor(vm); qemuDomainObjExitMonitor(vm);
endjob: endjob:
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
if (vm) if (vm)
@ -3252,7 +3259,7 @@ static int qemudDomainDestroy(virDomainPtr dom) {
VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_DESTROYED); VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
if (!vm->persistent) { if (!vm->persistent) {
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) > 0)
virDomainRemoveInactive(&driver->domains, virDomainRemoveInactive(&driver->domains,
vm); vm);
vm = NULL; vm = NULL;
@ -3260,8 +3267,9 @@ static int qemudDomainDestroy(virDomainPtr dom) {
ret = 0; ret = 0;
endjob: endjob:
if (vm) if (vm &&
qemuDomainObjEndJob(vm); qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
if (vm) if (vm)
@ -3402,7 +3410,8 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) {
ret = 0; ret = 0;
endjob: endjob:
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
if (vm) if (vm)
@ -3452,7 +3461,8 @@ static int qemudDomainGetInfo(virDomainPtr dom,
err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
qemuDomainObjExitMonitor(vm); qemuDomainObjExitMonitor(vm);
if (err < 0) { if (err < 0) {
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
goto cleanup; goto cleanup;
} }
@ -3462,7 +3472,10 @@ static int qemudDomainGetInfo(virDomainPtr dom,
else else
info->memory = balloon; info->memory = balloon;
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) == 0) {
vm = NULL;
goto cleanup;
}
} else { } else {
info->memory = vm->def->memory; info->memory = vm->def->memory;
} }
@ -3671,15 +3684,16 @@ static int qemudDomainSave(virDomainPtr dom,
VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_SAVED); VIR_DOMAIN_EVENT_STOPPED_SAVED);
if (!vm->persistent) { if (!vm->persistent) {
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) > 0)
virDomainRemoveInactive(&driver->domains, virDomainRemoveInactive(&driver->domains,
vm); vm);
vm = NULL; vm = NULL;
} }
endjob: endjob:
if (vm) if (vm &&
qemuDomainObjEndJob(vm); qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
if (fd != -1) if (fd != -1)
@ -3766,7 +3780,8 @@ static int qemudDomainCoreDump(virDomainPtr dom,
} }
endjob: endjob:
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
if (vm) if (vm)
@ -3827,7 +3842,8 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) {
ret = 0; ret = 0;
endjob: endjob:
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
if (vm) if (vm)
@ -4235,7 +4251,7 @@ static int qemudDomainRestore(virConnectPtr conn,
fd = -1; fd = -1;
if (ret < 0) { if (ret < 0) {
if (!vm->persistent) { if (!vm->persistent) {
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) > 0)
virDomainRemoveInactive(&driver->domains, virDomainRemoveInactive(&driver->domains,
vm); vm);
vm = NULL; vm = NULL;
@ -4265,8 +4281,9 @@ static int qemudDomainRestore(virConnectPtr conn,
ret = 0; ret = 0;
endjob: endjob:
if (vm) if (vm &&
qemuDomainObjEndJob(vm); qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
virDomainDefFree(def); virDomainDefFree(def);
@ -4314,7 +4331,10 @@ static char *qemudDomainDumpXML(virDomainPtr dom,
qemuDomainObjEnterMonitor(vm); qemuDomainObjEnterMonitor(vm);
err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
qemuDomainObjExitMonitor(vm); qemuDomainObjExitMonitor(vm);
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) == 0) {
vm = NULL;
goto cleanup;
}
if (err < 0) if (err < 0)
goto cleanup; goto cleanup;
if (err > 0) if (err > 0)
@ -4547,7 +4567,8 @@ static int qemudDomainStart(virDomainPtr dom) {
VIR_DOMAIN_EVENT_STARTED_BOOTED); VIR_DOMAIN_EVENT_STARTED_BOOTED);
endjob: endjob:
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
if (vm) if (vm)
@ -5338,7 +5359,8 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
ret = -1; ret = -1;
endjob: endjob:
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
if (cgroup) if (cgroup)
@ -5672,7 +5694,8 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
ret = -1; ret = -1;
endjob: endjob:
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
virDomainDeviceDefFree(dev); virDomainDeviceDefFree(dev);
@ -5996,7 +6019,8 @@ qemudDomainBlockStats (virDomainPtr dom,
qemuDomainObjExitMonitor(vm); qemuDomainObjExitMonitor(vm);
endjob: endjob:
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
VIR_FREE(qemu_dev_name); VIR_FREE(qemu_dev_name);
@ -6214,7 +6238,8 @@ qemudDomainMemoryPeek (virDomainPtr dom,
ret = 0; ret = 0;
endjob: endjob:
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
VIR_FREE(tmp); VIR_FREE(tmp);
@ -6708,7 +6733,7 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn,
if (qemust == NULL) { if (qemust == NULL) {
qemudShutdownVMDaemon(NULL, driver, vm); qemudShutdownVMDaemon(NULL, driver, vm);
if (!vm->persistent) { if (!vm->persistent) {
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) > 0)
virDomainRemoveInactive(&driver->domains, vm); virDomainRemoveInactive(&driver->domains, vm);
vm = NULL; vm = NULL;
} }
@ -6727,8 +6752,9 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn,
ret = 0; ret = 0;
endjob: endjob:
if (vm) if (vm &&
qemuDomainObjEndJob(vm); qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
virDomainDefFree(def); virDomainDefFree(def);
@ -6895,7 +6921,7 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
* should have already done that. * should have already done that.
*/ */
if (!vm->persistent) { if (!vm->persistent) {
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) > 0)
virDomainRemoveInactive(&driver->domains, vm); virDomainRemoveInactive(&driver->domains, vm);
vm = NULL; vm = NULL;
} }
@ -6908,8 +6934,9 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
ret = 0; ret = 0;
endjob: endjob:
if (vm) if (vm &&
qemuDomainObjEndJob(vm); qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
virDomainDefFree(def); virDomainDefFree(def);
@ -7399,7 +7426,7 @@ qemudDomainMigratePerform (virDomainPtr dom,
VIR_DOMAIN_EVENT_STOPPED_MIGRATED); VIR_DOMAIN_EVENT_STOPPED_MIGRATED);
if (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE)) { if (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE)) {
virDomainDeleteConfig(dom->conn, driver->configDir, driver->autostartDir, vm); virDomainDeleteConfig(dom->conn, driver->configDir, driver->autostartDir, vm);
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) > 0)
virDomainRemoveInactive(&driver->domains, vm); virDomainRemoveInactive(&driver->domains, vm);
vm = NULL; vm = NULL;
} }
@ -7424,8 +7451,9 @@ endjob:
VIR_DOMAIN_EVENT_RESUMED, VIR_DOMAIN_EVENT_RESUMED,
VIR_DOMAIN_EVENT_RESUMED_MIGRATED); VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
} }
if (vm) if (vm &&
qemuDomainObjEndJob(vm); qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
if (vm) if (vm)
@ -7523,15 +7551,16 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_FAILED); VIR_DOMAIN_EVENT_STOPPED_FAILED);
if (!vm->persistent) { if (!vm->persistent) {
qemuDomainObjEndJob(vm); if (qemuDomainObjEndJob(vm) > 0)
virDomainRemoveInactive(&driver->domains, vm); virDomainRemoveInactive(&driver->domains, vm);
vm = NULL; vm = NULL;
} }
} }
endjob: endjob:
if (vm) if (vm &&
qemuDomainObjEndJob(vm); qemuDomainObjEndJob(vm) == 0)
vm = NULL;
cleanup: cleanup:
if (vm) if (vm)