enhance processWatchdogEvent()

This patch does 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.

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d5c1274e7efefcbbd42752f89ce52d8c7be25f52..f6e503ab9d0601b8cd6e4f91bfb309f9f2e41118 100644 (file)
--- 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 7295f9ee14fea1b2c0c73a9df7e1b35f565b8562..d405ddae4e93bcd3c51f37468cec9d0b8f0c211f 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -428,7 +428,14 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
         if (VIR_ALLOC(wdEvent) == 0) {
             wdEvent->action = VIR_DOMAIN_WATCHDOG_ACTION_DUMP;
             wdEvent->vm = vm;
-            ignore_value(virThreadPoolSendJob(driver->workerPool, wdEvent));
+            /* Hold an extra reference because we can't allow 'vm' to be
+             * deleted before handling watchdog event is finished.
+             */
+            virDomainObjRef(vm);
+            if (virThreadPoolSendJob(driver->workerPool, wdEvent) < 0) {
+                virDomainObjUnref(vm);
+                VIR_FREE(wdEvent);
+            }
         } else
             virReportOOMError();
     }