All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: "Richard W.M. Jones" <rjones@redhat.com>
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com
Subject: Re: [PATCH WIP] Implement -run-with exit-with-parent=on
Date: Wed, 1 Oct 2025 19:32:56 +0100	[thread overview]
Message-ID: <aN1z2C__6M8E7I8L@redhat.com> (raw)
In-Reply-To: <20251001174118.2756134-1-rjones@redhat.com>

On Wed, Oct 01, 2025 at 06:40:56PM +0100, Richard W.M. Jones wrote:
> Libguestfs wants to use qemu to run a captive appliance.  When the
> program linked to libguestfs exits, we want qemu to be cleaned up.
> Libguestfs goes to great lengths to do this at the moment: it either
> forks a separate process to ensure clean-up is done, or it asks
> libvirt to clean up the qemu process.  However this is complicated and
> not totally reliable.
> 
> On Linux, FreeBSD and macOS, there are mechanisms to ensure a signal
> or message is delivered to a process when its parent process goes
> away.  The qemu test suite even uses this mechanism on Linux (see
> PR_SET_PDEATHSIG in tests/qtest/libqtest.c).
> 
> In nbdkit we have long had the concept of running nbdkit captively,
> and we have the nbdkit --exit-with-parent flag to help
> (https://libguestfs.org/nbdkit-captive.1.html#EXIT-WITH-PARENT)
> 
> This commit adds the same mechanism that the qemu test suite uses, and
> that nbdkit has long implemented, to allow this feature to be used by
> all qemu users.  The syntax is:
> 
>   qemu -run-with exit-with-parent=on [...]
> 
> This is not a feature that most typical users of qemu (for running
> general purpose, long-lived VMs) should use, so it defaults to off.
> 
> The exit-with-parent.[ch] files are copied from nbdkit, where they
> have a 3-clause BSD license which is compatible with qemu:
> 
> https://gitlab.com/nbdkit/nbdkit/-/tree/master/common/utils?ref_type=heads
> 
> Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
> ---
>  include/qemu/exit-with-parent.h |  49 +++++++++
>  qemu-options.hx                 |  13 ++-
>  system/exit-with-parent.c       | 171 ++++++++++++++++++++++++++++++++
>  system/meson.build              |   1 +
>  system/vl.c                     |  13 +++
>  5 files changed, 245 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qemu/exit-with-parent.h b/include/qemu/exit-with-parent.h
> new file mode 100644
> index 0000000000..e907ccb5c9
> --- /dev/null
> +++ b/include/qemu/exit-with-parent.h
> @@ -0,0 +1,49 @@
> +/*
> + * SPDX-License-Identifier: BSD-3-Clause
> + * nbdkit

This 'nbdkit' line can be dropped  presumably, unless you want
to refer back to the original source ? Same question for the .c

> + * Copyright Red Hat
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are
> + * met:
> + *
> + * * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + *
> + * * Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + *
> + * * Neither the name of Red Hat nor the names of its contributors may be
> + * used to endorse or promote products derived from this software without
> + * specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
> + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
> + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */

IIUC, from the nbdkit history exit-with-parent.{ch} only ever
had 3 contributors, all from Red Hat. So if Red Hat is exclusive
copyright holder, I think you're be justified in removing the
license boilerplate here & below, as a representative of RH.

> +
> +#ifndef NBDKIT_EXIT_WITH_PARENT_H
> +#define NBDKIT_EXIT_WITH_PARENT_H
> +
> +#include <stdbool.h>

This is redundant in QEMU since it is guaranteed by osdep.h

> +
> +/* Test if the feature is available on the platform. */
> +extern bool can_exit_with_parent(void);
> +
> +/*
> + * --exit-with-parent: kill the current process if the parent exits.
> + * This may return -1 on error.
> + */
> +extern int set_exit_with_parent(void);
> +
> +#endif /* NBDKIT_EXIT_WITH_PARENT_H */

> diff --git a/system/exit-with-parent.c b/system/exit-with-parent.c
> new file mode 100644
> index 0000000000..fa97b586f0
> --- /dev/null
> +++ b/system/exit-with-parent.c
> @@ -0,0 +1,171 @@
> +
> +/*
> + * Implement the --exit-with-parent feature on operating systems which
> + * support it.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/exit-with-parent.h"
> +
> +#ifdef CONFIG_LINUX
> +#include <sys/prctl.h>
> +#endif
> +
> +#ifdef __FreeBSD__
> +#include <sys/procctl.h>
> +#endif
> +
> +#if defined(CONFIG_LINUX) && defined(PR_SET_PDEATHSIG)
> +
> +/* For Linux >= 2.1.57. */

I don't think we need the PR_SET_PDEATHSIG check
since the oldest Linux platform QEMU targets will
always be newer than 2.1.57 and so can assume to
have PR_SET_PDEATHSIG defined.

> +
> +int
> +set_exit_with_parent(void)
> +{
> +    return prctl(PR_SET_PDEATHSIG, SIGTERM);
> +}
> +
> +bool
> +can_exit_with_parent(void)
> +{
> +    return true;
> +}
> +
> +#elif defined(__FreeBSD__) && defined(PROC_PDEATHSIG_CTL)
> +
> +/* For FreeBSD >= 11.2 */

Also don't need the PROC_PDEATHSIG_CTL check either,
since with current FreeBSD being 14.0, the oldest
QEMU targets is now 13.0

> +
> +#include <sys/procctl.h>
> +
> +int
> +set_exit_with_parent(void)
> +{
> +    const int sig = SIGTERM;
> +    return procctl(P_PID, 0, PROC_PDEATHSIG_CTL, (void *) &sig);
> +}
> +
> +bool
> +can_exit_with_parent(void)
> +{
> +    return true;
> +}
> +
> +#elif defined(__APPLE__)
> +
> +/* For macOS. */
> +
> +#include <unistd.h>
> +#include <errno.h>

These two are guaranteed by osdep.h

> +#include <sys/event.h>
> +#include <pthread.h>

Use  "qemu/thread.h" instead

> +
> +static void *
> +exit_with_parent_loop(void *vp)
> +{
> +    const pid_t ppid = getppid();
> +    int fd;
> +    struct kevent kev, res[1];
> +    int r;
> +
> +    /* Register the kevent to wait for ppid to exit. */
> +    fd = kqueue();
> +    if (fd == -1) {
> +        error_report("exit_with_parent_loop: kqueue: %m");
> +        return NULL;
> +    }
> +    EV_SET(&kev, ppid, EVFILT_PROC, EV_ADD | EV_ENABLE, NOTE_EXIT, 0, NULL);
> +    if (kevent(fd, &kev, 1, NULL, 0, NULL) == -1) {
> +        error_report("exit_with_parent_loop: kevent: %m");
> +        close(fd);
> +        return NULL;
> +    }
> +
> +    /* Wait for the kevent to happen. */
> +    r = kevent(fd, 0, 0, res, 1, NULL);
> +    if (r == 1 && res[0].ident == ppid) {
> +        /* Shut down the whole process when the parent dies. */
> +        exit(0);
> +    }
> +
> +    return NULL;
> +}
> +
> +int
> +set_exit_with_parent(void)
> +{
> +    int r;
> +    pthread_attr_t attrs;
> +    pthread_t exit_with_parent_thread;
> +
> +    /*
> +     * We have to block waiting for kevent, so that requires that we
> +     * start a background thread.
> +     */
> +    pthread_attr_init(&attrs);
> +    pthread_attr_setdetachstate(&attrs, PTHREAD_CREATE_DETACHED);
> +    r = pthread_create(&exit_with_parent_thread, NULL,
> +                       exit_with_parent_loop, NULL);


  QemuThread exit_with_parent_thread;
  qemu_thread_create(&exit_with_parent_thread,
                     "exit-parent",
                     exit_with_parent_loop,
                     QEMU_THREAD_DETACHED);

mostly just so we get the thread name set.

> +    if (r != 0) {
> +        errno = r;
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +bool
> +can_exit_with_parent(void)
> +{
> +    return true;
> +}
> +
> +#else /* any platform that doesn't support this function */
> +
> +#include <stdlib.h>

Can be assumed with osdep.h

> +
> +int
> +set_exit_with_parent(void)
> +{
> +    abort();
 
Slightly better to use g_assert_not_reached()

> +}
> +
> +bool
> +can_exit_with_parent(void)
> +{
> +    return false;
> +}
> +
> +#endif

> diff --git a/system/vl.c b/system/vl.c
> index 00f3694725..47b6e60b94 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -53,6 +53,7 @@
>  #include "qemu/sockets.h"
>  #include "qemu/accel.h"
>  #include "qemu/async-teardown.h"
> +#include "qemu/exit-with-parent.h"
>  #include "hw/usb.h"
>  #include "hw/isa/isa.h"
>  #include "hw/scsi/scsi.h"
> @@ -783,6 +784,10 @@ static QemuOptsList qemu_run_with_opts = {
>              .name = "chroot",
>              .type = QEMU_OPT_STRING,
>          },
> +        {
> +            .name = "exit-with-parent",
> +            .type = QEMU_OPT_BOOL,
> +        },
>          {
>              .name = "user",
>              .type = QEMU_OPT_STRING,
> @@ -3690,6 +3695,14 @@ void qemu_init(int argc, char **argv)
>                  if (str) {
>                      os_set_chroot(str);
>                  }
> +                if (qemu_opt_get_bool(opts, "exit-with-parent", false)) {
> +                    if (!can_exit_with_parent()) {
> +                        error_report("exit-with-parent is not available"
> +                                     " on this platform");
> +                        exit(1);
> +                    }
> +                    set_exit_with_parent();
> +                }
>                  str = qemu_opt_get(opts, "user");
>                  if (str) {
>                      if (!os_set_runas(str)) {

Broadly looks good to me.

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



  reply	other threads:[~2025-10-01 18:35 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-01 17:40 [PATCH WIP] Implement -run-with exit-with-parent=on Richard W.M. Jones
2025-10-01 18:32 ` Daniel P. Berrangé [this message]
2025-10-01 19:14   ` Richard W.M. Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aN1z2C__6M8E7I8L@redhat.com \
    --to=berrange@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rjones@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.