qemu-devel.nongnu.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).