On 07.05.19 21:30, Eric Blake wrote: > 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 <mre...@redhat.com> >> --- >> 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).
I think the documentation is pretty clear. It stores the server's PID, whether it has been forked or not. I don't think we would gain anything from forbidding --pid-file without --fork, would we? >> #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). I'd prefer pid_filename myself, then, because pid_name sounds like a weird way to say "process name". O:-) > Otherwise, I agree that this is long overdue. Thanks! If you can justify > the behavior without --fork, I just can’t think of a reason not to allow it without --fork. Maybe a user doesn’t need --fork because they just start the server in the background and that’s good enough, but they still want a PID file. So basically like common.rc’s _qemu_nbd_wrapper() before this series. Max
signature.asc
Description: OpenPGP digital signature