On 04/06/2011 01:58 AM, Wen Congyang wrote:
If the domain is not persistent, and qemu quited unexpectedly before
s/quited/quits/
calling processWatchdogEvent(), vm will be freed and the function processWatchdogEvent() will be dangerous.
--- src/qemu/qemu_driver.c | 12 ++++++++---- src/qemu/qemu_process.c | 4 ++++ 2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 628cfe3..d02891c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2436,15 +2436,19 @@ static void processWatchdogEvent(void *data, void *opaque) } break; default: - goto cleanup; + qemuDriverLock(driver); + virDomainObjLock(wdEvent->vm); + goto unlock; }
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) + if (virDomainObjUnref(wdEvent->vm) > 0) virDomainObjUnlock(wdEvent->vm); qemuDriverUnlock(driver);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 48ecd5c..321374c 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));
ACK to the need to do this. Hmm, maybe we should just squash 2 and 3 together into one patch. Also, I found a flaw in 2 while reviewing this one. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org