qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] qemu-storage-daemon: add --pidfile option


From: Daniel P . Berrangé
Subject: Re: [PATCH] qemu-storage-daemon: add --pidfile option
Date: Mon, 1 Mar 2021 16:15:47 +0000
User-agent: Mutt/2.0.5 (2021-01-21)

On Mon, Mar 01, 2021 at 04:08:57PM +0000, Stefan Hajnoczi wrote:
> Daemons often have a --pidfile option where the pid is written to a file
> so that scripts can stop the daemon by sending a signal.
> 
> The pid file also acts as a lock to prevent multiple instances of the
> daemon from launching for a given pid file.
> 
> QEMU, qemu-nbd, qemu-ga, virtiofsd, and qemu-pr-helper all support the
> --pidfile option. Add it to qemu-storage-daemon too.
> 
> Reported-by: Richard W.M. Jones <rjones@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  docs/tools/qemu-storage-daemon.rst   | 10 ++++++++++
>  storage-daemon/qemu-storage-daemon.c | 29 ++++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/docs/tools/qemu-storage-daemon.rst 
> b/docs/tools/qemu-storage-daemon.rst
> index f63627eaf6..8f4ab16ffc 100644
> --- a/docs/tools/qemu-storage-daemon.rst
> +++ b/docs/tools/qemu-storage-daemon.rst
> @@ -118,6 +118,16 @@ Standard options:
>    List object properties with ``<type>,help``. See the :manpage:`qemu(1)`
>    manual page for a description of the object properties.
>  
> +.. option:: --pidfile PATH
> +
> +  is the path to a file where the daemon writes its pid. This allows scripts 
> to
> +  stop the daemon by sending a signal::
> +
> +    $ kill -SIGTERM $(<path/to/qsd.pid)
> +
> +  A file lock is applied to the file so only one instance of the daemon can 
> run
> +  with a given pid file path. The daemon unlinks its pid file when 
> terminating.

Usually a pidfile wants to have some explicit synchronization rules
defined. AFAICS, qsd doesn't have a --daemonize option to sync against.
If we're using the FD passing trick for the monitor, however, we want
a guarantee that the pidfile is written before the monitor accepts the
first client.

IOW, the parent process needs to know that once it has done the QMP
handshake, there is guaranteed tobe a pidfile present, or if the
QMP handshake fails, then the app is guaranteed to have quit.

IIUC, this impl should be ok in this respect, because we won't process
the QMP handdshake until the main loop runs, at which point the pidfile
is present. So we just need to document that the pidfile is guaranteed
to be written by the time QMP is active.


> +
>  Examples
>  --------
>  Launch the daemon with QMP monitor socket ``qmp.sock`` so clients can execute
> diff --git a/storage-daemon/qemu-storage-daemon.c 
> b/storage-daemon/qemu-storage-daemon.c
> index 9021a46b3a..011ae49ac3 100644
> --- a/storage-daemon/qemu-storage-daemon.c
> +++ b/storage-daemon/qemu-storage-daemon.c
> @@ -59,6 +59,7 @@
>  #include "sysemu/runstate.h"
>  #include "trace/control.h"
>  
> +static const char *pid_file;
>  static volatile bool exit_requested = false;
>  
>  void qemu_system_killed(int signal, pid_t pid)
> @@ -126,6 +127,7 @@ enum {
>      OPTION_MONITOR,
>      OPTION_NBD_SERVER,
>      OPTION_OBJECT,
> +    OPTION_PIDFILE,
>  };
>  
>  extern QemuOptsList qemu_chardev_opts;
> @@ -164,6 +166,7 @@ static void process_options(int argc, char *argv[])
>          {"monitor", required_argument, NULL, OPTION_MONITOR},
>          {"nbd-server", required_argument, NULL, OPTION_NBD_SERVER},
>          {"object", required_argument, NULL, OPTION_OBJECT},
> +        {"pidfile", required_argument, NULL, OPTION_PIDFILE},
>          {"trace", required_argument, NULL, 'T'},
>          {"version", no_argument, NULL, 'V'},
>          {0, 0, 0, 0}
> @@ -275,6 +278,9 @@ static void process_options(int argc, char *argv[])
>                  qobject_unref(args);
>                  break;
>              }
> +        case OPTION_PIDFILE:
> +            pid_file = optarg;
> +            break;
>          default:
>              g_assert_not_reached();
>          }
> @@ -285,6 +291,27 @@ static void process_options(int argc, char *argv[])
>      }
>  }
>  
> +static void pid_file_cleanup(void)
> +{
> +    unlink(pid_file);
> +}
> +
> +static void pid_file_init(void)
> +{
> +    Error *err = NULL;
> +
> +    if (!pid_file) {
> +        return;
> +    }
> +
> +    if (!qemu_write_pidfile(pid_file, &err)) {
> +        error_reportf_err(err, "cannot create PID file: ");
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    atexit(pid_file_cleanup);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  #ifdef CONFIG_POSIX
> @@ -312,6 +339,8 @@ int main(int argc, char *argv[])
>      qemu_init_main_loop(&error_fatal);
>      process_options(argc, argv);
>  
> +    pid_file_init();
> +
>      while (!exit_requested) {
>          main_loop_wait(false);
>      }
> -- 
> 2.29.2
> 

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 via email to

[Prev in Thread] Current Thread [Next in Thread]