[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] qemu: Fix shutdown regression



The commit that prevents disk corruption on domain shutdown
(96fc4784177ecb70357518fa863442455e45ad0e) causes regression with QEMU
0.14.* and 0.15.* because of a regression bug in QEMU that was fixed
only recently in QEMU git. With affected QEMU binaries, domains cannot
be shutdown properly and stay in a paused state. This patch tries to
avoid this by sending SIGKILL to 0.1[45].* QEMU processes. Though we
wait a bit more between sending SIGTERM and SIGKILL to reduce the
possibility of virtual disk corruption.
---
 src/qemu/qemu_capabilities.c |    7 +++++++
 src/qemu/qemu_capabilities.h |    1 +
 src/qemu/qemu_process.c      |   19 +++++++++++++------
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 36f47a9..823c500 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -136,6 +136,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
               "pci-ohci",
               "usb-redir",
               "usb-hub",
+              "no-shutdown-bug",
     );
 
 struct qemu_feature_flags {
@@ -1049,6 +1050,12 @@ qemuCapsComputeCmdFlags(const char *help,
 
     if (version >= 13000)
         qemuCapsSet(flags, QEMU_CAPS_PCI_MULTIFUNCTION);
+
+    /* QEMU version 0.14.* and 0.15.* are known not to handle SIGTERM
+     * properly when started with -no-shutdown
+     */
+    if (version >= 14000 && version <= 15999)
+        qemuCapsSet(flags, QEMU_CAPS_NO_SHUTDOWN_BUG);
 }
 
 /* We parse the output of 'qemu -help' to get the QEMU
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 96b7a3b..53d5ace 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -110,6 +110,7 @@ enum qemuCapsFlags {
     QEMU_CAPS_PCI_OHCI          = 71, /* -device pci-ohci */
     QEMU_CAPS_USB_REDIR         = 72, /* -device usb-redir */
     QEMU_CAPS_USB_HUB           = 73, /* -device usb-hub */
+    QEMU_CAPS_NO_SHUTDOWN_BUG   = 74, /* -no-shutdown doesn't exit on SIGTERM */
 
     QEMU_CAPS_LAST,                   /* this must always be the last item */
 };
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3baaa19..3311699 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -434,6 +434,7 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
                           virDomainObjPtr vm)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    bool gracefully = true;
     VIR_DEBUG("vm=%p", vm);
 
     virDomainObjLock(vm);
@@ -443,6 +444,12 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
         goto cleanup;
     }
 
+    if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN_BUG)) {
+        VIR_DEBUG("Emulator is likely affected by -no-shutdown bug;"
+                  " we will not avoid sending SIGKILL to it");
+        gracefully = false;
+    }
+
     priv->gotShutdown = true;
     if (priv->fakeReboot) {
         virDomainObjRef(vm);
@@ -452,12 +459,12 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
                             qemuProcessFakeReboot,
                             vm) < 0) {
             VIR_ERROR(_("Failed to create reboot thread, killing domain"));
-            qemuProcessKill(vm, true);
+            qemuProcessKill(vm, gracefully);
             if (virDomainObjUnref(vm) == 0)
                 vm = NULL;
         }
     } else {
-        qemuProcessKill(vm, true);
+        qemuProcessKill(vm, gracefully);
     }
 
 cleanup:
@@ -3227,15 +3234,15 @@ void qemuProcessKill(virDomainObjPtr vm, bool gracefully)
     }
 
     /* This loop sends SIGTERM, then waits a few iterations
-     * (1.6 seconds) to see if it dies. If still alive then
+     * (3 seconds) to see if it dies. If still alive then
      * it does SIGKILL, and waits a few more iterations (1.6
      * seconds more) to confirm that it has really gone.
      */
-    for (i = 0 ; i < 15 ; i++) {
+    for (i = 0 ; i < 23 ; i++) {
         int signum;
         if (i == 0)
             signum = SIGTERM;
-        else if (i == 8)
+        else if (i == 15)
             signum = SIGKILL;
         else
             signum = 0; /* Just check for existence */
@@ -3249,7 +3256,7 @@ void qemuProcessKill(virDomainObjPtr vm, bool gracefully)
             break;
         }
 
-        if (i == 0 && gracefully)
+        if (gracefully)
             break;
 
         usleep(200 * 1000);
-- 
1.7.6.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]