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 <[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).
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
