diff mbox series

[v1,1/1] osdep: asynchronous teardown for shutdown on Linux

Message ID 20211206110611.27283-1-imbrenda@linux.ibm.com
State New
Headers show
Series [v1,1/1] osdep: asynchronous teardown for shutdown on Linux | expand

Commit Message

Claudio Imbrenda Dec. 6, 2021, 11:06 a.m. UTC
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 using the clone syscall, so that it will share its
address space with qemu.

The new process will then 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.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 include/qemu/osdep.h |  2 ++
 os-posix.c           |  3 +++
 qemu-options.hx      | 17 ++++++++++++++++
 util/osdep.c         | 47 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 69 insertions(+)

Comments

Daniel P. Berrangé Dec. 6, 2021, 11:21 a.m. UTC | #1
On Mon, Dec 06, 2021 at 12:06:11PM +0100, 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 using the clone syscall, so that it will share its
> address space with qemu.
> 
> The new process will then 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.

If this suggested workaround has any benefit to the shutdown of a VM
with libvirt, then it is a bug in libvirt IMHO.

When libvirt tears down a QEMU VM, it should be waiting for *every*
process in the VM's cgroup to be terminated before it reports that
the VM is shutoff. IOW, the fact that this workaround lets the main
QEMU process exit quickly should not matter. libvirt should still
be blocked in exactly the same place in its code, waiting for the
"async" cleanup process to exit. IOW, this should not be async at
all from libvirt's POV.


Regards,
Daniel
Claudio Imbrenda Dec. 6, 2021, 11:43 a.m. UTC | #2
On Mon, 6 Dec 2021 11:21:10 +0000
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Mon, Dec 06, 2021 at 12:06:11PM +0100, 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 using the clone syscall, so that it will share its
> > address space with qemu.
> > 
> > The new process will then 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.  
> 
> If this suggested workaround has any benefit to the shutdown of a VM
> with libvirt, then it is a bug in libvirt IMHO.
> 
> When libvirt tears down a QEMU VM, it should be waiting for *every*
> process in the VM's cgroup to be terminated before it reports that
> the VM is shutoff. IOW, the fact that this workaround lets the main
> QEMU process exit quickly should not matter. libvirt should still
> be blocked in exactly the same place in its code, waiting for the
> "async" cleanup process to exit. IOW, this should not be async at
> all from libvirt's POV.

interesting, I did not know that about libvirt.

maybe libvirt could be fixed/improved to allow this patch to work?

surely without this patch an asynchronous teardown will not be possible
at all

> 
> 
> Regards,
> Daniel
Daniel P. Berrangé Dec. 6, 2021, 11:47 a.m. UTC | #3
On Mon, Dec 06, 2021 at 12:43:12PM +0100, Claudio Imbrenda wrote:
> On Mon, 6 Dec 2021 11:21:10 +0000
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> > On Mon, Dec 06, 2021 at 12:06:11PM +0100, 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 using the clone syscall, so that it will share its
> > > address space with qemu.
> > > 
> > > The new process will then 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.  
> > 
> > If this suggested workaround has any benefit to the shutdown of a VM
> > with libvirt, then it is a bug in libvirt IMHO.
> > 
> > When libvirt tears down a QEMU VM, it should be waiting for *every*
> > process in the VM's cgroup to be terminated before it reports that
> > the VM is shutoff. IOW, the fact that this workaround lets the main
> > QEMU process exit quickly should not matter. libvirt should still
> > be blocked in exactly the same place in its code, waiting for the
> > "async" cleanup process to exit. IOW, this should not be async at
> > all from libvirt's POV.
> 
> interesting, I did not know that about libvirt.
> 
> maybe libvirt could be fixed/improved to allow this patch to work?

That would not be desirable. When libvirt reports a VM as shutoff,
it is expected that all resources associated with the VM huave been
fully released, such that they are available for launching a new
VM.  We can't allow resources to be asynchronously released as that
violates app's expectation that the resources are released once the
VM is shutoff.

> surely without this patch an asynchronous teardown will not be possible
> at all

I appreciate that the current slow teardown is a pain, but async
teardown does not sound like an appealing alternative given that
the app can't use the resources again until the teardown is
complete.

Regards,
Daniel
Claudio Imbrenda Dec. 6, 2021, 12:15 p.m. UTC | #4
On Mon, 6 Dec 2021 11:47:55 +0000
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Mon, Dec 06, 2021 at 12:43:12PM +0100, Claudio Imbrenda wrote:
> > On Mon, 6 Dec 2021 11:21:10 +0000
> > Daniel P. Berrangé <berrange@redhat.com> wrote:
> >   
> > > On Mon, Dec 06, 2021 at 12:06:11PM +0100, 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 using the clone syscall, so that it will share its
> > > > address space with qemu.
> > > > 
> > > > The new process will then 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.    
> > > 
> > > If this suggested workaround has any benefit to the shutdown of a VM
> > > with libvirt, then it is a bug in libvirt IMHO.
> > > 
> > > When libvirt tears down a QEMU VM, it should be waiting for *every*
> > > process in the VM's cgroup to be terminated before it reports that
> > > the VM is shutoff. IOW, the fact that this workaround lets the main
> > > QEMU process exit quickly should not matter. libvirt should still
> > > be blocked in exactly the same place in its code, waiting for the
> > > "async" cleanup process to exit. IOW, this should not be async at
> > > all from libvirt's POV.  
> > 
> > interesting, I did not know that about libvirt.
> > 
> > maybe libvirt could be fixed/improved to allow this patch to work?  
> 
> That would not be desirable. When libvirt reports a VM as shutoff,
> it is expected that all resources associated with the VM huave been
> fully released, such that they are available for launching a new
> VM.  We can't allow resources to be asynchronously released as that
> violates app's expectation that the resources are released once the
> VM is shutoff.

what about people who do not use libvirt? should those also be
prevented from taking advantage of this feature only because libvirt
can't use it?

> 
> > surely without this patch an asynchronous teardown will not be possible
> > at all  
> 
> I appreciate that the current slow teardown is a pain, but async
> teardown does not sound like an appealing alternative given that
> the app can't use the resources again until the teardown is
> complete.

when a VM starts, it will not use all of the memory at once. it will
start using it a little at a time. time during which the asynchronous
process can complete the teardown.

consider what would happen if you need to shut down and restart a big
VM (>10TB)

> 
> Regards,
> Daniel
Daniel P. Berrangé Dec. 6, 2021, 12:27 p.m. UTC | #5
On Mon, Dec 06, 2021 at 01:15:14PM +0100, Claudio Imbrenda wrote:
> On Mon, 6 Dec 2021 11:47:55 +0000
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> > On Mon, Dec 06, 2021 at 12:43:12PM +0100, Claudio Imbrenda wrote:
> > > On Mon, 6 Dec 2021 11:21:10 +0000
> > > Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >   
> > > > On Mon, Dec 06, 2021 at 12:06:11PM +0100, 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 using the clone syscall, so that it will share its
> > > > > address space with qemu.
> > > > > 
> > > > > The new process will then 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.    
> > > > 
> > > > If this suggested workaround has any benefit to the shutdown of a VM
> > > > with libvirt, then it is a bug in libvirt IMHO.
> > > > 
> > > > When libvirt tears down a QEMU VM, it should be waiting for *every*
> > > > process in the VM's cgroup to be terminated before it reports that
> > > > the VM is shutoff. IOW, the fact that this workaround lets the main
> > > > QEMU process exit quickly should not matter. libvirt should still
> > > > be blocked in exactly the same place in its code, waiting for the
> > > > "async" cleanup process to exit. IOW, this should not be async at
> > > > all from libvirt's POV.  
> > > 
> > > interesting, I did not know that about libvirt.
> > > 
> > > maybe libvirt could be fixed/improved to allow this patch to work?  
> > 
> > That would not be desirable. When libvirt reports a VM as shutoff,
> > it is expected that all resources associated with the VM huave been
> > fully released, such that they are available for launching a new
> > VM.  We can't allow resources to be asynchronously released as that
> > violates app's expectation that the resources are released once the
> > VM is shutoff.
> 
> what about people who do not use libvirt? should those also be
> prevented from taking advantage of this feature only because libvirt
> can't use it?

Do we have any other mgmt tools this that won't ultimately have the
same issue ?  I'd expect the same to apply to anything that is using
cgroups for managing resources used by a QEMU process at least.

> > > surely without this patch an asynchronous teardown will not be possible
> > > at all  
> > 
> > I appreciate that the current slow teardown is a pain, but async
> > teardown does not sound like an appealing alternative given that
> > the app can't use the resources again until the teardown is
> > complete.
> 
> when a VM starts, it will not use all of the memory at once. it will
> start using it a little at a time. time during which the asynchronous
> process can complete the teardown.

How quickly it uses memory will depend on various factors. If it tries
to use more memory before the async cleanup has released enough, then
this looks like it risks putting the host into overcommit / OOM killer
scenarios surely ?

Regards,
Daniel
Halil Pasic Dec. 7, 2021, 2:59 p.m. UTC | #6
On Mon, 6 Dec 2021 11:47:55 +0000
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Mon, Dec 06, 2021 at 12:43:12PM +0100, Claudio Imbrenda wrote:
> > On Mon, 6 Dec 2021 11:21:10 +0000
> > Daniel P. Berrangé <berrange@redhat.com> wrote:
> >   
> > > On Mon, Dec 06, 2021 at 12:06:11PM +0100, 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 using the clone syscall, so that it will share its
> > > > address space with qemu.
> > > > 
> > > > The new process will then 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.    
> > > 
> > > If this suggested workaround has any benefit to the shutdown of a VM
> > > with libvirt, then it is a bug in libvirt IMHO.
> > > 
> > > When libvirt tears down a QEMU VM, it should be waiting for *every*
> > > process in the VM's cgroup to be terminated before it reports that
> > > the VM is shutoff. IOW, the fact that this workaround lets the main
> > > QEMU process exit quickly should not matter. libvirt should still
> > > be blocked in exactly the same place in its code, waiting for the
> > > "async" cleanup process to exit. IOW, this should not be async at
> > > all from libvirt's POV.  
> > 
> > interesting, I did not know that about libvirt.
> > 
> > maybe libvirt could be fixed/improved to allow this patch to work?  
> 
> That would not be desirable. When libvirt reports a VM as shutoff,
> it is expected that all resources associated with the VM huave been
> fully released, such that they are available for launching a new
> VM.  We can't allow resources to be asynchronously released as that
> violates app's expectation that the resources are released once the
> VM is shutoff.

I do see your point. But I believe, a part of the problem is that
currently 'can start VM again' and 'all resources associated with
the previous run of the VM were cleaned up' are tied together. And
intuitively it makes a ton of sense. It is just that due to certain
reasons complete shutdown with complete cleanup takes way too long. So
we are looking for a solution to decouple the two. I believe complete
cleanup is inherently hard, so we should not hope solving that. Do you
agree?

Under the assumption that we won't be able to make the cleanup (making
all the resources available again) fast enough, I believe the only way
forward is coming up with a solution if the user explicitly says so
the assumption you just laid out should not be justified any  more.
Maybe something like enlightening the management software about this
'non-interchangeable resources are released, interchangeable resources
not yet fully released' state and add something like a 'force-start' 
operation, where the user explicitly opts-into potentially consuming more
resources (because of the overlap) for less downtime.

What do you think?

> 
> > surely without this patch an asynchronous teardown will not be possible
> > at all  
> 
> I appreciate that the current slow teardown is a pain, but async
> teardown does not sound like an appealing alternative given that
> the app can't use the resources again until the teardown is
> complete.

I don't fully agree with this. I think this statement disregards that
some resources are non-interchangeable in a sense that we need the exact
same resource free, while other resources are interchangeable in a sense
that we don't care which instance we get as long as we get enough
instances from a certain class. When we stop a VM and then start the same
VM again, we don't expect to get the same chunks of memory we had before,
but we just allocate new memory.

Yes we may run into trouble, but we may not. As long as we don't just
change this under the hood, but make it an option that somebody has
to consciously choose to enable, I think we are fine.

Regards,
Halil
diff mbox series

Patch

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 60718fc342..f5493c9489 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -657,6 +657,8 @@  const char *qemu_hw_version(void);
 void fips_set_state(bool requested);
 bool fips_get_state(void);
 
+void init_async_teardown(void);
+
 /* Return a dynamically allocated pathname denoting a file or directory that is
  * appropriate for storing local state.
  *
diff --git a/os-posix.c b/os-posix.c
index ae6c9f2a5e..d37a654e2c 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -158,6 +158,9 @@  int os_parse_cmd_args(int index, const char *optarg)
                     "to enable FIPS compliance");
         fips_set_state(true);
         break;
+    case QEMU_OPTION_asyncteardown:
+        init_async_teardown();
+        break;
 #endif
     default:
         return -1;
diff --git a/qemu-options.hx b/qemu-options.hx
index ae2c6dbbfc..b4ce828726 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4600,6 +4600,23 @@  SRST
     Enable FIPS 140-2 compliance mode.
 ERST
 
+#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/osdep.c b/util/osdep.c
index 42a0a4986a..f36be51262 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -33,6 +33,14 @@ 
 extern int madvise(char *, size_t, int);
 #endif
 
+#ifdef CONFIG_LINUX
+#include <sys/types.h>
+#include <sys/select.h>
+#include <sys/unistd.h>
+#include <sys/syscall.h>
+#include <signal.h>
+#endif
+
 #include "qemu-common.h"
 #include "qemu/cutils.h"
 #include "qemu/sockets.h"
@@ -548,6 +556,45 @@  bool fips_get_state(void)
     return fips_enabled;
 }
 
+#ifdef __linux__
+static int async_teardown_fn(void *arg)
+{
+    sigset_t all_signals;
+    fd_set r, w, e;
+    int fd;
+
+    /* open a pidfd descriptor for the parent qemu process */
+    fd = syscall(__NR_pidfd_open, getppid(), 0);
+    /* if something went wrong, or if the file descriptor is too big */
+    if ((fd < 0) || (fd >= FD_SETSIZE)) {
+        _exit(1);
+    }
+    /* zero all fd sets */
+    FD_ZERO(&r);
+    FD_ZERO(&w);
+    FD_ZERO(&e);
+    /* set the fd for the pidfd in the "read" set */
+    FD_SET(fd, &r);
+    /* block all signals */
+    sigfillset(&all_signals);
+    sigprocmask(SIG_BLOCK, &all_signals, NULL);
+    /* wait for the pid to disappear -> fd will appear as ready for read */
+    (void) select(fd + 1, &r, &w, &e, NULL);
+    _exit(0);
+}
+
+void init_async_teardown(void)
+{
+    const int size = 8192; /* should be more than enough */
+    char *stack = malloc(size);
+
+    /* start a new process sharing the address space with qemu */
+    clone(async_teardown_fn, stack + size, CLONE_VM, NULL, NULL, NULL, NULL);
+}
+#else /* __linux__ */
+void init_async_teardown(void) {}
+#endif
+
 #ifdef _WIN32
 static void socket_cleanup(void)
 {