Re: [PATCH v2] nfsd: map EBUSY to NFS4ERR_ACCESS for all operations

[Amir Goldstein <amir73il@xxxxxxxxx>]

> > I am fine with handling EBUSY in unlink/rmdir/rename/open
> > only for now if that is what everyone prefers.
>
> As far as I can tell, NFSv2 and NFSv3 REMOVE/RMDIR are working
> correctly. NFSv4 REMOVE needs to return a status code that depends
> on whether the target object is a file or not. Probably not much more
> than something like this:
>
>         status = vfs_unlink( ... );
> +       /* RFC 8881 Section 18.25.4 paragraph 5 */
> +       if (status == nfserr_file_open && !S_ISREG(...))
> +               status = nfserr_access;
>
> added to nfsd4_remove().

Don't you think it's a bit awkward mapping back and forth like this?
Don't you think something like this is a more sane way to keep the
mapping rules in one place:

@@ -111,6 +111,26 @@ nfserrno (int errno)
        return nfserr_io;
 }

+static __be32
+nfsd_map_errno(int host_err, int may_flags, int type)
+{
+       switch (host_err) {
+       case -EBUSY:
+               /*
+                * According to RFC 8881 Section 18.25.4 paragraph 5,
+                * removal of regular file can fail with NFS4ERR_FILE_OPEN.
+                * For failure to remove directory we return NFS4ERR_ACCESS,
+                * same as NFS4ERR_FILE_OPEN is mapped in v3 and v2.
+                */
+               if (may_flags == NFSD_MAY_REMOVE && type == S_IFREG)
+                       return nfserr_file_open;
+               else
+                       return nfserr_acces;
+       }
+
+       return nfserrno(host_err);
+}
+
 /*
  * Called from nfsd_lookup and encode_dirent. Check if we have crossed
  * a mount point.
@@ -2006,14 +2026,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 = nfsd_map_errno(host_err, NFSD_MAY_REMOVE, type);
 out:
        return err;

>
> Let's visit RENAME once that is addressed.

And then next patch would be:

@@ -1828,6 +1828,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct
svc_fh *ffhp, char *fname, int flen,
        __be32          err;
        int             host_err;
        bool            close_cached = false;
+       int             type;

        err = fh_verify(rqstp, ffhp, S_IFDIR, NFSD_MAY_REMOVE);
        if (err)
@@ -1922,8 +1923,10 @@ nfsd_rename(struct svc_rqst *rqstp, struct
svc_fh *ffhp, char *fname, int flen,
  out_dput_new:
        dput(ndentry);
  out_dput_old:
+       type = d_inode(odentry)->i_mode & S_IFMT;
        dput(odentry);
  out_nfserr:
-        err = nfserrno(host_err);
+       err = nfsd_map_errno(host_err, NFSD_MAY_REMOVE, type);

>
> Then handle OPEN as a third patch, because I bet we are going to meet
> some complications there.
>
>

Did you think of anything better to do for OPEN other than NFS4ERR_ACCESS?

Thanks,
Amir.