Re: [PATCH] nfsd: map EBUSY for all operations

[Trond Myklebust <trondmy@hammerspace.com>]

On Mon, 2025-01-20 at 19:21 +0100, Amir Goldstein wrote:
> On Mon, Jan 20, 2025 at 6:28 PM Trond Myklebust
> <trondmy@hammerspace.com> wrote:
> > 
> > On Mon, 2025-01-20 at 18:20 +0100, Amir Goldstein wrote:
> > > v4 client maps NFS4ERR_FILE_OPEN => EBUSY for all operations.
> > > 
> > > v4 server only maps EBUSY => NFS4ERR_FILE_OPEN for
> > > rmdir()/unlink()
> > > although it is also possible to get EBUSY from rename() for the
> > > same
> > > reason (victim is a local mount point).
> > > 
> > > Filesystems could return EBUSY for other operations, so just map
> > > it
> > > in server for all operations.
> > > 
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> > > 
> > > Chuck,
> > > 
> > > I ran into this error with a FUSE filesystem and returns -EBUSY
> > > on
> > > open,
> > > but I noticed that vfs can also return EBUSY at least for
> > > rename().
> > > 
> > > Thanks,
> > > Amir.
> > > 
> > >  fs/nfsd/vfs.c | 10 ++--------
> > >  1 file changed, 2 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > > index 29cb7b812d713..a61f99c081894 100644
> > > --- a/fs/nfsd/vfs.c
> > > +++ b/fs/nfsd/vfs.c
> > > @@ -100,6 +100,7 @@ nfserrno (int errno)
> > >               { nfserr_perm, -ENOKEY },
> > >               { nfserr_no_grace, -ENOGRACE},
> > >               { nfserr_io, -EBADMSG },
> > > +             { nfserr_file_open, -EBUSY},
> > >       };
> > >       int     i;
> > > 
> > > @@ -2006,14 +2007,7 @@ nfsd_unlink(struct svc_rqst *rqstp, struct
> > > svc_fh *fhp, int type,
> > >  out_drop_write:
> > >       fh_drop_write(fhp);
> > >  out_nfserr:
> > > -     if (host_err == -EBUSY) {
> > > -             /* name is mounted-on. There is no perfect
> > > -              * error status.
> > > -              */
> > > -             err = nfserr_file_open;
> > > -     } else {
> > > -             err = nfserrno(host_err);
> > > -     }
> > > +     err = nfserrno(host_err);
> > >  out:
> > >       return err;
> > >  out_unlock:
> > 
> > If this is a transient error, then it would seem that NFS4ERR_DELAY
> > would be more appropriate.
> 
> It is not a transient error, not in the case of a fuse file open
> (it is busy as in locked for as long as it is going to be locked)
> and not in the case of failure to unlink/rename a local mountpoint.
> NFS4ERR_DELAY will cause the client to retry for a long time?
> 
> > NFS4ERR_FILE_OPEN is not supposed to apply
> > to directories, and so clients would be very confused about how to
> > recover if you were to return it in this situation.
> 
> Do you mean specifically for OPEN command, because commit
> 466e16f0920f3 ("nfsd: check for EBUSY from vfs_rmdir/vfs_unink.")
> added the NFS4ERR_FILE_OPEN response for directories five years
> ago and vfs_rmdir can certainly return a non-transient EBUSY.
> 

I'm saying that clients expect NFS4ERR_FILE_OPEN to be returned in
response to LINK, REMOVE or RENAME only in situations where the error
itself applies to a regular file.
The protocol says that the client can expect this return value to mean
it is dealing with a server with Windows-like semantics that doesn't
allow these particular operations while the file is being held open. It
says nothing about expecting the same behaviour for mountpoints, and
since the latter have a very different life cycle than file open state
does, you should not treat those cases as being the same.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com