At 03/30/2011 02:05 PM, Hu Tao Write:
On Wed, Mar 30, 2011 at 12:34:49PM +0800, Wen Congyang wrote:
If the domain is not persistent, and qemu quited unexpectedly before calling processWatchdogEvent(), vm will be freed and the function processWatchdogEvent() will be dangerous.
--- src/qemu/qemu_driver.c | 10 ++++++---- src/qemu/qemu_process.c | 4 ++++ 2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d79d61b..c9c681f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2443,15 +2443,17 @@ static void processWatchdogEvent(void *data, void *opaque) }
endjob: - if (qemuDomainObjEndJob(wdEvent->vm) == 0) - wdEvent->vm = NULL; + /* Safe to ignore value since ref count was incremented in + * qemuProcessHandleWatchdog(). + */ + ignore_value(qemuDomainObjEndJob(wdEvent->vm));
unlock: - if (wdEvent->vm) - virDomainObjUnlock(wdEvent->vm); qemuDriverUnlock(driver);
cleanup: + if (virDomainObjUnref(wdEvent->vm) > 0) + virDomainObjUnlock(wdEvent->vm);
These two lines should be protected by qemu driver lock.
We should not unlock domain here as we reached here without locking domain. Will update this patch.
VIR_FREE(wdEvent); }
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e31e1b4..cd8c726 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -426,6 +426,10 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon ATTRIBUTE_UNUSED, if (VIR_ALLOC(wdEvent) == 0) { wdEvent->action = VIR_DOMAIN_WATCHDOG_ACTION_DUMP; wdEvent->vm = vm; + /* Hold an extra reference because we can't allow 'vm' to be + * deleted before handling watchdog event is finished. + */ + virDomainObjRef(vm); ignore_value(virThreadPoolSendJob(driver->workerPool, wdEvent)); } else virReportOOMError(); -- 1.7.1