Re: [PATCH v3 2/3] nfsd: Add ALLOCATE support

J. Bruce Fields
2014-11-06T20:08:46Z

On Fri, Oct 31, 2014 at 04:53:45PM -0400, Anna.Schumaker@netapp.com wrote:
> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>
> The ALLOCATE operation is used to preallocate space in a file. I can do
> this by using vfs_fallocate() to do the actual preallocation.
>
> ALLOCATE only returns a status indicator, so we don't need to write a
> special encode() function.
>
> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> ---
> fs/nfsd/nfs4proc.c | 36 ++++++++++++++++++++++++++++++++++++
> fs/nfsd/nfs4xdr.c | 19 ++++++++++++++++++-
> fs/nfsd/vfs.c | 20 ++++++++++++++++++++
> fs/nfsd/vfs.h | 2 ++
> fs/nfsd/xdr4.h | 8 ++++++++
> 5 files changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index cdeb3cf..57bfcad 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1014,6 +1014,36 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> }
>
> static __be32
> +nfsd4_fallocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> + struct nfsd4_fallocate *fallocate, int flags)
> +{
> + __be32 status = nfserr_notsupp;
> + struct file *file;
> +
> + status = nfs4_preprocess_stateid_op(SVC_NET(rqstp), cstate,
> + &fallocate->falloc_stateid,
> + WR_STATE, &file);
> + if (status != nfs_ok) {
> + dprintk("NFSD: nfsd4_fallocate: couldn't process stateid!\n");
> + return status;
> + }
> +
> + status = nfsd4_vfs_fallocate(rqstp, &cstate->current_fh, file,
> + fallocate->falloc_offset,
> + fallocate->falloc_length,
> + flags);
> + fput(file);
> + return status;
> +}
> +
> +static __be32
> +nfsd4_allocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> + struct nfsd4_fallocate *fallocate)
> +{
> + return nfsd4_fallocate(rqstp, cstate, fallocate, 0);
> +}
> +
> +static __be32
> nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> struct nfsd4_seek *seek)
> {
> @@ -1926,6 +1956,12 @@ static struct nfsd4_operation nfsd4_ops[] = {
> },
>
> /* NFSv4.2 operations */
> + [OP_ALLOCATE] = {
> + .op_func = (nfsd4op_func)nfsd4_allocate,
> + .op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
> + .op_name = "OP_ALLOCATE",
> + .op_rsize_bop = (nfsd4op_rsize)nfsd4_write_rsize,
> + },
> [OP_SEEK] = {
> .op_func = (nfsd4op_func)nfsd4_seek,
> .op_name = "OP_SEEK",
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index eeea7a9..a60cff8 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1514,6 +1514,23 @@ static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, str
> }
>
> static __be32
> +nfsd4_decode_fallocate(struct nfsd4_compoundargs *argp,
> + struct nfsd4_fallocate *fallocate)
> +{
> + DECODE_HEAD;
> +
> + status = nfsd4_decode_stateid(argp, &fallocate->falloc_stateid);
> + if (status)
> + return status;
> +
> + READ_BUF(16);
> + p = xdr_decode_hyper(p, &fallocate->falloc_offset);
> + xdr_decode_hyper(p, &fallocate->falloc_length);
> +
> + DECODE_TAIL;
> +}
> +
> +static __be32
> nfsd4_decode_seek(struct nfsd4_compoundargs *argp, struct nfsd4_seek *seek)
> {
> DECODE_HEAD;
> @@ -1604,7 +1621,7 @@ static nfsd4_dec nfsd4_dec_ops[] = {
> [OP_RECLAIM_COMPLETE] = (nfsd4_dec)nfsd4_decode_reclaim_complete,
>
> /* new operations for NFSv4.2 */
> - [OP_ALLOCATE] = (nfsd4_dec)nfsd4_decode_notsupp,
> + [OP_ALLOCATE] = (nfsd4_dec)nfsd4_decode_fallocate,
> [OP_COPY] = (nfsd4_dec)nfsd4_decode_notsupp,
> [OP_COPY_NOTIFY] = (nfsd4_dec)nfsd4_decode_notsupp,
> [OP_DEALLOCATE] = (nfsd4_dec)nfsd4_decode_notsupp,
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 989129e..9d29631 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -16,6 +16,7 @@
> #include <linux/fs.h>
> #include <linux/file.h>
> #include <linux/splice.h>
> +#include <linux/falloc.h>
> #include <linux/fcntl.h>
> #include <linux/namei.h>
> #include <linux/delay.h>
> @@ -533,6 +534,25 @@ __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
> }
> #endif
>
> +__be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh *fhp,
> + struct file *file, loff_t offset, loff_t len,
> + int flags)
> +{
> + __be32 err;
> + int error;
> +
> + err = nfsd_permission(rqstp, fhp->fh_export, fhp->fh_dentry, NFSD_MAY_WRITE);
> + if (err)
> + return err;
> +
> + error = vfs_fallocate(file, flags, offset, len);
> + if (!error)
> + error = commit_metadata(fhp);
> +
> + if (error == -ENODEV)
> + return nfserr_inval;

If I'm reading the code right, this is the case where we try to
fallocate on something other than a regular file?

Actually the vfs code has a note syaing filesystems may allow fallocate
on directories. Do we? There's nothing explicit in the spec that I can
see but I don't think it'd make sense.

So how about replacing this by an explicit check for the type before the
fallocate call?

(And also add a note to the spec requiring NFS4ERR_INVAL on anything not
a regular file.)

Otherwise these patches look fine.

--b.

> + return nfserrno(error);
> +}
> #endif /* defined(CONFIG_NFSD_V4) */
>
> #ifdef CONFIG_NFSD_V3
> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> index c2ff3f1..7ffdb14 100644
> --- a/fs/nfsd/vfs.h
> +++ b/fs/nfsd/vfs.h
> @@ -54,6 +54,8 @@ int nfsd_mountpoint(struct dentry *, struct svc_export *);
> #ifdef CONFIG_NFSD_V4
> __be32 nfsd4_set_nfs4_label(struct svc_rqst *, struct svc_fh *,
> struct xdr_netobj *);
> +__be32 nfsd4_vfs_fallocate(struct svc_rqst *, struct svc_fh *,
> + struct file *, loff_t, loff_t, int);
> #endif /* CONFIG_NFSD_V4 */
> __be32 nfsd_create(struct svc_rqst *, struct svc_fh *,
> char *name, int len, struct iattr *attrs,
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index 5720e94..eeaa0d0 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -428,6 +428,13 @@ struct nfsd4_reclaim_complete {
> u32 rca_one_fs;
> };
>
> +struct nfsd4_fallocate {
> + /* request */
> + stateid_t falloc_stateid;
> + loff_t falloc_offset;
> + u64 falloc_length;
> +};
> +
> struct nfsd4_seek {
> /* request */
> stateid_t seek_stateid;
> @@ -486,6 +493,7 @@ struct nfsd4_op {
> struct nfsd4_free_stateid free_stateid;
>
> /* NFSv4.2 */
> + struct nfsd4_fallocate allocate;
> struct nfsd4_seek seek;
> } u;
> struct nfs4_replay * replay;
> --
> 2.1.3
>