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

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>,
	"Eric Blake" <eblake@redhat.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"qemu-block@nongnu.org" <qemu-block@nongnu.org>
Subject: Re: [PATCH] qemu-nbd: Document benefit of --pid-file
Date: Tue, 8 Oct 2019 13:53:13 +0000	[thread overview]
Message-ID: <99240aec-6d19-cea6-5b95-6bbf5a9106e8@virtuozzo.com> (raw)
In-Reply-To: <20191008133834.GG1192@redhat.com>

08.10.2019 16:38, 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.
>>>>
>>>> I'm not seeing how checking the pid file is better than checking
>>>> the socket directly ? I think it is probably actually worse.
>>>>
>>>> The main problem with the socket is that while we unlink on clean
>>>> shutdown, it may still exist in disk if the process has exitted
>>>> abnormally.
>>>>
>>>> With the pidfile though we don't ever unlink it, even on clean
>>>> shutdown, as we don't use the pid files existance as a mutual
>>>> exclusion check. We instead acquire fcntl locks on it.
>>>>
>>>> IOW the pidfile could exist already when qemu-nbd starts up and
>>>> will still exist when it quits.
>>>
>>> Good point.
>>>
>>> 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.

I've already implemented loop of attempting to connect in my series (patch 4/3).
It's a bit more difficult to implement, but it's done. And it's a bit better,
as it test exactly what we want to test. Can we proceed with it?

> 
> 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 ?
> 
> Regards,
> Daniel
> 

-- 
Best regards,
Vladimir