Re: [Qemu-devel] [PATCH V2 2/2] block/nfs: add support for nfs_umount

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3476 bytes --]

On 13.09.19 12:09, ronnie sahlberg wrote:
> On Wed, Sep 11, 2019 at 5:48 PM Max Reitz <mreitz@redhat.com> wrote:
>>
>> On 10.09.19 17:41, Peter Lieven wrote:
>>> libnfs recently added support for unmounting. Add support
>>> in Qemu too.
>>>
>>> Signed-off-by: Peter Lieven <pl@kamp.de>
>>> ---
>>>  block/nfs.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/block/nfs.c b/block/nfs.c
>>> index 2c98508275..f39acfdb28 100644
>>> --- a/block/nfs.c
>>> +++ b/block/nfs.c
>>> @@ -398,6 +398,9 @@ static void nfs_client_close(NFSClient *client)
>>>              nfs_close(client->context, client->fh);
>>>              client->fh = NULL;
>>>          }
>>> +#ifdef LIBNFS_FEATURE_UMOUNT
>>> +        nfs_umount(client->context);
>>> +#endif
>>>          nfs_destroy_context(client->context);
>>>          client->context = NULL;
>>>      }
>>
>> I don’t understand what unmounting means in this context.  Is it just
>> generic clean-up for NFSv3 (it appears that it’s a no-op for NFSv4)?
>> Why isn’t that done by nfs_destroy_context()?
> 
> Umount is weird since there isn't actually any state in NFSv3 and
> "mounting" in nfsv3 is really just a matter of converting the path to
> be mounted into a filehandle.
> That is all the mount protocol is really used for.
> 
> This is all handled in a separate protocol/server called rpc.mountd
> that is separate from NFSd. Running as a different process and
> listening to a different port.
> And the only purpose of rpc.mountd is to take a path to a share and
> return a nfsv3 filehandle to the root of that path.
> As a side-effect, rpc.mountd also keeps track of which clients have
> called MNT but not yet UMNT and thus showmount -a
> can give a lost of all client that have "mounted" the share but not
> yet called "UMNT".
> 
> It has no effect at all on NFSv3 and is purely cosmetic. This ONLY
> affects "showmount -a" output.
> NFSv4 does away with all these separate protocols such as mount,
> statd, nlm and even portmapper
> so there is not even a concept of showmount -a for nfsv4.
> 
> 
> As the libnfs maintainer, why did I do nfs_umount() the way I did?
> First of all, I think of nfs UMNT as really just cosmetic and didn't
> want to put too much work into it. But people wanted it.
> 
> I implemented it as a sync function since I think few people would
> actually use it at all and it meant that I just didn't have to invest
> in having to build an async piupelinje.
> 
> I did NOT implement it inside nfs_destroy_context() since that
> function is supposed to be, in my view, non-blocking andn should just
> tear the connection and immediately return.
> As unmount would be
> * close the tcp socket to the nfs server
> * open new socket to portmapper and ask "where is rpc.mountd"
> * close socket to portmapper, then open new socket to rpc.mountd
>  * tell rpc.mountd to remove us from the showmount -a list
> * close socket
> 
> I just took the cheap and easy path. Do it as a sync function with my
> own eventloop.
> 
> Again, UMNT has no real effect on anything related to NFS except what
> showmount -a will return. That is one big reason why
> I was just not much motivated enough to build it as an async function.
> 
> Once we all switch to NFSv4 this will all be moot since the MOUNT
> protocol no longer exists and neither does rpc.mountd.

OK.  Thanks a lot for the detailed explanation! :-)

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]