When libvirt is shutting down the qemu process, it first sends SIGTERM, then waits for 1.6 seconds and, if it sees the process still there, sends a SIGKILL. There have been reports that this behavior can lead to data loss because the guest running in qemu doesn't have time to flush it's disk cache buffers before it's unceremoniously whacked. One suggestion on how to solve that problem was to remove SIGKILL from the normal virDomainDestroyFlags, but still provide the ability to kill qemu with SIGKILL by using a new flag to virDomainDestroyFlags. This patch is a quick attempt at that in order to start a conversation on the topic. So what are your opinions? Is this the right way to solve the problem? Any better ideas? --- include/libvirt/libvirt.h.in | 12 ++++++------ src/qemu/qemu_driver.c | 7 +++++-- src/qemu/qemu_process.c | 40 ++++++++++++++++++++++------------------ src/qemu/qemu_process.h | 8 +++++++- 4 files changed, 40 insertions(+), 27 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e99cd00..f9ac865 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1182,12 +1182,6 @@ virConnectPtr virDomainGetConnect (virDomainPtr domain); * Domain creation and destruction */ - -/* - * typedef enum { - * } virDomainDestroyFlagsValues; - */ - virDomainPtr virDomainCreateXML (virConnectPtr conn, const char *xmlDesc, unsigned int flags); @@ -1222,6 +1216,12 @@ int virDomainReset (virDomainPtr domain, unsigned int flags); int virDomainDestroy (virDomainPtr domain); + +typedef enum { + VIR_DOMAIN_DESTROY_NONE = 0, /* Default behavior */ + VIR_DOMAIN_DESTROY_IMMEDIATE = 1 << 0, /* immediately kill, could lead to data loss!! */ +} virDomainDestroyFlagsValues; + int virDomainDestroyFlags (virDomainPtr domain, unsigned int flags); int virDomainRef (virDomainPtr domain); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ab69dca..0e80241 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1754,7 +1754,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, virDomainEventPtr event = NULL; qemuDomainObjPrivatePtr priv; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_DESTROY_IMMEDIATE, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1775,7 +1775,10 @@ qemuDomainDestroyFlags(virDomainPtr dom, * can kill the process even if a job is active. Killing * it now means the job will be released */ - qemuProcessKill(vm, false); + if (flags & VIR_DOMAIN_DESTROY_IMMEDIATE) + qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_IMMEDIATELY); + else + qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCEFULLY); /* We need to prevent monitor EOF callback from doing our work (and sending * misleading events) while the vm is unlocked inside BeginJob API diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d22020b..efbe46c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -586,7 +586,7 @@ endjob: cleanup: if (vm) { if (ret == -1) - qemuProcessKill(vm, false); + qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCEFULLY); if (virDomainObjUnref(vm) > 0) virDomainObjUnlock(vm); } @@ -611,12 +611,12 @@ qemuProcessShutdownOrReboot(struct qemud_driver *driver, qemuProcessFakeReboot, vm) < 0) { VIR_ERROR(_("Failed to create reboot thread, killing domain")); - qemuProcessKill(vm, true); + qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_GRACEFULLY); /* Safe to ignore value since ref count was incremented above */ ignore_value(virDomainObjUnref(vm)); } } else { - qemuProcessKill(vm, true); + qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_GRACEFULLY); } } @@ -3524,30 +3524,34 @@ cleanup: } -void qemuProcessKill(virDomainObjPtr vm, bool gracefully) +void qemuProcessKill(virDomainObjPtr vm, unsigned int killMode) { int i; - VIR_DEBUG("vm=%s pid=%d gracefully=%d", - vm->def->name, vm->pid, gracefully); + VIR_DEBUG("vm=%s pid=%d killMode=%d", + vm->def->name, vm->pid, killMode); if (!virDomainObjIsActive(vm)) { VIR_DEBUG("VM '%s' not active", vm->def->name); return; } - /* This loop sends SIGTERM, then waits a few iterations - * (1.6 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. + /* This loop sends SIGTERM (or SIGKILL if killMode is + * VIR_QEMU_KILL_MODE_IMMEDIATELY), then waits a few iterations + * (3.2 seconds) to see if it dies. Note that the IMMEDIATELY mode + * will very likely result in lost data in the guest, so it should + * only be used if the guest is hung and can't be destroyed in any + * other manner. */ - for (i = 0 ; i < 15 ; i++) { + for (i = 0 ; i < 16 ; i++) { int signum; - if (i == 0) - signum = SIGTERM; - else if (i == 8) - signum = SIGKILL; - else + if (i == 0) { + if (killMode == VIR_QEMU_PROCESS_KILL_IMMEDIATELY) + signum = SIGKILL; + else + signum = SIGTERM; + } else { signum = 0; /* Just check for existence */ + } if (virKillProcess(vm->pid, signum) < 0) { if (errno != ESRCH) { @@ -3558,7 +3562,7 @@ void qemuProcessKill(virDomainObjPtr vm, bool gracefully) break; } - if (i == 0 && gracefully) + if (i == 0 && (killMode == VIR_QEMU_PROCESS_KILL_GRACEFULLY)) break; usleep(200 * 1000); @@ -3651,7 +3655,7 @@ void qemuProcessStop(struct qemud_driver *driver, } /* shut it off for sure */ - qemuProcessKill(vm, false); + qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCEFULLY); /* Stop autodestroy in case guest is restarted */ qemuProcessAutoDestroyRemove(driver, vm); diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 062d79e..755d4a6 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -68,7 +68,13 @@ int qemuProcessAttach(virConnectPtr conn, virDomainChrSourceDefPtr monConfig, bool monJSON); -void qemuProcessKill(virDomainObjPtr vm, bool gracefully); +typedef enum { + VIR_QEMU_PROCESS_KILL_GRACEFULLY = 0, + VIR_QEMU_PROCESS_KILL_FORCEFULLY = 1, + VIR_QEMU_PROCESS_KILL_IMMEDIATELY = 2, +} virQemuProcessKillMode; + +void qemuProcessKill(virDomainObjPtr vm, unsigned int killMode); int qemuProcessAutoDestroyInit(struct qemud_driver *driver); void qemuProcessAutoDestroyRun(struct qemud_driver *driver, -- 1.7.7.5