[libvirt] [PATCH] Move most of qemuProcessKill into virProcessKillPainfully
Daniel P. Berrange
berrange at redhat.com
Wed Sep 26 14:44:39 UTC 2012
From: "Daniel P. Berrange" <berrange at redhat.com>
In the cgroups APIs we have a virCgroupKillPainfully function
which does the loop sending SIGTERM, then SIGKILL and waiting
for the process to exit. There is similar functionality for
simple processes in qemuProcessKill, but it is tangled with
the QEMU code. Untangle it to provide a virProcessKillPainfuly
function
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 8 ++---
src/qemu/qemu_process.c | 79 ++++++++----------------------------------------
src/util/virprocess.c | 57 ++++++++++++++++++++++++++++++++++
src/util/virprocess.h | 2 ++
5 files changed, 76 insertions(+), 71 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4635a4d..dab607a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1708,6 +1708,7 @@ virPidFileDeletePath;
# virprocess.h
virProcessAbort;
virProcessKill;
+virProcessKillPainfully;
virProcessTranslateStatus;
virProcessWait;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7ac53ac..22fef7a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2006,13 +2006,11 @@ qemuDomainDestroyFlags(virDomainPtr dom,
* it now means the job will be released
*/
if (flags & VIR_DOMAIN_DESTROY_GRACEFUL) {
- if (qemuProcessKill(driver, vm, 0) < 0) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("failed to kill qemu process with SIGTERM"));
+ if (qemuProcessKill(driver, vm, 0) < 0)
goto cleanup;
- }
} else {
- ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE));
+ if (qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE) < 0)
+ goto cleanup;
}
/* We need to prevent monitor EOF callback from doing our work (and sending
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3cd30af..70b72af 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3877,9 +3877,7 @@ int
qemuProcessKill(struct qemud_driver *driver,
virDomainObjPtr vm, unsigned int flags)
{
- int i, ret = -1;
- const char *signame = "TERM";
- bool driver_unlocked = false;
+ int ret;
VIR_DEBUG("vm=%s pid=%d flags=%x",
vm->def->name, vm->pid, flags);
@@ -3891,78 +3889,27 @@ qemuProcessKill(struct qemud_driver *driver,
}
}
- /* This loop sends SIGTERM (or SIGKILL if flags has
- * VIR_QEMU_PROCESS_KILL_FORCE and VIR_QEMU_PROCESS_KILL_NOWAIT),
- * then waits a few iterations (10 seconds) to see if it dies. If
- * the qemu process still hasn't exited, and
- * VIR_QEMU_PROCESS_KILL_FORCE is requested, a SIGKILL will then
- * be sent, and qemuProcessKill will wait up to 5 seconds more for
- * the process to exit before returning. Note that the FORCE mode
- * could 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 < 75; i++) {
- int signum;
- if (i == 0) {
- if ((flags & VIR_QEMU_PROCESS_KILL_FORCE) &&
- (flags & VIR_QEMU_PROCESS_KILL_NOWAIT)) {
- signum = SIGKILL; /* kill it immediately */
- signame="KILL";
- } else {
- signum = SIGTERM; /* kindly suggest it should exit */
- }
- } else if ((i == 50) & (flags & VIR_QEMU_PROCESS_KILL_FORCE)) {
- VIR_WARN("Timed out waiting after SIG%s to process %d, "
- "sending SIGKILL", signame, vm->pid);
- signum = SIGKILL; /* kill it after a grace period */
- signame="KILL";
- } else {
- signum = 0; /* Just check for existence */
- }
-
- if (virProcessKill(vm->pid, signum) < 0) {
- if (errno != ESRCH) {
- char ebuf[1024];
- VIR_WARN("Failed to terminate process %d with SIG%s: %s",
- vm->pid, signame,
- virStrerror(errno, ebuf, sizeof(ebuf)));
- goto cleanup;
- }
- ret = 0;
- goto cleanup; /* process is dead */
- }
+ if ((flags & VIR_QEMU_PROCESS_KILL_NOWAIT)) {
+ virProcessKill(vm->pid,
+ (flags & VIR_QEMU_PROCESS_KILL_FORCE) ?
+ SIGKILL : SIGTERM);
+ return 0;
+ }
- if (i == 0 && (flags & VIR_QEMU_PROCESS_KILL_NOWAIT)) {
- ret = 0;
- goto cleanup;
- }
+ if (driver)
+ qemuDriverUnlock(driver);
- if (driver && !driver_unlocked) {
- /* THREADS.txt says we can't hold the driver lock while sleeping */
- qemuDriverUnlock(driver);
- driver_unlocked = true;
- }
+ ret = virProcessKillPainfully(vm->pid,
+ !!(flags & VIR_QEMU_PROCESS_KILL_FORCE));
- usleep(200 * 1000);
- }
- VIR_WARN("Timed out waiting after SIG%s to process %d", signame, vm->pid);
-cleanup:
- if (driver_unlocked) {
- /* We had unlocked the driver, so re-lock it. THREADS.txt says
- * we can't have the domain locked when locking the driver, so
- * we must first unlock the domain. BUT, before we can unlock
- * the domain, we need to add a ref to it in case there aren't
- * any active jobs (analysis of all callers didn't reveal such
- * a case, but there are too many to maintain certainty, so we
- * will do this as a precaution).
- */
+ if (driver) {
virObjectRef(vm);
virDomainObjUnlock(vm);
qemuDriverLock(driver);
virDomainObjLock(vm);
virObjectUnref(vm);
}
+
return ret;
}
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 958f5f7..c70aa58 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -235,3 +235,60 @@ int virProcessKill(pid_t pid, int sig)
return kill(pid, sig);
#endif
}
+
+
+/*
+ * Try to kill the process and verify it has exited
+ *
+ * Returns 0 if it was killed gracefully, 1 if it
+ * was killed forcably, -1 if it is still alive,
+ * or another error occurred.
+ */
+int
+virProcessKillPainfully(pid_t pid, bool force)
+{
+ int i, ret = -1;
+ const char *signame = "TERM";
+
+ VIR_DEBUG("vpid=%d force=%d", pid, force);
+
+ /* This loop sends SIGTERM, then waits a few iterations (10 seconds)
+ * to see if it dies. If the process still hasn't exited, and
+ * @force is requested, a SIGKILL will be sent, and this will
+ * wait upto 5 seconds more for the process to exit before
+ * returning.
+ *
+ * Note that setting @force could result in dataloss for the process.
+ */
+ for (i = 0 ; i < 75; i++) {
+ int signum;
+ if (i == 0) {
+ signum = SIGTERM; /* kindly suggest it should exit */
+ } else if ((i == 50) & force) {
+ VIR_DEBUG("Timed out waiting after SIGTERM to process %d, "
+ "sending SIGKILL", pid);
+ signum = SIGKILL; /* kill it after a grace period */
+ signame = "KILL";
+ } else {
+ signum = 0; /* Just check for existence */
+ }
+
+ if (virProcessKill(pid, signum) < 0) {
+ if (errno != ESRCH) {
+ virReportSystemError(errno,
+ _("Failed to terminate process %d with SIG%s"),
+ pid, signame);
+ goto cleanup;
+ }
+ ret = signum == SIGTERM ? 0 : 1;
+ goto cleanup; /* process is dead */
+ }
+
+ usleep(200 * 1000);
+ }
+
+ VIR_DEBUG("Timed out waiting after SIGKILL to process %d", pid);
+
+cleanup:
+ return ret;
+}
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 048a73c..d537d92 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -38,5 +38,7 @@ virProcessWait(pid_t pid, int *exitstatus)
int virProcessKill(pid_t pid, int sig);
+int virProcessKillPainfully(pid_t pid, bool force);
+
#endif /* __VIR_PROCESS_H__ */
--
1.7.11.2
More information about the libvir-list
mailing list