Re: [Qemu-block] [PATCH 1/5] qemu-nbd: Add --pid-file option

[Max Reitz]

On 07.05.19 21:51, Eric Blake wrote:
> On 5/7/19 2:39 PM, Max Reitz wrote:
>> 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 <address@hidden>
>>>> ---
>>>>  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?
> 
> I can't think of any reason to forbid it. So it sounds like we are
> intentional, this writes the pid into --pid-file regardless of whether
> that pid can be learned by other means as well.
> 
> 
>>>> +    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:-)
> 
> Works for me, even if it is longer. Do you want to respin, or just have
> me touch it up when folding it into my NBD tree?

I suppose I’d prefer a respin, independently of what you make of patches
4 and 5.

Max

signature.asc
Description: OpenPGP digital signature