Re: [PATCH] qemu-nbd: Document benefit of --pid-file

[Eric Blake]

On 10/8/19 8:38 AM, Daniel P. Berrangé wrote:
> On Tue, Oct 08, 2019 at 08:28:16AM -0500, Eric Blake wrote:
> > On 10/8/19 4:40 AM, Vladimir Sementsov-Ogievskiy wrote:
> > > 08.10.2019 12:24, Daniel P. Berrangé wrote:
> > > > On Mon, Oct 07, 2019 at 02:48:40PM -0500, Eric Blake wrote:
> > > > > One benefit of --pid-file is that it is easier to probe the file
> > > > > system to see if a pid file has been created than it is to probe if a
> > > > > socket is available for connection. Document that this is an
> > > > > intentional feature.

Revisiting an older thread,


> > > I was just a bit confused, because pid file is something unrelated to
> > > socket, and why use one thing to check the existence of another, if we
> > > can directly try to connect.
> >
> > Consider the case of writing a testsuite that involves an nbd client, where
> > you want to fire up qemu-nbd as the server.  Checking for a pid file in
> > shell is easy, and can be done prior to the point of spawning a client.
> > Checking for a successful connect is harder - the shell is not the point
> > that would actually connect, so checking if connect works involves the shell
> > actually spawning off the child process that attempts the connect.  If the
> > client itself has a retry builtin, then you don't need to do anything in
> > shell - just spawn the client with retry (at which point, the client
> > retrying on the connection is smarter than the client retrying on the pid
> > file).  But pid files are immensely useful when you have a client that does
> > not have builtin retry, and when writing a testing framework where you use
> > shell to learn whether it is safe to spawn the client: rather than having to
> > modify the client or write a shell loop that respawns child attempts, you
> > merely need a shell loop probing for the pid file to exist.
>
> We shouldn't need todo any of those tricks IIUC.  The --fork argument is
> supposed to only let the parent process exit once the server is running.
>
> IOW, if you run qemu-nbd --fork, in the foreground, then when execution
> continues the sockets should be present & accepting connections. No need
> to check for existance of any files or check connecting, etc.
>
>
> Except that AFAICT, --fork isn't actually implemented with this semantics
> in qemu-nbd. It looks like we only listen on the sockets after the parent
> has already exited :-( Can we fix that to synchronize wrt socket listeners ?

Actually, after re-reading the code, it IS implemented this way. The 
parent process when --fork is used does not exit until after the child 
process closes its temporary stderr pipe to the parent, which is after 
the socket has been set up and only right before beginning the main 
event loop.  So when --fork is in use, waiting for the process to exit 
is sufficient.

But when --fork is not in use (because you want to keep the qemu-nbd 
process in the foreground for whatever reason), then --pid-file can 
indeed serve as a witness when the single process has progressed far 
enough to have the socket open.

At any rate, I'm less certain if it is worth trying to revive this 
patch.  A doc-only change is still okay for 4.2-rc2, except that we have 
to agree on what such a doc change should be and if it adds anything 
useful to the reader.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org