[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 1/5] qemu-nbd: Add --pid-file option
From: |
Daniel P . Berrangé |
Subject: |
Re: [Qemu-block] [PATCH 1/5] qemu-nbd: Add --pid-file option |
Date: |
Wed, 8 May 2019 10:01:31 +0100 |
User-agent: |
Mutt/1.11.4 (2019-03-13) |
On Tue, May 07, 2019 at 08:36:06PM +0200, Max Reitz wrote:
> --fork is a bit boring if there is no way to get the child's PID. This
> option helps.
>
> Signed-off-by: Max Reitz <address@hidden>
> ---
> qemu-nbd.c | 29 +++++++++++++++++++++++++++++
> qemu-nbd.texi | 2 ++
> 2 files changed, 31 insertions(+)
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index dca9e72cee..7e48154f44 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -59,6 +59,7 @@
> #define QEMU_NBD_OPT_IMAGE_OPTS 262
> #define QEMU_NBD_OPT_FORK 263
> #define QEMU_NBD_OPT_TLSAUTHZ 264
> +#define QEMU_NBD_OPT_PID_FILE 265
>
> #define MBR_SIZE 512
>
> @@ -111,6 +112,7 @@ static void usage(const char *name)
> " specify tracing options\n"
> " --fork fork off the server process and exit the
> parent\n"
> " once the server is running\n"
> +" --pid-file=PATH store the server's process ID in the given
> file\n"
> #if HAVE_NBD_DEVICE
> "\n"
> "Kernel NBD client support:\n"
> @@ -651,6 +653,7 @@ int main(int argc, char **argv)
> { "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
> { "trace", required_argument, NULL, 'T' },
> { "fork", no_argument, NULL, QEMU_NBD_OPT_FORK },
> + { "pid-file", required_argument, NULL, QEMU_NBD_OPT_PID_FILE },
> { NULL, 0, NULL, 0 }
> };
> int ch;
> @@ -677,6 +680,8 @@ int main(int argc, char **argv)
> bool list = false;
> int old_stderr = -1;
> unsigned socket_activation;
> + const char *pid_path = NULL;
> + FILE *pid_file;
>
> /* The client thread uses SIGTERM to interrupt the server. A signal
> * handler ensures that "qemu-nbd -v -c" exits with a nice status code.
> @@ -876,6 +881,9 @@ int main(int argc, char **argv)
> case 'L':
> list = true;
> break;
> + case QEMU_NBD_OPT_PID_FILE:
> + pid_path = optarg;
> + break;
> }
> }
>
> @@ -1196,6 +1204,27 @@ int main(int argc, char **argv)
>
> nbd_update_server_watch();
>
> + if (pid_path) {
> + pid_file = fopen(pid_path, "w");
> + if (!pid_file) {
> + error_report("Failed to store PID in %s: %s",
> + pid_path, strerror(errno));
> + exit(EXIT_FAILURE);
> + }
> +
> + ret = fprintf(pid_file, "%ld", (long)getpid());
> + if (ret < 0) {
> + ret = -errno;
> + }
> + fclose(pid_file);
> +
> + if (ret < 0) {
> + error_report("Failed to store PID in %s: %s",
> + pid_path, strerror(-ret));
> + exit(EXIT_FAILURE);
> + }
> + }
This is racy because multiple qemu-nbd's can be started pointing to
the same pidfile and one will blindly overwrite the other.
Please use qemu_write_pidfile instead which acquires locks on the
pidfile in a race free manner.
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 :|
[Qemu-block] [PATCH 2/5] iotests.py: Add qemu_nbd_early_pipe(), Max Reitz, 2019/05/07
[Qemu-block] [PATCH 4/5] iotests: Use qemu-nbd's --pid-file, Max Reitz, 2019/05/07
[Qemu-block] [PATCH 3/5] qemu-nbd: Do not close stderr, Max Reitz, 2019/05/07
[Qemu-block] [PATCH 5/5] iotests: Let 233 run concurrently, Max Reitz, 2019/05/07