[libvirt] [PATCH V3 2/2] enhance processWatchdogEvent()
This patch do the following two things:
1. hold an extra reference while handling watchdog event
If the domain is not persistent, and qemu quits unexpectedly before
calling processWatchdogEvent(), vm will be freed and the function
processWatchdogEvent() will be dangerous.
2. unlock qemu driver and vm before returning from processWatchdogEvent()
When the function processWatchdogEvent() failed, we only free wdEvent,
but forget to unlock qemu driver and vm, free dumpfile.
---
src/qemu/qemu_driver.c | 34 ++++++++++++++++++++++------------
src/qemu/qemu_process.c | 4 ++++
2 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d5c1274..f6e503a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2442,6 +2442,9 @@ static void processWatchdogEvent(void *data, void *opaque)
struct qemuDomainWatchdogEvent *wdEvent = data;
struct qemud_driver *driver = opaque;
+ qemuDriverLock(driver);
+ virDomainObjLock(wdEvent->vm);
+
switch (wdEvent->action) {
case VIR_DOMAIN_WATCHDOG_ACTION_DUMP:
{
@@ -2452,19 +2455,19 @@ static void processWatchdogEvent(void *data, void *opaque)
wdEvent->vm->def->name,
(unsigned int)time(NULL)) < 0) {
virReportOOMError();
- break;
+ goto unlock;
}
- qemuDriverLock(driver);
- virDomainObjLock(wdEvent->vm);
-
- if (qemuDomainObjBeginJobWithDriver(driver, wdEvent->vm) < 0)
- break;
+ if (qemuDomainObjBeginJobWithDriver(driver, wdEvent->vm) < 0) {
+ VIR_FREE(dumpfile);
+ goto unlock;
+ }
if (!virDomainObjIsActive(wdEvent->vm)) {
qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
- break;
+ VIR_FREE(dumpfile);
+ goto endjob;
}
ret = doCoreDump(driver,
@@ -2481,16 +2484,23 @@ static void processWatchdogEvent(void *data, void *opaque)
qemuReportError(VIR_ERR_OPERATION_FAILED,
"%s", _("Resuming after dump
failed"));
- if (qemuDomainObjEndJob(wdEvent->vm) > 0)
- virDomainObjUnlock(wdEvent->vm);
-
- qemuDriverUnlock(driver);
-
VIR_FREE(dumpfile);
}
break;
+ default:
+ goto unlock;
}
+endjob:
+ /* Safe to ignore value since ref count was incremented in
+ * qemuProcessHandleWatchdog().
+ */
+ ignore_value(qemuDomainObjEndJob(wdEvent->vm));
+
+unlock:
+ if (virDomainObjUnref(wdEvent->vm) > 0)
+ virDomainObjUnlock(wdEvent->vm);
+ qemuDriverUnlock(driver);
VIR_FREE(wdEvent);
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7295f9e..b6bec46 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -428,6 +428,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