| Message ID | 20220809064024.15259-1-imbrenda@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/1] os-posix: asynchronous teardown for shutdown on Linux | expand |
Hi, Claudio. On 8/9/22 03:40, Claudio Imbrenda wrote: > This patch adds support for asynchronously tearing down a VM on Linux. > > When qemu terminates, either naturally or because of a fatal signal, > the VM is torn down. If the VM is huge, it can take a considerable > amount of time for it to be cleaned up. In case of a protected VM, it > might take even longer than a non-protected VM (this is the case on > s390x, for example). > > Some users might want to shut down a VM and restart it immediately, > without having to wait. This is especially true if management > infrastructure like libvirt is used. > > This patch implements a simple trick on Linux to allow qemu to return > immediately, with the teardown of the VM being performed > asynchronously. > > If the new commandline option -async-teardown is used, a new process is > spawned from qemu at startup, using the clone syscall, in such way that > it will share its address space with qemu. > > The new process will have the name "cleanup/<QEMU_PID>". It will wait > until qemu terminates, and then it will exit itself. > > This allows qemu to terminate quickly, without having to wait for the > whole address space to be torn down. The teardown process will exit > after qemu, so it will be the last user of the address space, and > therefore it will take care of the actual teardown. > > The teardown process will share the same cgroups as qemu, so both > memory usage and cpu time will be accounted properly. > > This feature can already be used with libvirt by adding the following > to the XML domain definition to pass the parameter to qemu directly: > > <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0"> > <arg value='-async-teardown'/> > </commandline> > > More advanced interfaces like pidfd or close_range have intentionally > been avoided in order to be more compatible with older kernels. > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> I've smoke-tested this on ppc and everything looks fine. For what's worth: Reviewed-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> Tested-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> Have you measured the benefits of using -async-teardown vs. not using it? If so, can you please share the details so I can give it a try on ppc, too? The wall-clock perception is that nothing has changed, for better or worse. My tests used mid-sized VMs, like 128 vCPUs, 64GB RAM. Cheers! > --- > include/qemu/async-teardown.h | 22 ++++++ > os-posix.c | 6 ++ > qemu-options.hx | 17 +++++ > util/async-teardown.c | 123 ++++++++++++++++++++++++++++++++++ > util/meson.build | 1 + > 5 files changed, 169 insertions(+) > create mode 100644 include/qemu/async-teardown.h > create mode 100644 util/async-teardown.c > > diff --git a/include/qemu/async-teardown.h b/include/qemu/async-teardown.h > new file mode 100644 > index 0000000000..092e7a37e7 > --- /dev/null > +++ b/include/qemu/async-teardown.h > @@ -0,0 +1,22 @@ > +/* > + * Asynchronous teardown > + * > + * Copyright IBM, Corp. 2022 > + * > + * Authors: > + * Claudio Imbrenda <imbrenda@linux.ibm.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or (at your > + * option) any later version. See the COPYING file in the top-level directory. > + * > + */ > +#ifndef QEMU_ASYNC_TEARDOWN_H > +#define QEMU_ASYNC_TEARDOWN_H > + > +#include "config-host.h" > + > +#ifdef CONFIG_LINUX > +void init_async_teardown(void); > +#endif > + > +#endif > diff --git a/os-posix.c b/os-posix.c > index 321fc4bd13..4858650c3e 100644 > --- a/os-posix.c > +++ b/os-posix.c > @@ -39,6 +39,7 @@ > > #ifdef CONFIG_LINUX > #include <sys/prctl.h> > +#include "qemu/async-teardown.h" > #endif > > /* > @@ -150,6 +151,11 @@ int os_parse_cmd_args(int index, const char *optarg) > case QEMU_OPTION_daemonize: > daemonize = 1; > break; > +#if defined(CONFIG_LINUX) > + case QEMU_OPTION_asyncteardown: > + init_async_teardown(); > + break; > +#endif > default: > return -1; > } > diff --git a/qemu-options.hx b/qemu-options.hx > index 3f23a42fa8..d434353159 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -4743,6 +4743,23 @@ HXCOMM Internal use > DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL) > DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL) > > +#ifdef __linux__ > +DEF("async-teardown", 0, QEMU_OPTION_asyncteardown, > + "-async-teardown enable asynchronous teardown\n", > + QEMU_ARCH_ALL) > +#endif > +SRST > +``-async-teardown`` > + Enable asynchronous teardown. A new teardown process will be > + created at startup, using clone. The teardown process will share > + the address space of the main qemu process, and wait for the main > + process to terminate. At that point, the teardown process will > + also exit. This allows qemu to terminate quickly if the guest was > + huge, leaving the teardown of the address space to the teardown > + process. Since the teardown process shares the same cgroups as the > + main qemu process, accounting is performed correctly. > +ERST > + > DEF("msg", HAS_ARG, QEMU_OPTION_msg, > "-msg [timestamp[=on|off]][,guest-name=[on|off]]\n" > " control error message format\n" > diff --git a/util/async-teardown.c b/util/async-teardown.c > new file mode 100644 > index 0000000000..07fe549891 > --- /dev/null > +++ b/util/async-teardown.c > @@ -0,0 +1,123 @@ > +/* > + * Asynchronous teardown > + * > + * Copyright IBM, Corp. 2022 > + * > + * Authors: > + * Claudio Imbrenda <imbrenda@linux.ibm.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or (at your > + * option) any later version. See the COPYING file in the top-level directory. > + * > + */ > +#include <stdlib.h> > +#include <stdio.h> > +#include <sys/types.h> > +#include <sys/unistd.h> > +#include <dirent.h> > +#include <sys/prctl.h> > +#include <signal.h> > +#include <sched.h> > + > +#include "qemu/async-teardown.h" > + > +static pid_t the_ppid; > + > +/* > + * Close all open file descriptors. > + */ > +static void close_all_open_fd(void) > +{ > + struct dirent *de; > + int fd, dfd; > + DIR *dir; > + > + dir = opendir("/proc/self/fd"); > + if (!dir) { > + return; > + } > + /* Avoid closing the directory. */ > + dfd = dirfd(dir); > + > + for (de = readdir(dir); de; de = readdir(dir)) { > + fd = atoi(de->d_name); > + if (fd != dfd) { > + close(fd); > + } > + } > + closedir(dir); > +} > + > +static void hup_handler(int signal) > +{ > + /* Check every second if this process has been reparented. */ > + while (the_ppid == getppid()) { > + /* sleep() is safe to use in a signal handler. */ > + sleep(1); > + } > + > + /* At this point the parent process has terminated completely. */ > + _exit(0); > +} > + > +static int async_teardown_fn(void *arg) > +{ > + struct sigaction sa = { .sa_handler = hup_handler }; > + sigset_t hup_signal; > + char name[16]; > + > + /* Set a meaningful name for this process. */ > + snprintf(name, 16, "cleanup/%d", the_ppid); > + prctl(PR_SET_NAME, (unsigned long)name); > + > + /* > + * Close all file descriptors that might have been inherited from the > + * main qemu process when doing clone, needed to make libvirt happy. > + * Not using close_range for increased compatibility with older kernels. > + */ > + close_all_open_fd(); > + > + /* Set up a handler for SIGHUP and unblock SIGHUP. */ > + sigaction(SIGHUP, &sa, NULL); > + sigemptyset(&hup_signal); > + sigaddset(&hup_signal, SIGHUP); > + sigprocmask(SIG_UNBLOCK, &hup_signal, NULL); > + > + /* Ask to receive SIGHUP when the parent dies. */ > + prctl(PR_SET_PDEATHSIG, SIGHUP); > + > + /* > + * Sleep forever, unless the parent process has already terminated. The > + * only interruption can come from the SIGHUP signal, which in normal > + * operation is received when the parent process dies. > + */ > + if (the_ppid == getppid()) { > + pause(); > + } > + > + /* At this point the parent process has terminated completely. */ > + _exit(0); > +} > + > +/* > + * Block all signals, start (clone) a new process sharing the address space > + * with qemu (CLONE_VM), then restore signals. > + */ > +void init_async_teardown(void) > +{ > + sigset_t all_signals, old_signals; > + const int stack_size = 8192; /* Should be more than enough */ > + char *stack, *stack_ptr; > + > + the_ppid = getpid(); > + stack = malloc(stack_size); > + if (!stack) { > + return; > + } > + stack_ptr = stack + stack_size; > + > + sigfillset(&all_signals); > + sigprocmask(SIG_BLOCK, &all_signals, &old_signals); > + clone(async_teardown_fn, stack_ptr, CLONE_VM, NULL, NULL, NULL, NULL); > + sigprocmask(SIG_SETMASK, &old_signals, NULL); > +} > diff --git a/util/meson.build b/util/meson.build > index 5e282130df..63acd59bb0 100644 > --- a/util/meson.build > +++ b/util/meson.build > @@ -2,6 +2,7 @@ util_ss.add(files('osdep.c', 'cutils.c', 'unicode.c', 'qemu-timer-common.c')) > if not config_host_data.get('CONFIG_ATOMIC64') > util_ss.add(files('atomic64.c')) > endif > +util_ss.add(when: 'CONFIG_LINUX', if_true: files('async-teardown.c')) > util_ss.add(when: 'CONFIG_POSIX', if_true: files('aio-posix.c')) > util_ss.add(when: 'CONFIG_POSIX', if_true: files('fdmon-poll.c')) > if config_host_data.get('CONFIG_EPOLL_CREATE1')
On Wed, 10 Aug 2022 17:30:41 -0300 Murilo Opsfelder Araújo <muriloo@linux.ibm.com> wrote: > Hi, Claudio. Hi Murilo, [...] > I've smoke-tested this on ppc and everything looks fine. > For what's worth: > > Reviewed-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> > Tested-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> thanks a lot for testing this! > > > Have you measured the benefits of using -async-teardown vs. not using it? > If so, can you please share the details so I can give it a try on ppc, too? > > The wall-clock perception is that nothing has changed, for better or worse. > My tests used mid-sized VMs, like 128 vCPUs, 64GB RAM. The number of CPUs doesn't really have any impact. 64G of RAM is quite small to notice a sizeable difference, although you should be able to see a few seconds of speedup in the shutdown. Also, starting a guest with a lot of RAM is not enough, you have to make sure that the guest ram is actually allocated (completely fill the ram in the guest before shutting it down) I just tested a 64G and a 256G guest on s390x. I measured the time between the last line in the console ("Reached target Power-Off.") and the moment when control comes back to the shell. The measurement was not exactly super accurate (I manually ran "date +%s" in another shell when I saw the last line in the console, and then again when I got the shell back from qemu). The 64G guest needs a few seconds, the 256G needs almost exactly 4 times as much. With the asynchronous teardown it's almost instant in both cases (less than 1s, too fast to measure manually). Try a multi-TB guest if you can (at the moment I can't) to see a more marked effect. Also consider that this is for _normal_ guests. Protected guests on s390x have an even slower teardown due to the way protected virtualization is implemented in the hardware. I hope this was helpful > > Cheers! > > > --- > > include/qemu/async-teardown.h | 22 ++++++ > > os-posix.c | 6 ++ > > qemu-options.hx | 17 +++++ > > util/async-teardown.c | 123 ++++++++++++++++++++++++++++++++++ > > util/meson.build | 1 + > > 5 files changed, 169 insertions(+) > > create mode 100644 include/qemu/async-teardown.h > > create mode 100644 util/async-teardown.c > > > > diff --git a/include/qemu/async-teardown.h b/include/qemu/async-teardown.h > > new file mode 100644 > > index 0000000000..092e7a37e7 > > --- /dev/null > > +++ b/include/qemu/async-teardown.h > > @@ -0,0 +1,22 @@ > > +/* > > + * Asynchronous teardown > > + * > > + * Copyright IBM, Corp. 2022 > > + * > > + * Authors: > > + * Claudio Imbrenda <imbrenda@linux.ibm.com> > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2 or (at your > > + * option) any later version. See the COPYING file in the top-level directory. > > + * > > + */ > > +#ifndef QEMU_ASYNC_TEARDOWN_H > > +#define QEMU_ASYNC_TEARDOWN_H > > + > > +#include "config-host.h" > > + > > +#ifdef CONFIG_LINUX > > +void init_async_teardown(void); > > +#endif > > + > > +#endif > > diff --git a/os-posix.c b/os-posix.c > > index 321fc4bd13..4858650c3e 100644 > > --- a/os-posix.c > > +++ b/os-posix.c > > @@ -39,6 +39,7 @@ > > > > #ifdef CONFIG_LINUX > > #include <sys/prctl.h> > > +#include "qemu/async-teardown.h" > > #endif > > > > /* > > @@ -150,6 +151,11 @@ int os_parse_cmd_args(int index, const char *optarg) > > case QEMU_OPTION_daemonize: > > daemonize = 1; > > break; > > +#if defined(CONFIG_LINUX) > > + case QEMU_OPTION_asyncteardown: > > + init_async_teardown(); > > + break; > > +#endif > > default: > > return -1; > > } > > diff --git a/qemu-options.hx b/qemu-options.hx > > index 3f23a42fa8..d434353159 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -4743,6 +4743,23 @@ HXCOMM Internal use > > DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL) > > DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL) > > > > +#ifdef __linux__ > > +DEF("async-teardown", 0, QEMU_OPTION_asyncteardown, > > + "-async-teardown enable asynchronous teardown\n", > > + QEMU_ARCH_ALL) > > +#endif > > +SRST > > +``-async-teardown`` > > + Enable asynchronous teardown. A new teardown process will be > > + created at startup, using clone. The teardown process will share > > + the address space of the main qemu process, and wait for the main > > + process to terminate. At that point, the teardown process will > > + also exit. This allows qemu to terminate quickly if the guest was > > + huge, leaving the teardown of the address space to the teardown > > + process. Since the teardown process shares the same cgroups as the > > + main qemu process, accounting is performed correctly. > > +ERST > > + > > DEF("msg", HAS_ARG, QEMU_OPTION_msg, > > "-msg [timestamp[=on|off]][,guest-name=[on|off]]\n" > > " control error message format\n" > > diff --git a/util/async-teardown.c b/util/async-teardown.c > > new file mode 100644 > > index 0000000000..07fe549891 > > --- /dev/null > > +++ b/util/async-teardown.c > > @@ -0,0 +1,123 @@ > > +/* > > + * Asynchronous teardown > > + * > > + * Copyright IBM, Corp. 2022 > > + * > > + * Authors: > > + * Claudio Imbrenda <imbrenda@linux.ibm.com> > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2 or (at your > > + * option) any later version. See the COPYING file in the top-level directory. > > + * > > + */ > > +#include <stdlib.h> > > +#include <stdio.h> > > +#include <sys/types.h> > > +#include <sys/unistd.h> > > +#include <dirent.h> > > +#include <sys/prctl.h> > > +#include <signal.h> > > +#include <sched.h> > > + > > +#include "qemu/async-teardown.h" > > + > > +static pid_t the_ppid; > > + > > +/* > > + * Close all open file descriptors. > > + */ > > +static void close_all_open_fd(void) > > +{ > > + struct dirent *de; > > + int fd, dfd; > > + DIR *dir; > > + > > + dir = opendir("/proc/self/fd"); > > + if (!dir) { > > + return; > > + } > > + /* Avoid closing the directory. */ > > + dfd = dirfd(dir); > > + > > + for (de = readdir(dir); de; de = readdir(dir)) { > > + fd = atoi(de->d_name); > > + if (fd != dfd) { > > + close(fd); > > + } > > + } > > + closedir(dir); > > +} > > + > > +static void hup_handler(int signal) > > +{ > > + /* Check every second if this process has been reparented. */ > > + while (the_ppid == getppid()) { > > + /* sleep() is safe to use in a signal handler. */ > > + sleep(1); > > + } > > + > > + /* At this point the parent process has terminated completely. */ > > + _exit(0); > > +} > > + > > +static int async_teardown_fn(void *arg) > > +{ > > + struct sigaction sa = { .sa_handler = hup_handler }; > > + sigset_t hup_signal; > > + char name[16]; > > + > > + /* Set a meaningful name for this process. */ > > + snprintf(name, 16, "cleanup/%d", the_ppid); > > + prctl(PR_SET_NAME, (unsigned long)name); > > + > > + /* > > + * Close all file descriptors that might have been inherited from the > > + * main qemu process when doing clone, needed to make libvirt happy. > > + * Not using close_range for increased compatibility with older kernels. > > + */ > > + close_all_open_fd(); > > + > > + /* Set up a handler for SIGHUP and unblock SIGHUP. */ > > + sigaction(SIGHUP, &sa, NULL); > > + sigemptyset(&hup_signal); > > + sigaddset(&hup_signal, SIGHUP); > > + sigprocmask(SIG_UNBLOCK, &hup_signal, NULL); > > + > > + /* Ask to receive SIGHUP when the parent dies. */ > > + prctl(PR_SET_PDEATHSIG, SIGHUP); > > + > > + /* > > + * Sleep forever, unless the parent process has already terminated. The > > + * only interruption can come from the SIGHUP signal, which in normal > > + * operation is received when the parent process dies. > > + */ > > + if (the_ppid == getppid()) { > > + pause(); > > + } > > + > > + /* At this point the parent process has terminated completely. */ > > + _exit(0); > > +} > > + > > +/* > > + * Block all signals, start (clone) a new process sharing the address space > > + * with qemu (CLONE_VM), then restore signals. > > + */ > > +void init_async_teardown(void) > > +{ > > + sigset_t all_signals, old_signals; > > + const int stack_size = 8192; /* Should be more than enough */ > > + char *stack, *stack_ptr; > > + > > + the_ppid = getpid(); > > + stack = malloc(stack_size); > > + if (!stack) { > > + return; > > + } > > + stack_ptr = stack + stack_size; > > + > > + sigfillset(&all_signals); > > + sigprocmask(SIG_BLOCK, &all_signals, &old_signals); > > + clone(async_teardown_fn, stack_ptr, CLONE_VM, NULL, NULL, NULL, NULL); > > + sigprocmask(SIG_SETMASK, &old_signals, NULL); > > +} > > diff --git a/util/meson.build b/util/meson.build > > index 5e282130df..63acd59bb0 100644 > > --- a/util/meson.build > > +++ b/util/meson.build > > @@ -2,6 +2,7 @@ util_ss.add(files('osdep.c', 'cutils.c', 'unicode.c', 'qemu-timer-common.c')) > > if not config_host_data.get('CONFIG_ATOMIC64') > > util_ss.add(files('atomic64.c')) > > endif > > +util_ss.add(when: 'CONFIG_LINUX', if_true: files('async-teardown.c')) > > util_ss.add(when: 'CONFIG_POSIX', if_true: files('aio-posix.c')) > > util_ss.add(when: 'CONFIG_POSIX', if_true: files('fdmon-poll.c')) > > if config_host_data.get('CONFIG_EPOLL_CREATE1') > >
On Tue, Aug 09, 2022 at 08:40:24AM +0200, Claudio Imbrenda wrote: > This patch adds support for asynchronously tearing down a VM on Linux. > > When qemu terminates, either naturally or because of a fatal signal, > the VM is torn down. If the VM is huge, it can take a considerable > amount of time for it to be cleaned up. In case of a protected VM, it > might take even longer than a non-protected VM (this is the case on > s390x, for example). > > Some users might want to shut down a VM and restart it immediately, > without having to wait. This is especially true if management > infrastructure like libvirt is used. > > This patch implements a simple trick on Linux to allow qemu to return > immediately, with the teardown of the VM being performed > asynchronously. > > If the new commandline option -async-teardown is used, a new process is > spawned from qemu at startup, using the clone syscall, in such way that > it will share its address space with qemu. > > The new process will have the name "cleanup/<QEMU_PID>". It will wait > until qemu terminates, and then it will exit itself. > > This allows qemu to terminate quickly, without having to wait for the > whole address space to be torn down. The teardown process will exit > after qemu, so it will be the last user of the address space, and > therefore it will take care of the actual teardown. > > The teardown process will share the same cgroups as qemu, so both > memory usage and cpu time will be accounted properly. > > This feature can already be used with libvirt by adding the following > to the XML domain definition to pass the parameter to qemu directly: > > <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0"> > <arg value='-async-teardown'/> > </commandline> > > More advanced interfaces like pidfd or close_range have intentionally > been avoided in order to be more compatible with older kernels. > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > include/qemu/async-teardown.h | 22 ++++++ > os-posix.c | 6 ++ > qemu-options.hx | 17 +++++ > util/async-teardown.c | 123 ++++++++++++++++++++++++++++++++++ > util/meson.build | 1 + > 5 files changed, 169 insertions(+) > create mode 100644 include/qemu/async-teardown.h > create mode 100644 util/async-teardown.c > > diff --git a/include/qemu/async-teardown.h b/include/qemu/async-teardown.h > new file mode 100644 > index 0000000000..092e7a37e7 > --- /dev/null > +++ b/include/qemu/async-teardown.h > @@ -0,0 +1,22 @@ > +/* > + * Asynchronous teardown > + * > + * Copyright IBM, Corp. 2022 > + * > + * Authors: > + * Claudio Imbrenda <imbrenda@linux.ibm.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or (at your > + * option) any later version. See the COPYING file in the top-level directory. > + * > + */ > +#ifndef QEMU_ASYNC_TEARDOWN_H > +#define QEMU_ASYNC_TEARDOWN_H > + > +#include "config-host.h" > + > +#ifdef CONFIG_LINUX > +void init_async_teardown(void); > +#endif > + > +#endif > diff --git a/os-posix.c b/os-posix.c > index 321fc4bd13..4858650c3e 100644 > --- a/os-posix.c > +++ b/os-posix.c > @@ -39,6 +39,7 @@ > > #ifdef CONFIG_LINUX > #include <sys/prctl.h> > +#include "qemu/async-teardown.h" > #endif > > /* > @@ -150,6 +151,11 @@ int os_parse_cmd_args(int index, const char *optarg) > case QEMU_OPTION_daemonize: > daemonize = 1; > break; > +#if defined(CONFIG_LINUX) > + case QEMU_OPTION_asyncteardown: > + init_async_teardown(); > + break; > +#endif > default: > return -1; > } > diff --git a/qemu-options.hx b/qemu-options.hx > index 3f23a42fa8..d434353159 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -4743,6 +4743,23 @@ HXCOMM Internal use > DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL) > DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL) > > +#ifdef __linux__ > +DEF("async-teardown", 0, QEMU_OPTION_asyncteardown, > + "-async-teardown enable asynchronous teardown\n", > + QEMU_ARCH_ALL) > +#endif > +SRST > +``-async-teardown`` > + Enable asynchronous teardown. A new teardown process will be > + created at startup, using clone. The teardown process will share > + the address space of the main qemu process, and wait for the main > + process to terminate. At that point, the teardown process will > + also exit. This allows qemu to terminate quickly if the guest was > + huge, leaving the teardown of the address space to the teardown > + process. Since the teardown process shares the same cgroups as the > + main qemu process, accounting is performed correctly. > +ERST > + > DEF("msg", HAS_ARG, QEMU_OPTION_msg, > "-msg [timestamp[=on|off]][,guest-name=[on|off]]\n" > " control error message format\n" It occurrs to me that we've got a general goal of getting away from adding new top level command line arguments. Most of the time there's an obvious existing place to put them, but I'm really not sure where this particular option would fit ? it isn't tied to any aspect of the VM backend configuration nor hardware frontends. The closest match is the lifecycle action option (-no-shutdown) which were merged into a -action arg, but even that's a bit of a stretch. Markus/Paolo: do you have suggestions ? > diff --git a/util/async-teardown.c b/util/async-teardown.c > new file mode 100644 > index 0000000000..07fe549891 > --- /dev/null > +++ b/util/async-teardown.c > @@ -0,0 +1,123 @@ > +/* > + * Asynchronous teardown > + * > + * Copyright IBM, Corp. 2022 > + * > + * Authors: > + * Claudio Imbrenda <imbrenda@linux.ibm.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or (at your > + * option) any later version. See the COPYING file in the top-level directory. > + * > + */ > +#include <stdlib.h> > +#include <stdio.h> > +#include <sys/types.h> > +#include <sys/unistd.h> > +#include <dirent.h> > +#include <sys/prctl.h> > +#include <signal.h> > +#include <sched.h> > + > +#include "qemu/async-teardown.h" > + > +static pid_t the_ppid; > + > +/* > + * Close all open file descriptors. > + */ > +static void close_all_open_fd(void) > +{ > + struct dirent *de; > + int fd, dfd; > + DIR *dir; > + > + dir = opendir("/proc/self/fd"); > + if (!dir) { > + return; > + } > + /* Avoid closing the directory. */ > + dfd = dirfd(dir); > + > + for (de = readdir(dir); de; de = readdir(dir)) { > + fd = atoi(de->d_name); > + if (fd != dfd) { > + close(fd); > + } > + } > + closedir(dir); > +} I know you said you're intentionally avoided close_range, but the cost is that this makes 'proc' a mandatory dependancy. IMHO, we should build with close_range support if it is available in the glibc headers. Only use proc as a fallback if close_range reports ENOSYS, or is compiled out. > + > +static void hup_handler(int signal) > +{ > + /* Check every second if this process has been reparented. */ > + while (the_ppid == getppid()) { > + /* sleep() is safe to use in a signal handler. */ > + sleep(1); > + } > + > + /* At this point the parent process has terminated completely. */ > + _exit(0); > +} > + > +static int async_teardown_fn(void *arg) > +{ > + struct sigaction sa = { .sa_handler = hup_handler }; > + sigset_t hup_signal; > + char name[16]; > + > + /* Set a meaningful name for this process. */ > + snprintf(name, 16, "cleanup/%d", the_ppid); > + prctl(PR_SET_NAME, (unsigned long)name); > + > + /* > + * Close all file descriptors that might have been inherited from the > + * main qemu process when doing clone, needed to make libvirt happy. > + * Not using close_range for increased compatibility with older kernels. > + */ > + close_all_open_fd(); > + > + /* Set up a handler for SIGHUP and unblock SIGHUP. */ > + sigaction(SIGHUP, &sa, NULL); > + sigemptyset(&hup_signal); > + sigaddset(&hup_signal, SIGHUP); > + sigprocmask(SIG_UNBLOCK, &hup_signal, NULL); > + > + /* Ask to receive SIGHUP when the parent dies. */ > + prctl(PR_SET_PDEATHSIG, SIGHUP); Hmm, I was hoping you could just use SIGKILL to guarantee that this gets killed off. Is SIGKILL delivered too soon to allow for the main QEMU process to have exited quickly ? If so I wonder what happens when systemd just delivers SIGKILL to all processes in the cgroup - I'm not sure there's a guarantee it will SIGKILL the main qemu before it SIGKILLs this helper > + > + /* > + * Sleep forever, unless the parent process has already terminated. The > + * only interruption can come from the SIGHUP signal, which in normal > + * operation is received when the parent process dies. > + */ > + if (the_ppid == getppid()) { > + pause(); > + } > + > + /* At this point the parent process has terminated completely. */ > + _exit(0); > +} > + > +/* > + * Block all signals, start (clone) a new process sharing the address space > + * with qemu (CLONE_VM), then restore signals. > + */ > +void init_async_teardown(void) > +{ > + sigset_t all_signals, old_signals; > + const int stack_size = 8192; /* Should be more than enough */ > + char *stack, *stack_ptr; > + > + the_ppid = getpid(); > + stack = malloc(stack_size); > + if (!stack) { > + return; THis should be fatal. How about using qemu_alloc_stack instead ? > + } > + stack_ptr = stack + stack_size; > + > + sigfillset(&all_signals); > + sigprocmask(SIG_BLOCK, &all_signals, &old_signals); > + clone(async_teardown_fn, stack_ptr, CLONE_VM, NULL, NULL, NULL, NULL); > + sigprocmask(SIG_SETMASK, &old_signals, NULL); > +} > diff --git a/util/meson.build b/util/meson.build > index 5e282130df..63acd59bb0 100644 > --- a/util/meson.build > +++ b/util/meson.build > @@ -2,6 +2,7 @@ util_ss.add(files('osdep.c', 'cutils.c', 'unicode.c', 'qemu-timer-common.c')) > if not config_host_data.get('CONFIG_ATOMIC64') > util_ss.add(files('atomic64.c')) > endif > +util_ss.add(when: 'CONFIG_LINUX', if_true: files('async-teardown.c')) > util_ss.add(when: 'CONFIG_POSIX', if_true: files('aio-posix.c')) > util_ss.add(when: 'CONFIG_POSIX', if_true: files('fdmon-poll.c')) > if config_host_data.get('CONFIG_EPOLL_CREATE1') > -- > 2.37.1 > With regards, Daniel
Am 11.08.22 um 14:27 schrieb Daniel P. Berrangé: [...] >> --- a/qemu-options.hx >> +++ b/qemu-options.hx >> @@ -4743,6 +4743,23 @@ HXCOMM Internal use >> DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL) >> DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL) >> >> +#ifdef __linux__ >> +DEF("async-teardown", 0, QEMU_OPTION_asyncteardown, >> + "-async-teardown enable asynchronous teardown\n", >> + QEMU_ARCH_ALL) >> +#endif >> +SRST >> +``-async-teardown`` >> + Enable asynchronous teardown. A new teardown process will be >> + created at startup, using clone. The teardown process will share >> + the address space of the main qemu process, and wait for the main >> + process to terminate. At that point, the teardown process will >> + also exit. This allows qemu to terminate quickly if the guest was >> + huge, leaving the teardown of the address space to the teardown >> + process. Since the teardown process shares the same cgroups as the >> + main qemu process, accounting is performed correctly. >> +ERST >> + >> DEF("msg", HAS_ARG, QEMU_OPTION_msg, >> "-msg [timestamp[=on|off]][,guest-name=[on|off]]\n" >> " control error message format\n" > > It occurrs to me that we've got a general goal of getting away from > adding new top level command line arguments. Most of the time there's > an obvious existing place to put them, but I'm really not sure > where this particular option would fit ? > > it isn't tied to any aspect of the VM backend configuration nor > hardware frontends. > > The closest match is the lifecycle action option (-no-shutdown) > which were merged into a -action arg, but even that's a bit of a > stretch. > > Markus/Paolo: do you have suggestions ? Also extending this to libvirt, would it make sense to add this to the event list <on_poweroff> <on_reboot> <on_crash> <on_lockfailure> as <on_teardown> with async/sync This might give an indication where to put it in qemu.
On Thu, 11 Aug 2022 13:27:44 +0100 Daniel P. Berrangé <berrange@redhat.com> wrote: > On Tue, Aug 09, 2022 at 08:40:24AM +0200, Claudio Imbrenda wrote: > > This patch adds support for asynchronously tearing down a VM on Linux. > > > > When qemu terminates, either naturally or because of a fatal signal, > > the VM is torn down. If the VM is huge, it can take a considerable > > amount of time for it to be cleaned up. In case of a protected VM, it > > might take even longer than a non-protected VM (this is the case on > > s390x, for example). > > > > Some users might want to shut down a VM and restart it immediately, > > without having to wait. This is especially true if management > > infrastructure like libvirt is used. > > > > This patch implements a simple trick on Linux to allow qemu to return > > immediately, with the teardown of the VM being performed > > asynchronously. > > > > If the new commandline option -async-teardown is used, a new process is > > spawned from qemu at startup, using the clone syscall, in such way that > > it will share its address space with qemu. > > > > The new process will have the name "cleanup/<QEMU_PID>". It will wait > > until qemu terminates, and then it will exit itself. > > > > This allows qemu to terminate quickly, without having to wait for the > > whole address space to be torn down. The teardown process will exit > > after qemu, so it will be the last user of the address space, and > > therefore it will take care of the actual teardown. > > > > The teardown process will share the same cgroups as qemu, so both > > memory usage and cpu time will be accounted properly. > > > > This feature can already be used with libvirt by adding the following > > to the XML domain definition to pass the parameter to qemu directly: > > > > <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0"> > > <arg value='-async-teardown'/> > > </commandline> > > > > More advanced interfaces like pidfd or close_range have intentionally > > been avoided in order to be more compatible with older kernels. > > > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > --- > > include/qemu/async-teardown.h | 22 ++++++ > > os-posix.c | 6 ++ > > qemu-options.hx | 17 +++++ > > util/async-teardown.c | 123 ++++++++++++++++++++++++++++++++++ > > util/meson.build | 1 + > > 5 files changed, 169 insertions(+) > > create mode 100644 include/qemu/async-teardown.h > > create mode 100644 util/async-teardown.c > > > > > diff --git a/include/qemu/async-teardown.h b/include/qemu/async-teardown.h > > new file mode 100644 > > index 0000000000..092e7a37e7 > > --- /dev/null > > +++ b/include/qemu/async-teardown.h > > @@ -0,0 +1,22 @@ > > +/* > > + * Asynchronous teardown > > + * > > + * Copyright IBM, Corp. 2022 > > + * > > + * Authors: > > + * Claudio Imbrenda <imbrenda@linux.ibm.com> > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2 or (at your > > + * option) any later version. See the COPYING file in the top-level directory. > > + * > > + */ > > +#ifndef QEMU_ASYNC_TEARDOWN_H > > +#define QEMU_ASYNC_TEARDOWN_H > > + > > +#include "config-host.h" > > + > > +#ifdef CONFIG_LINUX > > +void init_async_teardown(void); > > +#endif > > + > > +#endif > > diff --git a/os-posix.c b/os-posix.c > > index 321fc4bd13..4858650c3e 100644 > > --- a/os-posix.c > > +++ b/os-posix.c > > @@ -39,6 +39,7 @@ > > > > #ifdef CONFIG_LINUX > > #include <sys/prctl.h> > > +#include "qemu/async-teardown.h" > > #endif > > > > /* > > @@ -150,6 +151,11 @@ int os_parse_cmd_args(int index, const char *optarg) > > case QEMU_OPTION_daemonize: > > daemonize = 1; > > break; > > +#if defined(CONFIG_LINUX) > > + case QEMU_OPTION_asyncteardown: > > + init_async_teardown(); > > + break; > > +#endif > > default: > > return -1; > > } > > diff --git a/qemu-options.hx b/qemu-options.hx > > index 3f23a42fa8..d434353159 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -4743,6 +4743,23 @@ HXCOMM Internal use > > DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL) > > DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL) > > > > +#ifdef __linux__ > > +DEF("async-teardown", 0, QEMU_OPTION_asyncteardown, > > + "-async-teardown enable asynchronous teardown\n", > > + QEMU_ARCH_ALL) > > +#endif > > +SRST > > +``-async-teardown`` > > + Enable asynchronous teardown. A new teardown process will be > > + created at startup, using clone. The teardown process will share > > + the address space of the main qemu process, and wait for the main > > + process to terminate. At that point, the teardown process will > > + also exit. This allows qemu to terminate quickly if the guest was > > + huge, leaving the teardown of the address space to the teardown > > + process. Since the teardown process shares the same cgroups as the > > + main qemu process, accounting is performed correctly. > > +ERST > > + > > DEF("msg", HAS_ARG, QEMU_OPTION_msg, > > "-msg [timestamp[=on|off]][,guest-name=[on|off]]\n" > > " control error message format\n" > > It occurrs to me that we've got a general goal of getting away from > adding new top level command line arguments. Most of the time there's > an obvious existing place to put them, but I'm really not sure > where this particular option would fit ? > > it isn't tied to any aspect of the VM backend configuration nor > hardware frontends. > > The closest match is the lifecycle action option (-no-shutdown) > which were merged into a -action arg, but even that's a bit of a > stretch. > I have no preferences as to how the parameter is passed > Markus/Paolo: do you have suggestions ? > > > > diff --git a/util/async-teardown.c b/util/async-teardown.c > > new file mode 100644 > > index 0000000000..07fe549891 > > --- /dev/null > > +++ b/util/async-teardown.c > > @@ -0,0 +1,123 @@ > > +/* > > + * Asynchronous teardown > > + * > > + * Copyright IBM, Corp. 2022 > > + * > > + * Authors: > > + * Claudio Imbrenda <imbrenda@linux.ibm.com> > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2 or (at your > > + * option) any later version. See the COPYING file in the top-level directory. > > + * > > + */ > > +#include <stdlib.h> > > +#include <stdio.h> > > +#include <sys/types.h> > > +#include <sys/unistd.h> > > +#include <dirent.h> > > +#include <sys/prctl.h> > > +#include <signal.h> > > +#include <sched.h> > > + > > +#include "qemu/async-teardown.h" > > + > > +static pid_t the_ppid; > > + > > +/* > > + * Close all open file descriptors. > > + */ > > +static void close_all_open_fd(void) > > +{ > > + struct dirent *de; > > + int fd, dfd; > > + DIR *dir; > > + > > + dir = opendir("/proc/self/fd"); > > + if (!dir) { > > + return; > > + } > > + /* Avoid closing the directory. */ > > + dfd = dirfd(dir); > > + > > + for (de = readdir(dir); de; de = readdir(dir)) { > > + fd = atoi(de->d_name); > > + if (fd != dfd) { > > + close(fd); > > + } > > + } > > + closedir(dir); > > +} > > I know you said you're intentionally avoided close_range, but the > cost is that this makes 'proc' a mandatory dependancy. Running qemu without /proc sounds a little bit weird, but ok > > IMHO, we should build with close_range support if it is available > in the glibc headers. Only use proc as a fallback if close_range > reports ENOSYS, or is compiled out. sounds reasonable. what is the best way to do it? just some #ifdefs to check if the syscall is available, or a config option to check at config time if the syscall is defined? (and then I guess the ENOSYS check needs to be there anyway) > > > + > > +static void hup_handler(int signal) > > +{ > > + /* Check every second if this process has been reparented. */ > > + while (the_ppid == getppid()) { > > + /* sleep() is safe to use in a signal handler. */ > > + sleep(1); > > + } > > + > > + /* At this point the parent process has terminated completely. */ > > + _exit(0); > > +} > > + > > +static int async_teardown_fn(void *arg) > > +{ > > + struct sigaction sa = { .sa_handler = hup_handler }; > > + sigset_t hup_signal; > > + char name[16]; > > + > > + /* Set a meaningful name for this process. */ > > + snprintf(name, 16, "cleanup/%d", the_ppid); > > + prctl(PR_SET_NAME, (unsigned long)name); > > + > > + /* > > + * Close all file descriptors that might have been inherited from the > > + * main qemu process when doing clone, needed to make libvirt happy. > > + * Not using close_range for increased compatibility with older kernels. > > + */ > > + close_all_open_fd(); > > + > > + /* Set up a handler for SIGHUP and unblock SIGHUP. */ > > + sigaction(SIGHUP, &sa, NULL); > > + sigemptyset(&hup_signal); > > + sigaddset(&hup_signal, SIGHUP); > > + sigprocmask(SIG_UNBLOCK, &hup_signal, NULL); > > + > > + /* Ask to receive SIGHUP when the parent dies. */ > > + prctl(PR_SET_PDEATHSIG, SIGHUP); > > Hmm, I was hoping you could just use SIGKILL to guarantee that this > gets killed off. Is SIGKILL delivered too soon to allow for the > main QEMU process to have exited quickly ? yes, I tried. qemu has not finished exiting when the signal is delivered, the cleanup process dies before qemu, which defeats the purpose > > If so I wonder what happens when systemd just delivers SIGKILL to > all processes in the cgroup - I'm not sure there's a guarantee it > will SIGKILL the main qemu before it SIGKILLs this helper I'm afraid in that case there is no guarantee. for what it's worth, both virsh shutdown and destroy seem to do things properly. > > > + > > + /* > > + * Sleep forever, unless the parent process has already terminated. The > > + * only interruption can come from the SIGHUP signal, which in normal > > + * operation is received when the parent process dies. > > + */ > > + if (the_ppid == getppid()) { > > + pause(); > > + } > > + > > + /* At this point the parent process has terminated completely. */ > > + _exit(0); > > +} > > + > > +/* > > + * Block all signals, start (clone) a new process sharing the address space > > + * with qemu (CLONE_VM), then restore signals. > > + */ > > +void init_async_teardown(void) > > +{ > > + sigset_t all_signals, old_signals; > > + const int stack_size = 8192; /* Should be more than enough */ > > + char *stack, *stack_ptr; > > + > > + the_ppid = getpid(); > > + stack = malloc(stack_size); > > + if (!stack) { > > + return; > > THis should be fatal. How about using qemu_alloc_stack instead ? sounds like a great idea > > > + } > > + stack_ptr = stack + stack_size; > > + > > + sigfillset(&all_signals); > > + sigprocmask(SIG_BLOCK, &all_signals, &old_signals); > > + clone(async_teardown_fn, stack_ptr, CLONE_VM, NULL, NULL, NULL, NULL); > > + sigprocmask(SIG_SETMASK, &old_signals, NULL); > > +} > > diff --git a/util/meson.build b/util/meson.build > > index 5e282130df..63acd59bb0 100644 > > --- a/util/meson.build > > +++ b/util/meson.build > > @@ -2,6 +2,7 @@ util_ss.add(files('osdep.c', 'cutils.c', 'unicode.c', 'qemu-timer-common.c')) > > if not config_host_data.get('CONFIG_ATOMIC64') > > util_ss.add(files('atomic64.c')) > > endif > > +util_ss.add(when: 'CONFIG_LINUX', if_true: files('async-teardown.c')) > > util_ss.add(when: 'CONFIG_POSIX', if_true: files('aio-posix.c')) > > util_ss.add(when: 'CONFIG_POSIX', if_true: files('fdmon-poll.c')) > > if config_host_data.get('CONFIG_EPOLL_CREATE1') > > -- > > 2.37.1 > > > > With regards, > Daniel
On Thu, Aug 11, 2022 at 03:56:23PM +0200, Claudio Imbrenda wrote: > On Thu, 11 Aug 2022 13:27:44 +0100 > Daniel P. Berrangé <berrange@redhat.com> wrote: > > > On Tue, Aug 09, 2022 at 08:40:24AM +0200, Claudio Imbrenda wrote: > > > This patch adds support for asynchronously tearing down a VM on Linux. > > > > > > When qemu terminates, either naturally or because of a fatal signal, > > > the VM is torn down. If the VM is huge, it can take a considerable > > > amount of time for it to be cleaned up. In case of a protected VM, it > > > might take even longer than a non-protected VM (this is the case on > > > s390x, for example). > > > > > > Some users might want to shut down a VM and restart it immediately, > > > without having to wait. This is especially true if management > > > infrastructure like libvirt is used. > > > > > > This patch implements a simple trick on Linux to allow qemu to return > > > immediately, with the teardown of the VM being performed > > > asynchronously. > > > > > > If the new commandline option -async-teardown is used, a new process is > > > spawned from qemu at startup, using the clone syscall, in such way that > > > it will share its address space with qemu. > > > > > > The new process will have the name "cleanup/<QEMU_PID>". It will wait > > > until qemu terminates, and then it will exit itself. > > > > > > This allows qemu to terminate quickly, without having to wait for the > > > whole address space to be torn down. The teardown process will exit > > > after qemu, so it will be the last user of the address space, and > > > therefore it will take care of the actual teardown. > > > > > > The teardown process will share the same cgroups as qemu, so both > > > memory usage and cpu time will be accounted properly. > > > > > > This feature can already be used with libvirt by adding the following > > > to the XML domain definition to pass the parameter to qemu directly: > > > > > > <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0"> > > > <arg value='-async-teardown'/> > > > </commandline> > > > > > > More advanced interfaces like pidfd or close_range have intentionally > > > been avoided in order to be more compatible with older kernels. > > > > > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > > --- > > > include/qemu/async-teardown.h | 22 ++++++ > > > os-posix.c | 6 ++ > > > qemu-options.hx | 17 +++++ > > > util/async-teardown.c | 123 ++++++++++++++++++++++++++++++++++ > > > util/meson.build | 1 + > > > 5 files changed, 169 insertions(+) > > > create mode 100644 include/qemu/async-teardown.h > > > create mode 100644 util/async-teardown.c > > > > > > > > diff --git a/include/qemu/async-teardown.h b/include/qemu/async-teardown.h > > > new file mode 100644 > > > index 0000000000..092e7a37e7 > > > --- /dev/null > > > +++ b/include/qemu/async-teardown.h > > > @@ -0,0 +1,22 @@ > > > +/* > > > + * Asynchronous teardown > > > + * > > > + * Copyright IBM, Corp. 2022 > > > + * > > > + * Authors: > > > + * Claudio Imbrenda <imbrenda@linux.ibm.com> > > > + * > > > + * This work is licensed under the terms of the GNU GPL, version 2 or (at your > > > + * option) any later version. See the COPYING file in the top-level directory. > > > + * > > > + */ > > > +#ifndef QEMU_ASYNC_TEARDOWN_H > > > +#define QEMU_ASYNC_TEARDOWN_H > > > + > > > +#include "config-host.h" > > > + > > > +#ifdef CONFIG_LINUX > > > +void init_async_teardown(void); > > > +#endif > > > + > > > +#endif > > > diff --git a/os-posix.c b/os-posix.c > > > index 321fc4bd13..4858650c3e 100644 > > > --- a/os-posix.c > > > +++ b/os-posix.c > > > @@ -39,6 +39,7 @@ > > > > > > #ifdef CONFIG_LINUX > > > #include <sys/prctl.h> > > > +#include "qemu/async-teardown.h" > > > #endif > > > > > > /* > > > @@ -150,6 +151,11 @@ int os_parse_cmd_args(int index, const char *optarg) > > > case QEMU_OPTION_daemonize: > > > daemonize = 1; > > > break; > > > +#if defined(CONFIG_LINUX) > > > + case QEMU_OPTION_asyncteardown: > > > + init_async_teardown(); > > > + break; > > > +#endif > > > default: > > > return -1; > > > } > > > diff --git a/qemu-options.hx b/qemu-options.hx > > > index 3f23a42fa8..d434353159 100644 > > > --- a/qemu-options.hx > > > +++ b/qemu-options.hx > > > @@ -4743,6 +4743,23 @@ HXCOMM Internal use > > > DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL) > > > DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL) > > > > > > +#ifdef __linux__ > > > +DEF("async-teardown", 0, QEMU_OPTION_asyncteardown, > > > + "-async-teardown enable asynchronous teardown\n", > > > + QEMU_ARCH_ALL) > > > +#endif > > > +SRST > > > +``-async-teardown`` > > > + Enable asynchronous teardown. A new teardown process will be > > > + created at startup, using clone. The teardown process will share > > > + the address space of the main qemu process, and wait for the main > > > + process to terminate. At that point, the teardown process will > > > + also exit. This allows qemu to terminate quickly if the guest was > > > + huge, leaving the teardown of the address space to the teardown > > > + process. Since the teardown process shares the same cgroups as the > > > + main qemu process, accounting is performed correctly. > > > +ERST > > > + > > > DEF("msg", HAS_ARG, QEMU_OPTION_msg, > > > "-msg [timestamp[=on|off]][,guest-name=[on|off]]\n" > > > " control error message format\n" > > > > It occurrs to me that we've got a general goal of getting away from > > adding new top level command line arguments. Most of the time there's > > an obvious existing place to put them, but I'm really not sure > > where this particular option would fit ? > > > > it isn't tied to any aspect of the VM backend configuration nor > > hardware frontends. > > > > The closest match is the lifecycle action option (-no-shutdown) > > which were merged into a -action arg, but even that's a bit of a > > stretch. > > > > I have no preferences as to how the parameter is passed > > > Markus/Paolo: do you have suggestions ? > > > > > > > diff --git a/util/async-teardown.c b/util/async-teardown.c > > > new file mode 100644 > > > index 0000000000..07fe549891 > > > --- /dev/null > > > +++ b/util/async-teardown.c > > > @@ -0,0 +1,123 @@ > > > +/* > > > + * Asynchronous teardown > > > + * > > > + * Copyright IBM, Corp. 2022 > > > + * > > > + * Authors: > > > + * Claudio Imbrenda <imbrenda@linux.ibm.com> > > > + * > > > + * This work is licensed under the terms of the GNU GPL, version 2 or (at your > > > + * option) any later version. See the COPYING file in the top-level directory. > > > + * > > > + */ > > > +#include <stdlib.h> > > > +#include <stdio.h> > > > +#include <sys/types.h> > > > +#include <sys/unistd.h> > > > +#include <dirent.h> > > > +#include <sys/prctl.h> > > > +#include <signal.h> > > > +#include <sched.h> > > > + > > > +#include "qemu/async-teardown.h" > > > + > > > +static pid_t the_ppid; > > > + > > > +/* > > > + * Close all open file descriptors. > > > + */ > > > +static void close_all_open_fd(void) > > > +{ > > > + struct dirent *de; > > > + int fd, dfd; > > > + DIR *dir; > > > + > > > + dir = opendir("/proc/self/fd"); > > > + if (!dir) { > > > + return; > > > + } > > > + /* Avoid closing the directory. */ > > > + dfd = dirfd(dir); > > > + > > > + for (de = readdir(dir); de; de = readdir(dir)) { > > > + fd = atoi(de->d_name); > > > + if (fd != dfd) { > > > + close(fd); > > > + } > > > + } > > > + closedir(dir); > > > +} > > > > I know you said you're intentionally avoided close_range, but the > > cost is that this makes 'proc' a mandatory dependancy. > > Running qemu without /proc sounds a little bit weird, but ok People have been doing increasingly wierd things with containers ;-) > > IMHO, we should build with close_range support if it is available > > in the glibc headers. Only use proc as a fallback if close_range > > reports ENOSYS, or is compiled out. > > sounds reasonable. > > what is the best way to do it? just some #ifdefs to check if the > syscall is available, or a config option to check at config time if the > syscall is defined? > > (and then I guess the ENOSYS check needs to be there anyway) Probably a check for the function in meson.build, and then some #ifdefs, and a catch of ENOSYS. > > > + > > > +static void hup_handler(int signal) > > > +{ > > > + /* Check every second if this process has been reparented. */ > > > + while (the_ppid == getppid()) { > > > + /* sleep() is safe to use in a signal handler. */ > > > + sleep(1); > > > + } > > > + > > > + /* At this point the parent process has terminated completely. */ > > > + _exit(0); > > > +} > > > + > > > +static int async_teardown_fn(void *arg) > > > +{ > > > + struct sigaction sa = { .sa_handler = hup_handler }; > > > + sigset_t hup_signal; > > > + char name[16]; > > > + > > > + /* Set a meaningful name for this process. */ > > > + snprintf(name, 16, "cleanup/%d", the_ppid); > > > + prctl(PR_SET_NAME, (unsigned long)name); > > > + > > > + /* > > > + * Close all file descriptors that might have been inherited from the > > > + * main qemu process when doing clone, needed to make libvirt happy. > > > + * Not using close_range for increased compatibility with older kernels. > > > + */ > > > + close_all_open_fd(); > > > + > > > + /* Set up a handler for SIGHUP and unblock SIGHUP. */ > > > + sigaction(SIGHUP, &sa, NULL); > > > + sigemptyset(&hup_signal); > > > + sigaddset(&hup_signal, SIGHUP); > > > + sigprocmask(SIG_UNBLOCK, &hup_signal, NULL); > > > + > > > + /* Ask to receive SIGHUP when the parent dies. */ > > > + prctl(PR_SET_PDEATHSIG, SIGHUP); > > > > Hmm, I was hoping you could just use SIGKILL to guarantee that this > > gets killed off. Is SIGKILL delivered too soon to allow for the > > main QEMU process to have exited quickly ? > > yes, I tried. qemu has not finished exiting when the signal is > delivered, the cleanup process dies before qemu, which defeats the > purpose Ok, too bad. > > If so I wonder what happens when systemd just delivers SIGKILL to > > all processes in the cgroup - I'm not sure there's a guarantee it > > will SIGKILL the main qemu before it SIGKILLs this helper > > I'm afraid in that case there is no guarantee. > > for what it's worth, both virsh shutdown and destroy seem to do things > properly. Hmm, probably because libvirt tells QEMU to exit before systemd comes along and tells everything in the cgroup to die with SIGKILL. With regards, Daniel
On 8/11/22 11:02, Daniel P. Berrangé wrote: [...] >>> Hmm, I was hoping you could just use SIGKILL to guarantee that this >>> gets killed off. Is SIGKILL delivered too soon to allow for the >>> main QEMU process to have exited quickly ? >> >> yes, I tried. qemu has not finished exiting when the signal is >> delivered, the cleanup process dies before qemu, which defeats the >> purpose > > Ok, too bad. > >>> If so I wonder what happens when systemd just delivers SIGKILL to >>> all processes in the cgroup - I'm not sure there's a guarantee it >>> will SIGKILL the main qemu before it SIGKILLs this helper >> >> I'm afraid in that case there is no guarantee. >> >> for what it's worth, both virsh shutdown and destroy seem to do things >> properly. > > Hmm, probably because libvirt tells QEMU to exit before systemd comes > along and tells everything in the cgroup to die with SIGKILL. It seems Libvirt sends SIGKILL if qemu process doesn't terminate within 10 seconds after Libvirt sent SIGTERM: https://gitlab.com/libvirt/libvirt/-/blob/0615df084ec9996b5df88d6a1b59c557e22f3a12/src/util/virprocess.c#L375 So I guess this patch happened to work with Libvirt because the main qemu process terminated before the timeout and before SIGKILL was delivered. The cleanup process is trying to solve the problem where the main qemu process takes too long to terminate. However, if the cleanup process itself takes too long, SIGKILL will be sent by Libvirt anyway. Perhaps we can describe this situation in the parameter help, e.g.: If management layer decides to send SIGKILL (e.g.: due to timeout or deliberate decision), the cleanup process can exit before the main process, deceiving its purpose.
On Thu, 11 Aug 2022 23:05:52 -0300 Murilo Opsfelder Araújo <muriloo@linux.ibm.com> wrote: > On 8/11/22 11:02, Daniel P. Berrangé wrote: > [...] > >>> Hmm, I was hoping you could just use SIGKILL to guarantee that this > >>> gets killed off. Is SIGKILL delivered too soon to allow for the > >>> main QEMU process to have exited quickly ? > >> > >> yes, I tried. qemu has not finished exiting when the signal is > >> delivered, the cleanup process dies before qemu, which defeats the > >> purpose > > > > Ok, too bad. > > > >>> If so I wonder what happens when systemd just delivers SIGKILL to > >>> all processes in the cgroup - I'm not sure there's a guarantee it > >>> will SIGKILL the main qemu before it SIGKILLs this helper > >> > >> I'm afraid in that case there is no guarantee. > >> > >> for what it's worth, both virsh shutdown and destroy seem to do things > >> properly. > > > > Hmm, probably because libvirt tells QEMU to exit before systemd comes > > along and tells everything in the cgroup to die with SIGKILL. > > It seems Libvirt sends SIGKILL if qemu process doesn't terminate within 10 > seconds after Libvirt sent SIGTERM: > > https://gitlab.com/libvirt/libvirt/-/blob/0615df084ec9996b5df88d6a1b59c557e22f3a12/src/util/virprocess.c#L375 but this is fine. with asynchronous teardown, qemu will exit almost immediately when receiving SIGTERM, and the cleanup process will start cleaning up. > > So I guess this patch happened to work with Libvirt because the main qemu > process terminated before the timeout and before SIGKILL was delivered. it seems so > > The cleanup process is trying to solve the problem where the main qemu process > takes too long to terminate. However, if the cleanup process itself takes too > long, SIGKILL will be sent by Libvirt anyway. but that is not a problem, the sole purpose of the cleanup process is to terminate _after_ qemu. it doesn't matter what happens after qemu has terminated. if you look at the patch, after going to great lengths to assure that qemu has terminated, all the child process does is _exit(0). > > Perhaps we can describe this situation in the parameter help, e.g.: If > management layer decides to send SIGKILL (e.g.: due to timeout or deliberate > decision), the cleanup process can exit before the main process, deceiving its > purpose. if the management layer (or the user) decides to send SIGKILL immediately to the whole cgroup without sending SIGTERM first, then this whole asynchronous teardown mechanism is defeated, yes.
On 8/12/22 04:26, Claudio Imbrenda wrote: > On Thu, 11 Aug 2022 23:05:52 -0300 > Murilo Opsfelder Araújo <muriloo@linux.ibm.com> wrote: > >> On 8/11/22 11:02, Daniel P. Berrangé wrote: >> [...] >>>>> Hmm, I was hoping you could just use SIGKILL to guarantee that this >>>>> gets killed off. Is SIGKILL delivered too soon to allow for the >>>>> main QEMU process to have exited quickly ? >>>> >>>> yes, I tried. qemu has not finished exiting when the signal is >>>> delivered, the cleanup process dies before qemu, which defeats the >>>> purpose >>> >>> Ok, too bad. >>> >>>>> If so I wonder what happens when systemd just delivers SIGKILL to >>>>> all processes in the cgroup - I'm not sure there's a guarantee it >>>>> will SIGKILL the main qemu before it SIGKILLs this helper >>>> >>>> I'm afraid in that case there is no guarantee. >>>> >>>> for what it's worth, both virsh shutdown and destroy seem to do things >>>> properly. >>> >>> Hmm, probably because libvirt tells QEMU to exit before systemd comes >>> along and tells everything in the cgroup to die with SIGKILL. >> >> It seems Libvirt sends SIGKILL if qemu process doesn't terminate within 10 >> seconds after Libvirt sent SIGTERM: >> >> https://gitlab.com/libvirt/libvirt/-/blob/0615df084ec9996b5df88d6a1b59c557e22f3a12/src/util/virprocess.c#L375 > > but this is fine. > > with asynchronous teardown, qemu will exit almost immediately when > receiving SIGTERM, and the cleanup process will start cleaning up. Under normal and orderly conditions, yes. >> So I guess this patch happened to work with Libvirt because the main qemu >> process terminated before the timeout and before SIGKILL was delivered. > > it seems so > >> >> The cleanup process is trying to solve the problem where the main qemu process >> takes too long to terminate. However, if the cleanup process itself takes too >> long, SIGKILL will be sent by Libvirt anyway. > > but that is not a problem, the sole purpose of the cleanup process is > to terminate _after_ qemu. it doesn't matter what happens after qemu > has terminated. if you look at the patch, after going to great lengths > to assure that qemu has terminated, all the child process does is > _exit(0). > >> >> Perhaps we can describe this situation in the parameter help, e.g.: If >> management layer decides to send SIGKILL (e.g.: due to timeout or deliberate >> decision), the cleanup process can exit before the main process, deceiving its >> purpose. > > if the management layer (or the user) decides to send SIGKILL > immediately to the whole cgroup without sending SIGTERM first, then > this whole asynchronous teardown mechanism is defeated, yes. This situation is what we likely want to describe in the parameter help. I don't want to give users the false impression that this option will *always* behave the manner we expect it to work *most* of the time. -- Murilo
On Fri, 12 Aug 2022 08:38:59 -0300 Murilo Opsfelder Araújo <muriloo@linux.ibm.com> wrote: > On 8/12/22 04:26, Claudio Imbrenda wrote: > > On Thu, 11 Aug 2022 23:05:52 -0300 > > Murilo Opsfelder Araújo <muriloo@linux.ibm.com> wrote: > > > >> On 8/11/22 11:02, Daniel P. Berrangé wrote: > >> [...] > >>>>> Hmm, I was hoping you could just use SIGKILL to guarantee that this > >>>>> gets killed off. Is SIGKILL delivered too soon to allow for the > >>>>> main QEMU process to have exited quickly ? > >>>> > >>>> yes, I tried. qemu has not finished exiting when the signal is > >>>> delivered, the cleanup process dies before qemu, which defeats the > >>>> purpose > >>> > >>> Ok, too bad. > >>> > >>>>> If so I wonder what happens when systemd just delivers SIGKILL to > >>>>> all processes in the cgroup - I'm not sure there's a guarantee it > >>>>> will SIGKILL the main qemu before it SIGKILLs this helper > >>>> > >>>> I'm afraid in that case there is no guarantee. > >>>> > >>>> for what it's worth, both virsh shutdown and destroy seem to do things > >>>> properly. > >>> > >>> Hmm, probably because libvirt tells QEMU to exit before systemd comes > >>> along and tells everything in the cgroup to die with SIGKILL. > >> > >> It seems Libvirt sends SIGKILL if qemu process doesn't terminate within 10 > >> seconds after Libvirt sent SIGTERM: > >> > >> https://gitlab.com/libvirt/libvirt/-/blob/0615df084ec9996b5df88d6a1b59c557e22f3a12/src/util/virprocess.c#L375 > > > > but this is fine. > > > > with asynchronous teardown, qemu will exit almost immediately when > > receiving SIGTERM, and the cleanup process will start cleaning up. > > Under normal and orderly conditions, yes. > > >> So I guess this patch happened to work with Libvirt because the main qemu > >> process terminated before the timeout and before SIGKILL was delivered. > > > > it seems so > > > >> > >> The cleanup process is trying to solve the problem where the main qemu process > >> takes too long to terminate. However, if the cleanup process itself takes too > >> long, SIGKILL will be sent by Libvirt anyway. > > > > but that is not a problem, the sole purpose of the cleanup process is > > to terminate _after_ qemu. it doesn't matter what happens after qemu > > has terminated. if you look at the patch, after going to great lengths > > to assure that qemu has terminated, all the child process does is > > _exit(0). > > > >> > >> Perhaps we can describe this situation in the parameter help, e.g.: If > >> management layer decides to send SIGKILL (e.g.: due to timeout or deliberate > >> decision), the cleanup process can exit before the main process, deceiving its > >> purpose. > > > > if the management layer (or the user) decides to send SIGKILL > > immediately to the whole cgroup without sending SIGTERM first, then > > this whole asynchronous teardown mechanism is defeated, yes. > > This situation is what we likely want to describe in the parameter help. I don't > want to give users the false impression that this option will *always* behave > the manner we expect it to work *most* of the time. fair enough, I'll improve the documentation > > -- > Murilo
On Fri, Aug 12, 2022 at 09:26:23AM +0200, Claudio Imbrenda wrote: > On Thu, 11 Aug 2022 23:05:52 -0300 > Murilo Opsfelder Araújo <muriloo@linux.ibm.com> wrote: > > > On 8/11/22 11:02, Daniel P. Berrangé wrote: > > [...] > > >>> Hmm, I was hoping you could just use SIGKILL to guarantee that this > > >>> gets killed off. Is SIGKILL delivered too soon to allow for the > > >>> main QEMU process to have exited quickly ? > > >> > > >> yes, I tried. qemu has not finished exiting when the signal is > > >> delivered, the cleanup process dies before qemu, which defeats the > > >> purpose > > > > > > Ok, too bad. > > > > > >>> If so I wonder what happens when systemd just delivers SIGKILL to > > >>> all processes in the cgroup - I'm not sure there's a guarantee it > > >>> will SIGKILL the main qemu before it SIGKILLs this helper > > >> > > >> I'm afraid in that case there is no guarantee. > > >> > > >> for what it's worth, both virsh shutdown and destroy seem to do things > > >> properly. > > > > > > Hmm, probably because libvirt tells QEMU to exit before systemd comes > > > along and tells everything in the cgroup to die with SIGKILL. > > > > It seems Libvirt sends SIGKILL if qemu process doesn't terminate within 10 > > seconds after Libvirt sent SIGTERM: > > > > https://gitlab.com/libvirt/libvirt/-/blob/0615df084ec9996b5df88d6a1b59c557e22f3a12/src/util/virprocess.c#L375 > > but this is fine. > > with asynchronous teardown, qemu will exit almost immediately when > receiving SIGTERM, and the cleanup process will start cleaning up. Note, when you have PCI host devices attahced it can take a very long time for QEMU to exit. For this reason, the 10 second wait before switching to SIGKILL is extended by 2 seconds for each attachec PCI hostdev. I think the main time we will have problems is where there are storage failures that cause QEMU to get stuck in an uninterruptible sleep in kernel space. The classic example of this is an NFS server that goes away, QEMU will get stuck waiting for the NFS server to come back to life and be unkillable in this time even with SIGKILL. That said, this call to virProcessKillPainfully shouldn't impact the cleanmup process, becaused the SIGTERM/KILL are both directed to the QEMU PID alone, not the process group. The cleanup process should only get any signal later once libvirt has finished sending SIGTERM/KILL, it then asks systemd to cleanup the cgroups and at that time systemd can send SIGKILL to the cleanup process. So in fact I think we should be fine in all respects, except for the unkillable sleeps in kernel space. With regards, Daniel
Please excuse my late reply; I was on vacation. Daniel P. Berrangé <berrange@redhat.com> writes: > On Tue, Aug 09, 2022 at 08:40:24AM +0200, Claudio Imbrenda wrote: >> This patch adds support for asynchronously tearing down a VM on Linux. >> >> When qemu terminates, either naturally or because of a fatal signal, >> the VM is torn down. If the VM is huge, it can take a considerable >> amount of time for it to be cleaned up. In case of a protected VM, it >> might take even longer than a non-protected VM (this is the case on >> s390x, for example). >> >> Some users might want to shut down a VM and restart it immediately, >> without having to wait. This is especially true if management >> infrastructure like libvirt is used. >> >> This patch implements a simple trick on Linux to allow qemu to return >> immediately, with the teardown of the VM being performed >> asynchronously. >> >> If the new commandline option -async-teardown is used, a new process is >> spawned from qemu at startup, using the clone syscall, in such way that >> it will share its address space with qemu. >> >> The new process will have the name "cleanup/<QEMU_PID>". It will wait >> until qemu terminates, and then it will exit itself. >> >> This allows qemu to terminate quickly, without having to wait for the >> whole address space to be torn down. The teardown process will exit >> after qemu, so it will be the last user of the address space, and >> therefore it will take care of the actual teardown. >> >> The teardown process will share the same cgroups as qemu, so both >> memory usage and cpu time will be accounted properly. >> >> This feature can already be used with libvirt by adding the following >> to the XML domain definition to pass the parameter to qemu directly: >> >> <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0"> >> <arg value='-async-teardown'/> >> </commandline> >> >> More advanced interfaces like pidfd or close_range have intentionally >> been avoided in order to be more compatible with older kernels. >> >> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> [...] >> diff --git a/qemu-options.hx b/qemu-options.hx >> index 3f23a42fa8..d434353159 100644 >> --- a/qemu-options.hx >> +++ b/qemu-options.hx >> @@ -4743,6 +4743,23 @@ HXCOMM Internal use >> DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL) >> DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL) >> >> +#ifdef __linux__ >> +DEF("async-teardown", 0, QEMU_OPTION_asyncteardown, >> + "-async-teardown enable asynchronous teardown\n", >> + QEMU_ARCH_ALL) >> +#endif >> +SRST >> +``-async-teardown`` >> + Enable asynchronous teardown. A new teardown process will be >> + created at startup, using clone. The teardown process will share >> + the address space of the main qemu process, and wait for the main >> + process to terminate. At that point, the teardown process will >> + also exit. This allows qemu to terminate quickly if the guest was >> + huge, leaving the teardown of the address space to the teardown >> + process. Since the teardown process shares the same cgroups as the >> + main qemu process, accounting is performed correctly. >> +ERST >> + >> DEF("msg", HAS_ARG, QEMU_OPTION_msg, >> "-msg [timestamp[=on|off]][,guest-name=[on|off]]\n" >> " control error message format\n" > > It occurrs to me that we've got a general goal of getting away from > adding new top level command line arguments. Most of the time there's > an obvious existing place to put them, but I'm really not sure > where this particular option would fit ? > > it isn't tied to any aspect of the VM backend configuration nor > hardware frontends. > > The closest match is the lifecycle action option (-no-shutdown) > which were merged into a -action arg, but even that's a bit of a > stretch. If I understand the proposed new option correctly, it modifies how QEMU terminates, independent of why it terminates. Could be guest reboot with -action reboot-shutdown, monitor command quit, SIGTERM, ... I agree putting it under -action would be a bit of a stretch, as so far -action is entirely about configuring the reaction to guest certain actions: -action reboot=reset|shutdown action when guest reboots [default=reset] -action shutdown=poweroff|pause action when guest shuts down [default=poweroff] -action panic=pause|shutdown|exit-failure|none action when guest panics [default=shutdown] -action watchdog=reset|shutdown|poweroff|inject-nmi|pause|debug|none action when watchdog fires [default=reset] A different stretch: -daemonize, -runas, -chroot. These modify how QEMU starts. They too are "top-level". > Markus/Paolo: do you have suggestions ? Ramblings^WThoughts, not actionable suggestions, I'm afraid. [...]
diff --git a/include/qemu/async-teardown.h b/include/qemu/async-teardown.h new file mode 100644 index 0000000000..092e7a37e7 --- /dev/null +++ b/include/qemu/async-teardown.h @@ -0,0 +1,22 @@ +/* + * Asynchronous teardown + * + * Copyright IBM, Corp. 2022 + * + * Authors: + * Claudio Imbrenda <imbrenda@linux.ibm.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or (at your + * option) any later version. See the COPYING file in the top-level directory. + * + */ +#ifndef QEMU_ASYNC_TEARDOWN_H +#define QEMU_ASYNC_TEARDOWN_H + +#include "config-host.h" + +#ifdef CONFIG_LINUX +void init_async_teardown(void); +#endif + +#endif diff --git a/os-posix.c b/os-posix.c index 321fc4bd13..4858650c3e 100644 --- a/os-posix.c +++ b/os-posix.c @@ -39,6 +39,7 @@ #ifdef CONFIG_LINUX #include <sys/prctl.h> +#include "qemu/async-teardown.h" #endif /* @@ -150,6 +151,11 @@ int os_parse_cmd_args(int index, const char *optarg) case QEMU_OPTION_daemonize: daemonize = 1; break; +#if defined(CONFIG_LINUX) + case QEMU_OPTION_asyncteardown: + init_async_teardown(); + break; +#endif default: return -1; } diff --git a/qemu-options.hx b/qemu-options.hx index 3f23a42fa8..d434353159 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4743,6 +4743,23 @@ HXCOMM Internal use DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL) DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL) +#ifdef __linux__ +DEF("async-teardown", 0, QEMU_OPTION_asyncteardown, + "-async-teardown enable asynchronous teardown\n", + QEMU_ARCH_ALL) +#endif +SRST +``-async-teardown`` + Enable asynchronous teardown. A new teardown process will be + created at startup, using clone. The teardown process will share + the address space of the main qemu process, and wait for the main + process to terminate. At that point, the teardown process will + also exit. This allows qemu to terminate quickly if the guest was + huge, leaving the teardown of the address space to the teardown + process. Since the teardown process shares the same cgroups as the + main qemu process, accounting is performed correctly. +ERST + DEF("msg", HAS_ARG, QEMU_OPTION_msg, "-msg [timestamp[=on|off]][,guest-name=[on|off]]\n" " control error message format\n" diff --git a/util/async-teardown.c b/util/async-teardown.c new file mode 100644 index 0000000000..07fe549891 --- /dev/null +++ b/util/async-teardown.c @@ -0,0 +1,123 @@ +/* + * Asynchronous teardown + * + * Copyright IBM, Corp. 2022 + * + * Authors: + * Claudio Imbrenda <imbrenda@linux.ibm.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or (at your + * option) any later version. See the COPYING file in the top-level directory. + * + */ +#include <stdlib.h> +#include <stdio.h> +#include <sys/types.h> +#include <sys/unistd.h> +#include <dirent.h> +#include <sys/prctl.h> +#include <signal.h> +#include <sched.h> + +#include "qemu/async-teardown.h" + +static pid_t the_ppid; + +/* + * Close all open file descriptors. + */ +static void close_all_open_fd(void) +{ + struct dirent *de; + int fd, dfd; + DIR *dir; + + dir = opendir("/proc/self/fd"); + if (!dir) { + return; + } + /* Avoid closing the directory. */ + dfd = dirfd(dir); + + for (de = readdir(dir); de; de = readdir(dir)) { + fd = atoi(de->d_name); + if (fd != dfd) { + close(fd); + } + } + closedir(dir); +} + +static void hup_handler(int signal) +{ + /* Check every second if this process has been reparented. */ + while (the_ppid == getppid()) { + /* sleep() is safe to use in a signal handler. */ + sleep(1); + } + + /* At this point the parent process has terminated completely. */ + _exit(0); +} + +static int async_teardown_fn(void *arg) +{ + struct sigaction sa = { .sa_handler = hup_handler }; + sigset_t hup_signal; + char name[16]; + + /* Set a meaningful name for this process. */ + snprintf(name, 16, "cleanup/%d", the_ppid); + prctl(PR_SET_NAME, (unsigned long)name); + + /* + * Close all file descriptors that might have been inherited from the + * main qemu process when doing clone, needed to make libvirt happy. + * Not using close_range for increased compatibility with older kernels. + */ + close_all_open_fd(); + + /* Set up a handler for SIGHUP and unblock SIGHUP. */ + sigaction(SIGHUP, &sa, NULL); + sigemptyset(&hup_signal); + sigaddset(&hup_signal, SIGHUP); + sigprocmask(SIG_UNBLOCK, &hup_signal, NULL); + + /* Ask to receive SIGHUP when the parent dies. */ + prctl(PR_SET_PDEATHSIG, SIGHUP); + + /* + * Sleep forever, unless the parent process has already terminated. The + * only interruption can come from the SIGHUP signal, which in normal + * operation is received when the parent process dies. + */ + if (the_ppid == getppid()) { + pause(); + } + + /* At this point the parent process has terminated completely. */ + _exit(0); +} + +/* + * Block all signals, start (clone) a new process sharing the address space + * with qemu (CLONE_VM), then restore signals. + */ +void init_async_teardown(void) +{ + sigset_t all_signals, old_signals; + const int stack_size = 8192; /* Should be more than enough */ + char *stack, *stack_ptr; + + the_ppid = getpid(); + stack = malloc(stack_size); + if (!stack) { + return; + } + stack_ptr = stack + stack_size; + + sigfillset(&all_signals); + sigprocmask(SIG_BLOCK, &all_signals, &old_signals); + clone(async_teardown_fn, stack_ptr, CLONE_VM, NULL, NULL, NULL, NULL); + sigprocmask(SIG_SETMASK, &old_signals, NULL); +} diff --git a/util/meson.build b/util/meson.build index 5e282130df..63acd59bb0 100644 --- a/util/meson.build +++ b/util/meson.build @@ -2,6 +2,7 @@ util_ss.add(files('osdep.c', 'cutils.c', 'unicode.c', 'qemu-timer-common.c')) if not config_host_data.get('CONFIG_ATOMIC64') util_ss.add(files('atomic64.c')) endif +util_ss.add(when: 'CONFIG_LINUX', if_true: files('async-teardown.c')) util_ss.add(when: 'CONFIG_POSIX', if_true: files('aio-posix.c')) util_ss.add(when: 'CONFIG_POSIX', if_true: files('fdmon-poll.c')) if config_host_data.get('CONFIG_EPOLL_CREATE1')
This patch adds support for asynchronously tearing down a VM on Linux. When qemu terminates, either naturally or because of a fatal signal, the VM is torn down. If the VM is huge, it can take a considerable amount of time for it to be cleaned up. In case of a protected VM, it might take even longer than a non-protected VM (this is the case on s390x, for example). Some users might want to shut down a VM and restart it immediately, without having to wait. This is especially true if management infrastructure like libvirt is used. This patch implements a simple trick on Linux to allow qemu to return immediately, with the teardown of the VM being performed asynchronously. If the new commandline option -async-teardown is used, a new process is spawned from qemu at startup, using the clone syscall, in such way that it will share its address space with qemu. The new process will have the name "cleanup/<QEMU_PID>". It will wait until qemu terminates, and then it will exit itself. This allows qemu to terminate quickly, without having to wait for the whole address space to be torn down. The teardown process will exit after qemu, so it will be the last user of the address space, and therefore it will take care of the actual teardown. The teardown process will share the same cgroups as qemu, so both memory usage and cpu time will be accounted properly. This feature can already be used with libvirt by adding the following to the XML domain definition to pass the parameter to qemu directly: <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0"> <arg value='-async-teardown'/> </commandline> More advanced interfaces like pidfd or close_range have intentionally been avoided in order to be more compatible with older kernels. Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> --- include/qemu/async-teardown.h | 22 ++++++ os-posix.c | 6 ++ qemu-options.hx | 17 +++++ util/async-teardown.c | 123 ++++++++++++++++++++++++++++++++++ util/meson.build | 1 + 5 files changed, 169 insertions(+) create mode 100644 include/qemu/async-teardown.h create mode 100644 util/async-teardown.c