Re: [PATCH 06/14] nfsd: define xattr functions to call in to their vfs counterparts

From: Chuck Lever <chuck.lever@oracle.com>
To: Frank van der Linden <fllinden@amazon.com>
Cc: Bruce Fields <bfields@fieldses.org>,
	Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 06/14] nfsd: define xattr functions to call in to their vfs counterparts
Date: Thu, 12 Mar 2020 12:23:57 -0400	[thread overview]
Message-ID: <77A441AA-E880-4C74-B662-B315D6734ED2@oracle.com> (raw)
In-Reply-To: <20200311195954.27117-7-fllinden@amazon.com>

> On Mar 11, 2020, at 3:59 PM, Frank van der Linden <fllinden@amazon.com> wrote:
> 
> This adds the filehandle based functions for the xattr operations
> that call in to the vfs layer to do the actual work.

Before posting again, use "make C=1" to clear up new sparse
errors, and scripts/checkpatch.pl to find and correct whitespace
damage introduced in this series.

> Signed-off-by: Frank van der Linden <fllinden@amazon.com>
> ---
> fs/nfsd/vfs.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/vfs.h |  10 +++++
> 2 files changed, 140 insertions(+)
> 
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 0aa02eb18bd3..115449009bc0 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -2058,6 +2058,136 @@ static int exp_rdonly(struct svc_rqst *rqstp, struct svc_export *exp)
> 	return nfsexp_flags(rqstp, exp) & NFSEXP_READONLY;
> }
> 
> +#ifdef CONFIG_NFSD_V4
> +/*
> + * Helper function to translate error numbers. In the case of xattr operations,
> + * some error codes need to be translated outside of the standard translations.
> + *
> + * ENODATA needs to be translated to nfserr_noxattr.
> + * E2BIG to nfserr_xattr2big.
> + *
> + * Additionally, vfs_listxattr can return -ERANGE. This means that the
> + * file has too many extended attributes to retrieve inside an
> + * XATTR_LIST_MAX sized buffer. This is a bug in the xattr implementation:
> + * filesystems will allow the adding of extended attributes until they hit
> + * their own internal limit. This limit may be larger than XATTR_LIST_MAX.
> + * So, at that point, the attributes are present and valid, but can't
> + * be retrieved using listxattr, since the upper level xattr code enforces
> + * the XATTR_LIST_MAX limit.
> + *
> + * This bug means that we need to deal with listxattr returning -ERANGE. The
> + * best mapping is to return TOOSMALL.
> + */
> +static __be32
> +nfsd_xattr_errno(int err)
> +{
> +	switch (err) {
> +	case -ENODATA:
> +		return nfserr_noxattr;
> +	case -E2BIG:
> +		return nfserr_xattr2big;
> +	case -ERANGE:
> +		return nfserr_toosmall;
> +	}
> +	return nfserrno(err);
> +}
> +
> +__be32
> +nfsd_getxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name,
> +	      void *buf, int *lenp)
> +{
> +	ssize_t lerr;
> +	int err;
> +
> +	err = fh_verify(rqstp, fhp, 0, NFSD_MAY_READ);
> +	if (err)
> +		return err;
> +
> +	lerr = vfs_getxattr(fhp->fh_dentry, name, buf, *lenp);
> +	if (lerr < 0)
> +		err = nfsd_xattr_errno(lerr);
> +	else
> +		*lenp = lerr;
> +
> +	return err;
> +}
> +
> +__be32
> +nfsd_listxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, void *buf, int *lenp)
> +{
> +	ssize_t lerr;
> +	int err;
> +
> +	err = fh_verify(rqstp, fhp, 0, NFSD_MAY_READ);
> +	if (err)
> +		return err;
> +
> +	lerr = vfs_listxattr(fhp->fh_dentry, buf, *lenp);
> +
> +	if (lerr < 0)
> +		err = nfsd_xattr_errno(lerr);
> +	else
> +		*lenp = lerr;
> +
> +	return err;
> +}
> +
> +/*
> + * Removexattr and setxattr need to call fh_lock to both lock the inode
> + * and set the change attribute. Since the top-level vfs_removexattr
> + * and vfs_setxattr calls already do their own inode_lock calls, call
> + * the _locked variant. Pass in a NULL pointer for delegated_inode,
> + * and let the client deal with NFS4ERR_DELAY (same as with e.g.
> + * setattr and remove).
> + */
> +__be32
> +nfsd_removexattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name)
> +{
> +	int err, ret;
> +
> +	err = fh_verify(rqstp, fhp, 0, NFSD_MAY_WRITE);
> +	if (err)
> +		return err;
> +
> +	ret = fh_want_write(fhp);
> +	if (ret)
> +		return nfserrno(ret);
> +
> +	fh_lock(fhp);
> +
> +	ret = __vfs_removexattr_locked(fhp->fh_dentry, name, NULL);
> +
> +	fh_unlock(fhp);
> +	fh_drop_write(fhp);
> +
> +	return nfsd_xattr_errno(ret);
> +}
> +
> +__be32
> +nfsd_setxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name,
> +	      void *buf, u32 len, u32 flags)
> +{
> +	int err, ret;
> +
> +	err = fh_verify(rqstp, fhp, 0, NFSD_MAY_WRITE);
> +	if (err)
> +		return err;
> +
> +	ret = fh_want_write(fhp);
> +	if (ret)
> +		return nfserrno(ret);
> +	fh_lock(fhp);
> +
> +	ret = __vfs_setxattr_locked(fhp->fh_dentry, name, buf, len, flags,
> +				    NULL);
> +
> +	fh_unlock(fhp);
> +	fh_drop_write(fhp);
> +
> +	return nfsd_xattr_errno(ret);
> +}
> +#endif
> +
> /*
>  * Check for a user's access permissions to this inode.
>  */
> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> index 3eb660ad80d1..2d2cf5b0543b 100644
> --- a/fs/nfsd/vfs.h
> +++ b/fs/nfsd/vfs.h
> @@ -76,6 +76,16 @@ __be32		do_nfsd_create(struct svc_rqst *, struct svc_fh *,
> __be32		nfsd_commit(struct svc_rqst *, struct svc_fh *,
> 				loff_t, unsigned long, __be32 *verf);
> #endif /* CONFIG_NFSD_V3 */
> +#ifdef CONFIG_NFSD_V4
> +__be32		nfsd_getxattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> +			    char *name, void *buf, int *lenp);
> +__be32		nfsd_listxattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> +			    void *buf, int *lenp);
> +__be32		nfsd_removexattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> +			    char *name);
> +__be32		nfsd_setxattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> +			    char *name, void *buf, u32 len, u32 flags);
> +#endif
> int 		nfsd_open_break_lease(struct inode *, int);
> __be32		nfsd_open(struct svc_rqst *, struct svc_fh *, umode_t,
> 				int, struct file **);
> -- 
> 2.16.6
> 

--
Chuck Lever