Re: [Virtio-fs] [virtiofsd][virtiofsd-rs] unlink an openfile over NFS

[Vivek Goyal <vgoyal@redhat.com>]

On Wed, Dec 01, 2021 at 01:06:23PM +0100, German Maglione wrote:
> Hi,
> 
> I was working on [1] (related to [2]), and I saw that both versions
> (C and rust) of virtiofsd make the NFs client to "silly rename" an open
> file that were unlinked, because we keep each file open as O_PATH in the
> lo_do_lookup/do_lookup function. David pointed me to this problem, and I
> confirmed that this is the case.
> 
> This fires the silly rename in the nfs client. I'm talking with
> Bruce Fields (nfs team) to see how to move the silly rename functionality
> to the nfs server and outside the directory tree [4], to avoid having
> non-really-empty
> dirs full of .nfsxxx files. But it is not an easy task.
> Also, this will not solve some potential issues with the resource usage:
> disk space and open file limits (I hit this limit a couple of times during
> my
> tests). And, in some cases could be worst, more than one guest sharing the
> same
> exported dir, one guest just 'ls' or 'find' while the other 'rm' some files.
> (The 'find' command will open all files, btw)
> 
> Vivek, I saw in [5] that you mentioned that this could be fixed introducing
> synchronous, could you elaborate a bit or point me to some code?

Hi German,

Right now forget messages are asynchronous. They are sent to server and
client does not wait for any reply. That means when unlink() returns,
it is possible that a FORGET message is in progress and has not finished
yet.

Idea behind synchronous FORGET messages is that it will generate a reply
and client will wait for it. Given inode on server should be gone,
I am not sure how much sense does it make. But anyway conceputaully
that's the idea. If we want for FORGET message to finish, that will
mean that O_PATH fd opened by virtiofsd is closed and we will not
have NFS silly rename issue (atleast due to virtiofsd). If virtiofs
client has file open, then issue will still happen.

I think using file handles in virtiofsd_rs (--inode-file-handles) is
a reasonable solution for this problem. Trying to add synchronous
FORGET might be overkill.

Thanks
Vivek
> 
> Thanks,
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=2018072
> [2] https://bugzilla.redhat.com/show_bug.cgi?id=1908490
> [4] https://wiki.linux-nfs.org/wiki/index.php/Server-side_silly_rename
> [5]
> https://sourceforge.net/p/fuse/mailman/fuse-devel/?viewmonth=202101&viewday=4
> 
> 
> -- 
> German