On 5/7/19 1:36 PM, 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 <[email protected]> > --- > qemu-nbd.c | 29 +++++++++++++++++++++++++++++ > qemu-nbd.texi | 2 ++ > 2 files changed, 31 insertions(+) >
> @@ -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"
Should --pid-file imply --fork, or be an error if --fork was not
supplied? As coded, it writes a pid file regardless of --fork, even
though it is less obvious that it is useful in that case. I don't have a
strong preference (there doesn't seem to be a useful consensus on what
forking daemons should do), but it would at least be worth documenting
the intended action (even if that implies a tweak to the patch to match
the intent).
> #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;
Bikeshedding: pid_name is nicer (path makes me think of $PATH and other
colon-separated lists, which this is not).
Otherwise, I agree that this is long overdue. Thanks! If you can justify
the behavior without --fork,
Reviewed-by: Eric Blake <[email protected]>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
