On Thu, Jan 23, 2025 at 8:52 PM <cel@xxxxxxxxxx> wrote:
>
> From: Chuck Lever <chuck.lever@xxxxxxxxxx>
>
> RFC 8881 Section 18.26.4 paragraphs 1 - 3 tell us that RENAME should
> return NFS4ERR_FILE_OPEN only when the target object is a file that
> is currently open. If the target is a directory, some other status
> must be returned.
>
> Generally I expect that a delegation recall will be triggered in
> some of these circumstances. In other cases, the VFS might return
> -EBUSY for other reasons, and NFSD has to ensure that errno does
> not leak to clients as a status code that is not permitted by spec.
>
> There are some error flows where the target dentry hasn't been
> found yet. The default value for @type therefore is S_IFDIR to return
> an alternate status code in those cases.
>
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
> fs/nfsd/vfs.c | 30 ++++++++++++++++++++++++++----
> 1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 3ead7fb3bf04..5cfb5eb54c23 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1795,9 +1795,19 @@ nfsd_has_cached_files(struct dentry *dentry)
> return ret;
> }
>
> -/*
> - * Rename a file
> - * N.B. After this call _both_ ffhp and tfhp need an fh_put
> +/**
> + * nfsd_rename - rename a directory entry
> + * @rqstp: RPC transaction context
> + * @ffhp: the file handle of parent directory containing the entry to be renamed
> + * @fname: the filename of directory entry to be renamed
> + * @flen: the length of @fname in octets
> + * @tfhp: the file handle of parent directory to contain the renamed entry
> + * @tname: the filename of the new entry
> + * @tlen: the length of @tlen in octets
> + *
> + * After this call _both_ ffhp and tfhp need an fh_put.
> + *
> + * Returns a generic NFS status code in network byte-order.
> */
> __be32
> nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> @@ -1805,6 +1815,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> {
> struct dentry *fdentry, *tdentry, *odentry, *ndentry, *trap;
> struct inode *fdir, *tdir;
> + int type = S_IFDIR;
> __be32 err;
> int host_err;
> bool close_cached = false;
> @@ -1867,6 +1878,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> host_err = PTR_ERR(ndentry);
> if (IS_ERR(ndentry))
> goto out_dput_old;
> + type = d_inode(ndentry)->i_mode & S_IFMT;
> host_err = -ENOTEMPTY;
> if (ndentry == trap)
> goto out_dput_new;
> @@ -1904,7 +1916,17 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> out_dput_old:
> dput(odentry);
> out_nfserr:
> - err = nfserrno(host_err);
> + if (host_err == -EBUSY) {
> + /*
> + * See RFC 8881 Section 18.26.4 para 1-3: NFSv4 RENAME
> + * status distinguishes between reg file and dir.
> + */
> + if (type != S_IFDIR)
Maybe (type == S_IFREG) here as well?
> + err = nfserr_file_open;
> + else
> + err = nfserr_acces;
> + } else
> + err = nfserrno(host_err);
>
if {} else ; is a code smell with high potential for human
misreading of code.
Thanks,
Amir.
