2014-10-24 14:41:48

by Anna Schumaker

[permalink] [raw]
Subject: [PATCH v2 0/2] NFSD: Add ALLOCATE and DEALLOCATE support

From: Anna Schumaker <[email protected]>

These patches add server support for the ALLOCATE and DEALLOCATE operations
part of NFS v4.2.

I submitted my do_fallocate() -> vfs_fallocate() change as a separate patch,
so it has been removed from this submission. However, this code still depends
on this patch being applied first.

Changes in v2:
- ALLOCATE and DEALLOCATE can call a common nfsd4_fallocate() to check the
stateids, look up the file, and eventually call nfsd4_vfs_fallocate().
- I return NFS4ERR_INVAL in the case that vfs_fallocate() returns -ENODEV.
Please let me know if there is a better error to return instead!

These patches and the corresponding client changes are available in the
[fallocate] branch of:

git://git.linux-nfs.org/projects/anna/linux-nfs.git


Questions? Comments? Thoughts?

Anna


Anna Schumaker (2):
nfsd: Add ALLOCATE support
nfsd: Add DEALLOCATE support

fs/nfsd/nfs4proc.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
fs/nfsd/nfs4xdr.c | 21 +++++++++++++++++++--
fs/nfsd/vfs.c | 19 +++++++++++++++++++
fs/nfsd/vfs.h | 2 ++
fs/nfsd/xdr4.h | 9 +++++++++
5 files changed, 100 insertions(+), 2 deletions(-)

--
2.1.2



2014-10-28 19:09:24

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] nfsd: Add ALLOCATE support

On 10/28/2014 04:41 AM, Christoph Hellwig wrote:
>> + error = vfs_fallocate(file, flags, offset, len);
>> + if (error == -ENODEV)
>> + return nfserr_inval;
>> + else if (error < 0)
>> + return nfserrno(error);
>> + return nfserrno(commit_metadata(fhp));
>
>
> Shouldn't this be something like:
>
> error = vfs_fallocate(file, flags, offset, len);
> if (!error)
> error = commit_metadata(fhp);
>
> if (error == -ENODEV)
> return nfserr_inval;
> return nfserrno(error);

Sure. I'll post another version in the next few days (with the other patch included).

Thanks for the feedback!
Anna

>
> I think we really should move the ENODEV mapping to nfserrno, but
> this patch isn't the right place for that.
>


2014-10-24 14:41:50

by Anna Schumaker

[permalink] [raw]
Subject: [PATCH v2 2/2] nfsd: Add DEALLOCATE support

From: Anna Schumaker <[email protected]>

DEALLOCATE only returns a status value, meaning we can use the noop()
xdr encoder to reply to the client.

Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfsd/nfs4proc.c | 15 +++++++++++++++
fs/nfsd/nfs4xdr.c | 2 +-
fs/nfsd/xdr4.h | 1 +
3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 57bfcad..b3a9d0d 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -33,6 +33,7 @@
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#include <linux/file.h>
+#include <linux/falloc.h>
#include <linux/slab.h>

#include "idmap.h"
@@ -1044,6 +1045,14 @@ nfsd4_allocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
}

static __be32
+nfsd4_deallocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+ struct nfsd4_fallocate *fallocate)
+{
+ return nfsd4_fallocate(rqstp, cstate, fallocate,
+ FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE);
+}
+
+static __be32
nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfsd4_seek *seek)
{
@@ -1962,6 +1971,12 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_name = "OP_ALLOCATE",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_write_rsize,
},
+ [OP_DEALLOCATE] = {
+ .op_func = (nfsd4op_func)nfsd4_deallocate,
+ .op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
+ .op_name = "OP_DEALLOCATE",
+ .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 a60cff8..0622d4f 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1624,7 +1624,7 @@ static nfsd4_dec nfsd4_dec_ops[] = {
[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,
+ [OP_DEALLOCATE] = (nfsd4_dec)nfsd4_decode_fallocate,
[OP_IO_ADVISE] = (nfsd4_dec)nfsd4_decode_notsupp,
[OP_LAYOUTERROR] = (nfsd4_dec)nfsd4_decode_notsupp,
[OP_LAYOUTSTATS] = (nfsd4_dec)nfsd4_decode_notsupp,
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index eeaa0d0..90a5925 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -494,6 +494,7 @@ struct nfsd4_op {

/* NFSv4.2 */
struct nfsd4_fallocate allocate;
+ struct nfsd4_fallocate deallocate;
struct nfsd4_seek seek;
} u;
struct nfs4_replay * replay;
--
2.1.2


2014-10-28 08:37:22

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] NFSD: Add ALLOCATE and DEALLOCATE support

On Fri, Oct 24, 2014 at 10:41:43AM -0400, [email protected] wrote:
> From: Anna Schumaker <[email protected]>
>
> These patches add server support for the ALLOCATE and DEALLOCATE operations
> part of NFS v4.2.
>
> I submitted my do_fallocate() -> vfs_fallocate() change as a separate patch,
> so it has been removed from this submission. However, this code still depends
> on this patch being applied first.

Please inlude the patch to make everyones life easier. Al, can you
please ack the patch so that we can move forward with this series?


2014-10-28 20:48:35

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] NFSD: Add ALLOCATE and DEALLOCATE support

On Fri, Oct 24, 2014 at 10:41:43AM -0400, [email protected] wrote:
> From: Anna Schumaker <[email protected]>
>
> These patches add server support for the ALLOCATE and DEALLOCATE operations
> part of NFS v4.2.

Thanks!

What are you using for testing?

--b.

>
> I submitted my do_fallocate() -> vfs_fallocate() change as a separate patch,
> so it has been removed from this submission. However, this code still depends
> on this patch being applied first.
>
> Changes in v2:
> - ALLOCATE and DEALLOCATE can call a common nfsd4_fallocate() to check the
> stateids, look up the file, and eventually call nfsd4_vfs_fallocate().
> - I return NFS4ERR_INVAL in the case that vfs_fallocate() returns -ENODEV.
> Please let me know if there is a better error to return instead!
>
> These patches and the corresponding client changes are available in the
> [fallocate] branch of:
>
> git://git.linux-nfs.org/projects/anna/linux-nfs.git
>
>
> Questions? Comments? Thoughts?
>
> Anna
>
>
> Anna Schumaker (2):
> nfsd: Add ALLOCATE support
> nfsd: Add DEALLOCATE support
>
> fs/nfsd/nfs4proc.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/nfs4xdr.c | 21 +++++++++++++++++++--
> fs/nfsd/vfs.c | 19 +++++++++++++++++++
> fs/nfsd/vfs.h | 2 ++
> fs/nfsd/xdr4.h | 9 +++++++++
> 5 files changed, 100 insertions(+), 2 deletions(-)
>
> --
> 2.1.2
>

2014-10-24 14:41:49

by Anna Schumaker

[permalink] [raw]
Subject: [PATCH v2 1/2] nfsd: Add ALLOCATE support

From: Anna Schumaker <[email protected]>

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 <[email protected]>
---
fs/nfsd/nfs4proc.c | 36 ++++++++++++++++++++++++++++++++++++
fs/nfsd/nfs4xdr.c | 19 ++++++++++++++++++-
fs/nfsd/vfs.c | 19 +++++++++++++++++++
fs/nfsd/vfs.h | 2 ++
fs/nfsd/xdr4.h | 8 ++++++++
5 files changed, 83 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..9a38e87 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,24 @@ __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 == -ENODEV)
+ return nfserr_inval;
+ else if (error < 0)
+ return nfserrno(error);
+ return nfserrno(commit_metadata(fhp));
+}
#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.2


2014-10-28 08:41:09

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] nfsd: Add ALLOCATE support

> + error = vfs_fallocate(file, flags, offset, len);
> + if (error == -ENODEV)
> + return nfserr_inval;
> + else if (error < 0)
> + return nfserrno(error);
> + return nfserrno(commit_metadata(fhp));


Shouldn't this be something like:

error = vfs_fallocate(file, flags, offset, len);
if (!error)
error = commit_metadata(fhp);

if (error == -ENODEV)
return nfserr_inval;
return nfserrno(error);

I think we really should move the ENODEV mapping to nfserrno, but
this patch isn't the right place for that.