From 4c500f16965dd27e888d08048b075d7094a9f890 Mon Sep 17 00:00:00 2001 From: Stefan Reiter Date: Mon, 19 Oct 2020 14:18:37 +0200 Subject: [PATCH] qmeventd: add last-ditch effort SIGKILL cleanup 'alarm' is used to schedule an additionaly cleanup round 5 seconds after sending SIGTERM via terminate_client. This then sends SIGKILL via a pidfd (if supported by the kernel) or directly via kill, making sure that the QEMU process is *really* dead and won't be left behind in an undetermined state. This shouldn't be an issue under normal circumstances, but can help avoid dead processes lying around if QEMU hangs after SIGTERM. Signed-off-by: Stefan Reiter --- qmeventd/qmeventd.c | 87 ++++++++++++++++++++++++++++++++++++++++++++- qmeventd/qmeventd.h | 26 ++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/qmeventd/qmeventd.c b/qmeventd/qmeventd.c index 6b02a062..57f18673 100644 --- a/qmeventd/qmeventd.c +++ b/qmeventd/qmeventd.c @@ -63,6 +63,8 @@ static int verbose = 0; static int epoll_fd = 0; static const char *progname; GHashTable *vm_clients; // key=vmid (freed on remove), value=*Client (free manually) +GSList *forced_cleanups; +volatile sig_atomic_t alarm_triggered = 0; /* * Helper functions @@ -468,8 +470,39 @@ terminate_client(struct Client *client) client->state = STATE_TERMINATING; + // open a pidfd before kill for later cleanup + int pidfd = pidfd_open(client->pid, 0); + if (pidfd < 0) { + switch (errno) { + case ESRCH: + // process already dead for some reason, cleanup done + VERBOSE_PRINT("%s: failed to open pidfd, process already dead (pid %d)\n", + client->qemu.vmid, client->pid); + return; + + // otherwise fall back to just using the PID directly, but don't + // print if we only failed because we're running on an older kernel + case ENOSYS: + break; + default: + perror("failed to open QEMU pidfd for cleanup"); + break; + } + } + int err = kill(client->pid, SIGTERM); log_neg(err, "kill"); + + struct CleanupData *data_ptr = malloc(sizeof(struct CleanupData)); + struct CleanupData data = { + .pid = client->pid, + .pidfd = pidfd + }; + *data_ptr = data; + forced_cleanups = g_slist_prepend(forced_cleanups, (void *)data_ptr); + + // resets any other alarms, but will fire eventually and cleanup all + alarm(5); } void @@ -545,6 +578,55 @@ handle_client(struct Client *client) } +/* + * SIGALRM and cleanup handling + * + * terminate_client will set an alarm for 5 seconds and add its client's PID to + * the forced_cleanups list - when the timer expires, we iterate the list and + * attempt to issue SIGKILL to all processes which haven't yet stopped. + */ + +static void +alarm_handler(__attribute__((unused)) int signum) +{ + alarm_triggered = 1; +} + +static void +sigkill(void *ptr, __attribute__((unused)) void *unused) +{ + struct CleanupData data = *((struct CleanupData *)ptr); + int err; + + if (data.pidfd > 0) { + err = pidfd_send_signal(data.pidfd, SIGKILL, NULL, 0); + } else { + err = kill(data.pid, SIGKILL); + } + + if (err < 0) { + if (errno != ESRCH) { + fprintf(stderr, "SIGKILL cleanup of pid '%d' failed - %s\n", + data.pid, strerror(errno)); + } + } else { + fprintf(stderr, "cleanup failed, terminating pid '%d' with SIGKILL\n", + data.pid); + } +} + +static void +handle_forced_cleanup() +{ + if (alarm_triggered) { + alarm_triggered = 0; + g_slist_foreach(forced_cleanups, sigkill, NULL); + g_slist_free_full(forced_cleanups, free); + forced_cleanups = NULL; + } +} + + int main(int argc, char *argv[]) { @@ -577,6 +659,7 @@ main(int argc, char *argv[]) } signal(SIGCHLD, SIG_IGN); + signal(SIGALRM, alarm_handler); socket_path = argv[optind]; @@ -612,7 +695,7 @@ main(int argc, char *argv[]) for(;;) { nevents = epoll_wait(epoll_fd, events, 1, -1); if (nevents < 0 && errno == EINTR) { - // signal happened, try again + handle_forced_cleanup(); continue; } bail_neg(nevents, "epoll_wait"); @@ -630,5 +713,7 @@ main(int argc, char *argv[]) handle_client((struct Client *)events[n].data.ptr); } } + + handle_forced_cleanup(); } } diff --git a/qmeventd/qmeventd.h b/qmeventd/qmeventd.h index 30aea988..1921ef35 100644 --- a/qmeventd/qmeventd.h +++ b/qmeventd/qmeventd.h @@ -21,6 +21,15 @@ Author: Dominik Csapak */ +#include + +#ifndef __NR_pidfd_open +#define __NR_pidfd_open 434 +#endif +#ifndef __NR_pidfd_send_signal +#define __NR_pidfd_send_signal 424 +#endif + #define VERBOSE_PRINT(...) do { if (verbose) { printf(__VA_ARGS__); } } while (0) static inline void log_neg(int errval, const char *msg) @@ -38,6 +47,18 @@ static inline void bail_neg(int errval, const char *msg) } } +static inline int +pidfd_open(pid_t pid, unsigned int flags) +{ + return syscall(__NR_pidfd_open, pid, flags); +} + +static inline int +pidfd_send_signal(int pidfd, int sig, siginfo_t *info, unsigned int flags) +{ + return syscall(__NR_pidfd_send_signal, pidfd, sig, info, flags); +} + typedef enum { CLIENT_NONE, CLIENT_QEMU, @@ -77,6 +98,11 @@ struct Client { } vzdump; }; +struct CleanupData { + pid_t pid; + int pidfd; +}; + void handle_qmp_handshake(struct Client *client); void handle_qmp_event(struct Client *client, struct json_object *obj); void handle_qmp_return(struct Client *client, struct json_object *data, bool error);