All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux
@ 2022-08-03 17:31 Claudio Imbrenda
  2022-08-03 17:34 ` Daniel P. Berrangé
  2022-08-04 16:56 ` Daniel P. Berrangé
  0 siblings, 2 replies; 12+ messages in thread
From: Claudio Imbrenda @ 2022-08-03 17:31 UTC (permalink / raw)
  To: pbonzini
  Cc: qemu-devel, david, cohuck, thuth, borntraeger, frankja, berrange,
	alex.bennee

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 then simpy 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:

  <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0">
  <arg value='-async-teardown'/>
  </commandline>

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

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index b1c161c035..3154759d79 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -549,6 +549,8 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
 
 void qemu_set_cloexec(int fd);
 
+void init_async_teardown(void);
+
 /* Return a dynamically allocated directory path that is appropriate for storing
  * local state.
  *
diff --git a/os-posix.c b/os-posix.c
index 321fc4bd13..dd3e42b4c4 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -150,6 +150,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/osdep.c b/util/osdep.c
index 60fcbbaebe..bb0baf97a0 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -23,6 +23,15 @@
  */
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+
+#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/cutils.h"
 #include "qemu/sockets.h"
 #include "qemu/error-report.h"
@@ -512,6 +521,52 @@ const char *qemu_hw_version(void)
     return hw_version;
 }
 
+#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);
+
+    /*
+     * Close all file descriptors that might have been inherited from the
+     * main qemu process when doing clone. This is needed to make libvirt
+     * happy.
+     */
+    close_range(0, ~0U, 0);
+    _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)
 {
-- 
2.37.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux
  2022-08-03 17:31 [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux Claudio Imbrenda
@ 2022-08-03 17:34 ` Daniel P. Berrangé
  2022-08-04  5:56   ` Claudio Imbrenda
  2022-08-04  8:29   ` Daniel P. Berrangé
  2022-08-04 16:56 ` Daniel P. Berrangé
  1 sibling, 2 replies; 12+ messages in thread
From: Daniel P. Berrangé @ 2022-08-03 17:34 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: pbonzini, qemu-devel, david, cohuck, thuth, borntraeger, frankja,
	alex.bennee

On Wed, Aug 03, 2022 at 07:31:41PM +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 then simpy 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:
> 
>   <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0">
>   <arg value='-async-teardown'/>
>   </commandline>

How does this work in practice ?  Libvirt should be blocking until
all processes in the cgroup have exited, including this cloned
child process.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux
  2022-08-03 17:34 ` Daniel P. Berrangé
@ 2022-08-04  5:56   ` Claudio Imbrenda
  2022-08-04  8:20     ` Daniel P. Berrangé
  2022-08-04  8:29   ` Daniel P. Berrangé
  1 sibling, 1 reply; 12+ messages in thread
From: Claudio Imbrenda @ 2022-08-04  5:56 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: pbonzini, qemu-devel, david, cohuck, thuth, borntraeger, frankja,
	alex.bennee

On Wed, 3 Aug 2022 18:34:45 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Wed, Aug 03, 2022 at 07:31:41PM +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 then simpy 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:
> > 
> >   <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0">
> >   <arg value='-async-teardown'/>
> >   </commandline>  
> 
> How does this work in practice ?  Libvirt should be blocking until

I don't know the inner details of how libvirt works..

> all processes in the cgroup have exited, including this cloned
> child process.

..but I tested it and it works

my impression is that libvirt by default is only waiting for the
main qemu process.

the only issue I have found is the log file, which stays open as long
as some file descriptors (which the cloned process inherits from the
main qemu process) stay open. A new VM cannot be started if its log file
is still open by the logger process. The close_range() call solves the
issue.

> 
> With regards,
> Daniel



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux
  2022-08-04  5:56   ` Claudio Imbrenda
@ 2022-08-04  8:20     ` Daniel P. Berrangé
  2022-08-04 16:58       ` Daniel P. Berrangé
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel P. Berrangé @ 2022-08-04  8:20 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: pbonzini, qemu-devel, david, cohuck, thuth, borntraeger, frankja,
	alex.bennee

On Thu, Aug 04, 2022 at 07:56:49AM +0200, Claudio Imbrenda wrote:
> On Wed, 3 Aug 2022 18:34:45 +0100
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> > On Wed, Aug 03, 2022 at 07:31:41PM +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 then simpy 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:
> > > 
> > >   <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0">
> > >   <arg value='-async-teardown'/>
> > >   </commandline>  
> > 
> > How does this work in practice ?  Libvirt should be blocking until
> 
> I don't know the inner details of how libvirt works..
> 
> > all processes in the cgroup have exited, including this cloned
> > child process.
> 
> ..but I tested it and it works
> 
> my impression is that libvirt by default is only waiting for the
> main qemu process.

If true, that would be a bug that needs fixing and should not be
relied on.

> the only issue I have found is the log file, which stays open as long
> as some file descriptors (which the cloned process inherits from the
> main qemu process) stay open. A new VM cannot be started if its log file
> is still open by the logger process. The close_range() call solves the
> issue.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux
  2022-08-03 17:34 ` Daniel P. Berrangé
  2022-08-04  5:56   ` Claudio Imbrenda
@ 2022-08-04  8:29   ` Daniel P. Berrangé
  2022-08-04 14:49     ` Claudio Imbrenda
  1 sibling, 1 reply; 12+ messages in thread
From: Daniel P. Berrangé @ 2022-08-04  8:29 UTC (permalink / raw)
  To: Claudio Imbrenda, pbonzini, qemu-devel, david, cohuck, thuth,
	borntraeger, frankja, alex.bennee

On Wed, Aug 03, 2022 at 06:34:45PM +0100, Daniel P. Berrangé wrote:
> On Wed, Aug 03, 2022 at 07:31:41PM +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 then simpy 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:
> > 
> >   <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0">
> >   <arg value='-async-teardown'/>
> >   </commandline>
> 
> How does this work in practice ?  Libvirt should be blocking until
> all processes in the cgroup have exited, including this cloned
> child process.

Also, have you disabled use of seccomp with QEMU when testing this,
as the seccomp filter that libivrt enables is supposed to block
any use of clone() except for the creation of threads.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux
  2022-08-04  8:29   ` Daniel P. Berrangé
@ 2022-08-04 14:49     ` Claudio Imbrenda
  2022-08-04 16:41       ` Daniel P. Berrangé
  0 siblings, 1 reply; 12+ messages in thread
From: Claudio Imbrenda @ 2022-08-04 14:49 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: pbonzini, qemu-devel, david, cohuck, thuth, borntraeger, frankja,
	alex.bennee

On Thu, 4 Aug 2022 09:29:39 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Wed, Aug 03, 2022 at 06:34:45PM +0100, Daniel P. Berrangé wrote:
> > On Wed, Aug 03, 2022 at 07:31:41PM +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 then simpy 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:
> > > 
> > >   <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0">
> > >   <arg value='-async-teardown'/>
> > >   </commandline>  
> > 
> > How does this work in practice ?  Libvirt should be blocking until
> > all processes in the cgroup have exited, including this cloned
> > child process.  
> 
> Also, have you disabled use of seccomp with QEMU when testing this,
> as the seccomp filter that libivrt enables is supposed to block
> any use of clone() except for the creation of threads.

it was just a vanilla libvirt 8.0.0 as found on ubuntu 22.04; I have no
idea how it is configured by default

> 
> With regards,
> Daniel



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux
  2022-08-04 14:49     ` Claudio Imbrenda
@ 2022-08-04 16:41       ` Daniel P. Berrangé
  2022-08-05  6:59         ` Claudio Imbrenda
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel P. Berrangé @ 2022-08-04 16:41 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: pbonzini, qemu-devel, david, cohuck, thuth, borntraeger, frankja,
	alex.bennee

On Thu, Aug 04, 2022 at 04:49:29PM +0200, Claudio Imbrenda wrote:
> On Thu, 4 Aug 2022 09:29:39 +0100
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> > On Wed, Aug 03, 2022 at 06:34:45PM +0100, Daniel P. Berrangé wrote:
> > > On Wed, Aug 03, 2022 at 07:31:41PM +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 then simpy 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:
> > > > 
> > > >   <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0">
> > > >   <arg value='-async-teardown'/>
> > > >   </commandline>  
> > > 
> > > How does this work in practice ?  Libvirt should be blocking until
> > > all processes in the cgroup have exited, including this cloned
> > > child process.  
> > 
> > Also, have you disabled use of seccomp with QEMU when testing this,
> > as the seccomp filter that libivrt enables is supposed to block
> > any use of clone() except for the creation of threads.
> 
> it was just a vanilla libvirt 8.0.0 as found on ubuntu 22.04; I have no
> idea how it is configured by default

Ok, so the reason it is working is because the extra process is
cloned() right in middle of processing argv. This is before the
seccomp filter is applied to the process, so clone() is not blocked.

One think I note about this in practice is that (unsurprisingly)
if you do a process listing, users now see 2 QEMU processes instead
of one.

I wonder if we should consider overwriting argv in the child
process with "[qemu async teardown]" to give users a hint as to
why this duplicate process exists.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux
  2022-08-03 17:31 [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux Claudio Imbrenda
  2022-08-03 17:34 ` Daniel P. Berrangé
@ 2022-08-04 16:56 ` Daniel P. Berrangé
  2022-08-05  7:32   ` Claudio Imbrenda
  1 sibling, 1 reply; 12+ messages in thread
From: Daniel P. Berrangé @ 2022-08-04 16:56 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: pbonzini, qemu-devel, david, cohuck, thuth, borntraeger, frankja,
	alex.bennee

On Wed, Aug 03, 2022 at 07:31:41PM +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 then simpy 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:
> 
>   <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0">
>   <arg value='-async-teardown'/>
>   </commandline>
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>  include/qemu/osdep.h |  2 ++
>  os-posix.c           |  5 ++++
>  qemu-options.hx      | 17 ++++++++++++++
>  util/osdep.c         | 55 ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 79 insertions(+)


> diff --git a/util/osdep.c b/util/osdep.c
> index 60fcbbaebe..bb0baf97a0 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c
> @@ -23,6 +23,15 @@
>   */
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
> +
> +#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/cutils.h"
>  #include "qemu/sockets.h"
>  #include "qemu/error-report.h"
> @@ -512,6 +521,52 @@ const char *qemu_hw_version(void)
>      return hw_version;
>  }
>  
> +#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);

We ought to open this FD in the parent process to avoid a race
where the parent crashes immediately after clone() and gets
reparented to 'init' before this child process calls pidfd_open,
otherwise it'll sit around waiting for init to exit.

> +    /* 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);

Technnically this is racy as there's still a window in which the
child is running when signals are not blocked.

> +    /* wait for the pid to disappear -> fd will appear as ready for read */
> +    (void) select(fd + 1, &r, &w, &e, NULL);

While using pidfd can work, a stronger protection would be to use

   prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);

this guarantees that the kernel will deliver SIGKILL to this
process immediately when the parent QEMU exits.

We should probably do both in fact.

> +
> +    /*
> +     * Close all file descriptors that might have been inherited from the
> +     * main qemu process when doing clone. This is needed to make libvirt
> +     * happy.
> +     */
> +    close_range(0, ~0U, 0);

Shouldn't we close all the FDs immediately when this process is
created, rather than only at the end when we're exiting. I don't
see there's any need to keep them open. Doing it immediately
would be better when using prctl(PR_SET_PDEATHSIG)

> +    _exit(0);
> +}
> +
> +void init_async_teardown(void)
> +{
> +    const int size = 8192; /* should be more than enough */
> +    char *stack = malloc(size);
> +

You need to block all signals here.

> +    /* start a new process sharing the address space with qemu */
> +    clone(async_teardown_fn, stack + size, CLONE_VM, NULL, NULL, NULL, NULL);

And unblock signals again here.

That way the "everything blocked"  mask is inherited by the child
from the very first moment of its existance.

> +}
> +#else /* __linux__ */
> +void init_async_teardown(void) {}
> +#endif
> +
>  #ifdef _WIN32
>  static void socket_cleanup(void)
>  {
> -- 
> 2.37.1
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux
  2022-08-04  8:20     ` Daniel P. Berrangé
@ 2022-08-04 16:58       ` Daniel P. Berrangé
  2022-08-05  7:02         ` Claudio Imbrenda
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel P. Berrangé @ 2022-08-04 16:58 UTC (permalink / raw)
  To: Claudio Imbrenda, pbonzini, qemu-devel, david, cohuck, thuth,
	borntraeger, frankja, alex.bennee

On Thu, Aug 04, 2022 at 09:20:59AM +0100, Daniel P. Berrangé wrote:
> On Thu, Aug 04, 2022 at 07:56:49AM +0200, Claudio Imbrenda wrote:
> > On Wed, 3 Aug 2022 18:34:45 +0100
> > Daniel P. Berrangé <berrange@redhat.com> wrote:
> > 
> > > On Wed, Aug 03, 2022 at 07:31:41PM +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 then simpy 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:
> > > > 
> > > >   <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0">
> > > >   <arg value='-async-teardown'/>
> > > >   </commandline>  
> > > 
> > > How does this work in practice ?  Libvirt should be blocking until
> > 
> > I don't know the inner details of how libvirt works..
> > 
> > > all processes in the cgroup have exited, including this cloned
> > > child process.
> > 
> > ..but I tested it and it works
> > 
> > my impression is that libvirt by default is only waiting for the
> > main qemu process.
> 
> If true, that would be a bug that needs fixing and should not be
> relied on.

Libvirt is invoking 'TerminateMachine' DBus call on systemd-machined.
That in turn iterates over every process in the cgroup and kills
them off.

Docs are a little vague and I've not followed the code perfectly, but
that should mean TeminateMachine doesnt return until every process in
the cgroup has exited.

That said, since this is a dbus API call, libvirt will probably
timeout waiting for the DBus reply after something like 30-60
seconds IIRC.

> 
> > the only issue I have found is the log file, which stays open as long
> > as some file descriptors (which the cloned process inherits from the
> > main qemu process) stay open. A new VM cannot be started if its log file
> > is still open by the logger process. The close_range() call solves the
> > issue.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux
  2022-08-04 16:41       ` Daniel P. Berrangé
@ 2022-08-05  6:59         ` Claudio Imbrenda
  0 siblings, 0 replies; 12+ messages in thread
From: Claudio Imbrenda @ 2022-08-05  6:59 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: pbonzini, qemu-devel, david, cohuck, thuth, borntraeger, frankja,
	alex.bennee

On Thu, 4 Aug 2022 17:41:01 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Thu, Aug 04, 2022 at 04:49:29PM +0200, Claudio Imbrenda wrote:
> > On Thu, 4 Aug 2022 09:29:39 +0100
> > Daniel P. Berrangé <berrange@redhat.com> wrote:
> >   
> > > On Wed, Aug 03, 2022 at 06:34:45PM +0100, Daniel P. Berrangé wrote:  
> > > > On Wed, Aug 03, 2022 at 07:31:41PM +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 then simpy 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:
> > > > > 
> > > > >   <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0">
> > > > >   <arg value='-async-teardown'/>
> > > > >   </commandline>    
> > > > 
> > > > How does this work in practice ?  Libvirt should be blocking until
> > > > all processes in the cgroup have exited, including this cloned
> > > > child process.    
> > > 
> > > Also, have you disabled use of seccomp with QEMU when testing this,
> > > as the seccomp filter that libivrt enables is supposed to block
> > > any use of clone() except for the creation of threads.  
> > 
> > it was just a vanilla libvirt 8.0.0 as found on ubuntu 22.04; I have no
> > idea how it is configured by default  
> 
> Ok, so the reason it is working is because the extra process is
> cloned() right in middle of processing argv. This is before the
> seccomp filter is applied to the process, so clone() is not blocked.
> 
> One think I note about this in practice is that (unsurprisingly)
> if you do a process listing, users now see 2 QEMU processes instead
> of one.
> 
> I wonder if we should consider overwriting argv in the child
> process with "[qemu async teardown]" to give users a hint as to
> why this duplicate process exists.

sounds like a good idea

> 
> With regards,
> Daniel



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux
  2022-08-04 16:58       ` Daniel P. Berrangé
@ 2022-08-05  7:02         ` Claudio Imbrenda
  0 siblings, 0 replies; 12+ messages in thread
From: Claudio Imbrenda @ 2022-08-05  7:02 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: pbonzini, qemu-devel, david, cohuck, thuth, borntraeger, frankja,
	alex.bennee

On Thu, 4 Aug 2022 17:58:34 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Thu, Aug 04, 2022 at 09:20:59AM +0100, Daniel P. Berrangé wrote:
> > On Thu, Aug 04, 2022 at 07:56:49AM +0200, Claudio Imbrenda wrote:  
> > > On Wed, 3 Aug 2022 18:34:45 +0100
> > > Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >   
> > > > On Wed, Aug 03, 2022 at 07:31:41PM +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 then simpy 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:
> > > > > 
> > > > >   <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0">
> > > > >   <arg value='-async-teardown'/>
> > > > >   </commandline>    
> > > > 
> > > > How does this work in practice ?  Libvirt should be blocking until  
> > > 
> > > I don't know the inner details of how libvirt works..
> > >   
> > > > all processes in the cgroup have exited, including this cloned
> > > > child process.  
> > > 
> > > ..but I tested it and it works
> > > 
> > > my impression is that libvirt by default is only waiting for the
> > > main qemu process.  
> > 
> > If true, that would be a bug that needs fixing and should not be
> > relied on.  
> 
> Libvirt is invoking 'TerminateMachine' DBus call on systemd-machined.
> That in turn iterates over every process in the cgroup and kills
> them off.
> 
> Docs are a little vague and I've not followed the code perfectly, but
> that should mean TeminateMachine doesnt return until every process in
> the cgroup has exited.
> 
> That said, since this is a dbus API call, libvirt will probably
> timeout waiting for the DBus reply after something like 30-60
> seconds IIRC.

I have not observed any delays.

could it be that DBus doesn't wait for the process to be completely
dead, but only that the signal is delivered?

and which signal will DBus use?

> 
> >   
> > > the only issue I have found is the log file, which stays open as long
> > > as some file descriptors (which the cloned process inherits from the
> > > main qemu process) stay open. A new VM cannot be started if its log file
> > > is still open by the logger process. The close_range() call solves the
> > > issue.  
> 
> With regards,
> Daniel



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux
  2022-08-04 16:56 ` Daniel P. Berrangé
@ 2022-08-05  7:32   ` Claudio Imbrenda
  0 siblings, 0 replies; 12+ messages in thread
From: Claudio Imbrenda @ 2022-08-05  7:32 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: pbonzini, qemu-devel, david, cohuck, thuth, borntraeger, frankja,
	alex.bennee

On Thu, 4 Aug 2022 17:56:34 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Wed, Aug 03, 2022 at 07:31:41PM +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 then simpy 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:
> > 
> >   <commandline xmlns="http://libvirt.org/schemas/domain/qemu/1.0">
> >   <arg value='-async-teardown'/>
> >   </commandline>
> > 
> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > ---
> >  include/qemu/osdep.h |  2 ++
> >  os-posix.c           |  5 ++++
> >  qemu-options.hx      | 17 ++++++++++++++
> >  util/osdep.c         | 55 ++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 79 insertions(+)  
> 
> 
> > diff --git a/util/osdep.c b/util/osdep.c
> > index 60fcbbaebe..bb0baf97a0 100644
> > --- a/util/osdep.c
> > +++ b/util/osdep.c
> > @@ -23,6 +23,15 @@
> >   */
> >  #include "qemu/osdep.h"
> >  #include "qapi/error.h"
> > +
> > +#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/cutils.h"
> >  #include "qemu/sockets.h"
> >  #include "qemu/error-report.h"
> > @@ -512,6 +521,52 @@ const char *qemu_hw_version(void)
> >      return hw_version;
> >  }
> >  
> > +#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);  
> 
> We ought to open this FD in the parent process to avoid a race
> where the parent crashes immediately after clone() and gets
> reparented to 'init' before this child process calls pidfd_open,
> otherwise it'll sit around waiting for init to exit.

sounds good

> 
> > +    /* 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);  
> 
> Technnically this is racy as there's still a window in which the
> child is running when signals are not blocked.

true, I will apply the workaround you suggest below

> 
> > +    /* wait for the pid to disappear -> fd will appear as ready for read */
> > +    (void) select(fd + 1, &r, &w, &e, NULL);  
> 
> While using pidfd can work, a stronger protection would be to use
> 
>    prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);
> 
> this guarantees that the kernel will deliver SIGKILL to this
> process immediately when the parent QEMU exits.
> 
> We should probably do both in fact.

makes sense

> 
> > +
> > +    /*
> > +     * Close all file descriptors that might have been inherited from the
> > +     * main qemu process when doing clone. This is needed to make libvirt
> > +     * happy.
> > +     */
> > +    close_range(0, ~0U, 0);  
> 
> Shouldn't we close all the FDs immediately when this process is

that's exactly what I did in the beginning, but some things broke...
but I just tried again now and it seems to work, so probably the
breakage was due to something else

> created, rather than only at the end when we're exiting. I don't
> see there's any need to keep them open. Doing it immediately
> would be better when using prctl(PR_SET_PDEATHSIG)

yes

> 
> > +    _exit(0);
> > +}
> > +
> > +void init_async_teardown(void)
> > +{
> > +    const int size = 8192; /* should be more than enough */
> > +    char *stack = malloc(size);
> > +  
> 
> You need to block all signals here.
> 
> > +    /* start a new process sharing the address space with qemu */
> > +    clone(async_teardown_fn, stack + size, CLONE_VM, NULL, NULL, NULL, NULL);  
> 
> And unblock signals again here.
> 
> That way the "everything blocked"  mask is inherited by the child
> from the very first moment of its existance.

sounds like a very good idea

> 
> > +}
> > +#else /* __linux__ */
> > +void init_async_teardown(void) {}
> > +#endif
> > +
> >  #ifdef _WIN32
> >  static void socket_cleanup(void)
> >  {
> > -- 
> > 2.37.1
> >   
> 
> With regards,
> Daniel



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-08-05  8:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-03 17:31 [PATCH v2 1/1] osdep: asynchronous teardown for shutdown on Linux Claudio Imbrenda
2022-08-03 17:34 ` Daniel P. Berrangé
2022-08-04  5:56   ` Claudio Imbrenda
2022-08-04  8:20     ` Daniel P. Berrangé
2022-08-04 16:58       ` Daniel P. Berrangé
2022-08-05  7:02         ` Claudio Imbrenda
2022-08-04  8:29   ` Daniel P. Berrangé
2022-08-04 14:49     ` Claudio Imbrenda
2022-08-04 16:41       ` Daniel P. Berrangé
2022-08-05  6:59         ` Claudio Imbrenda
2022-08-04 16:56 ` Daniel P. Berrangé
2022-08-05  7:32   ` Claudio Imbrenda

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.