Discussion:
[PATCH v2 0/2] NFSD: Add v4.2 SEEK support
A***@public.gmane.org
2014-09-02 17:30:36 UTC
Permalink
From: Anna Schumaker <Anna.Schumaker-ZwjVKphTwtPQT0dZR+***@public.gmane.org>

These patches add server support for the NFS v4.2 operation SEEK. The first
patch adds basic NFS v4.2 infrastructure that all future operations will rely
on. The second patch adds the SEEK operation.

Changes in v2:
- Remove CONFIG_NFSD_V4_2_SEEK.
- Change the call to preprocess_stateid_op() to only use RD_STATE instead of
RD_STATE | WR_STATE.

These patches and the corresponding client patch are avaliable in the [seek]
branch of:

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


Questions? Comments? Thoughts?

Anna


Anna Schumaker (2):
NFSD: Add generic v4.2 infrastructure
NFSD: Implement SEEK

fs/nfsd/nfs4proc.c | 43 +++++++++++++++++++++++++++++++++++++
fs/nfsd/nfs4xdr.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++
fs/nfsd/xdr4.h | 14 ++++++++++++
include/linux/nfs4.h | 23 ++++++++++++++++++--
4 files changed, 138 insertions(+), 2 deletions(-)
--
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
A***@public.gmane.org
2014-09-02 17:30:37 UTC
Permalink
From: Anna Schumaker <Anna.Schumaker-HgOvQuBEEgTQT0dZR+***@public.gmane.org>

It's cleaner to introduce everything at once and have the server reply
with "not supported" than it would be to introduce extra operations when
implementing a specific one in the middle of the list.

Signed-off-by: Anna Schumaker <Anna.Schumaker-ZwjVKphTwtPQT0dZR+***@public.gmane.org>
---
fs/nfsd/nfs4xdr.c | 28 ++++++++++++++++++++++++++++
include/linux/nfs4.h | 18 ++++++++++++++++--
2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index f9821ce..94dde7b 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1593,6 +1593,20 @@ static nfsd4_dec nfsd4_dec_ops[] = {
[OP_WANT_DELEGATION] = (nfsd4_dec)nfsd4_decode_notsupp,
[OP_DESTROY_CLIENTID] = (nfsd4_dec)nfsd4_decode_destroy_clientid,
[OP_RECLAIM_COMPLETE] = (nfsd4_dec)nfsd4_decode_reclaim_complete,
+
+ /* new operations for NFSv4.2 */
+ [OP_ALLOCATE] = (nfsd4_dec)nfsd4_decode_notsupp,
+ [OP_COPY] = (nfsd4_dec)nfsd4_decode_notsupp,
+ [OP_COPY_NOTIFY] = (nfsd4_dec)nfsd4_decode_notsupp,
+ [OP_DEALLOCATE] = (nfsd4_dec)nfsd4_decode_notsupp,
+ [OP_IO_ADVISE] = (nfsd4_dec)nfsd4_decode_notsupp,
+ [OP_LAYOUTERROR] = (nfsd4_dec)nfsd4_decode_notsupp,
+ [OP_LAYOUTSTATS] = (nfsd4_dec)nfsd4_decode_notsupp,
+ [OP_OFFLOAD_CANCEL] = (nfsd4_dec)nfsd4_decode_notsupp,
+ [OP_OFFLOAD_STATUS] = (nfsd4_dec)nfsd4_decode_notsupp,
+ [OP_READ_PLUS] = (nfsd4_dec)nfsd4_decode_notsupp,
+ [OP_SEEK] = (nfsd4_dec)nfsd4_decode_notsupp,
+ [OP_WRITE_SAME] = (nfsd4_dec)nfsd4_decode_notsupp,
};

static inline bool
@@ -3822,6 +3836,20 @@ static nfsd4_enc nfsd4_enc_ops[] = {
[OP_WANT_DELEGATION] = (nfsd4_enc)nfsd4_encode_noop,
[OP_DESTROY_CLIENTID] = (nfsd4_enc)nfsd4_encode_noop,
[OP_RECLAIM_COMPLETE] = (nfsd4_enc)nfsd4_encode_noop,
+
+ /* NFSv4.2 operations */
+ [OP_ALLOCATE] = (nfsd4_enc)nfsd4_encode_noop,
+ [OP_COPY] = (nfsd4_enc)nfsd4_encode_noop,
+ [OP_COPY_NOTIFY] = (nfsd4_enc)nfsd4_encode_noop,
+ [OP_DEALLOCATE] = (nfsd4_enc)nfsd4_encode_noop,
+ [OP_IO_ADVISE] = (nfsd4_enc)nfsd4_encode_noop,
+ [OP_LAYOUTERROR] = (nfsd4_enc)nfsd4_encode_noop,
+ [OP_LAYOUTSTATS] = (nfsd4_enc)nfsd4_encode_noop,
+ [OP_OFFLOAD_CANCEL] = (nfsd4_enc)nfsd4_encode_noop,
+ [OP_OFFLOAD_STATUS] = (nfsd4_enc)nfsd4_encode_noop,
+ [OP_READ_PLUS] = (nfsd4_enc)nfsd4_encode_noop,
+ [OP_SEEK] = (nfsd4_enc)nfsd4_encode_noop,
+ [OP_WRITE_SAME] = (nfsd4_enc)nfsd4_encode_noop,
};

/*
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 79b2a0f..cf38224 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -110,6 +110,20 @@ enum nfs_opnum4 {
OP_DESTROY_CLIENTID = 57,
OP_RECLAIM_COMPLETE = 58,

+ /* nfs42 */
+ OP_ALLOCATE = 59,
+ OP_COPY = 60,
+ OP_COPY_NOTIFY = 61,
+ OP_DEALLOCATE = 62,
+ OP_IO_ADVISE = 63,
+ OP_LAYOUTERROR = 64,
+ OP_LAYOUTSTATS = 65,
+ OP_OFFLOAD_CANCEL = 66,
+ OP_OFFLOAD_STATUS = 67,
+ OP_READ_PLUS = 68,
+ OP_SEEK = 69,
+ OP_WRITE_SAME = 70,
+
OP_ILLEGAL = 10044,
};

@@ -117,10 +131,10 @@ enum nfs_opnum4 {
Needs to be updated if more operations are defined in future.*/

#define FIRST_NFS4_OP OP_ACCESS
-#define LAST_NFS4_OP OP_RECLAIM_COMPLETE
+#define LAST_NFS4_OP OP_WRITE_SAME
#define LAST_NFS40_OP OP_RELEASE_LOCKOWNER
#define LAST_NFS41_OP OP_RECLAIM_COMPLETE
-#define LAST_NFS42_OP OP_RECLAIM_COMPLETE
+#define LAST_NFS42_OP OP_WRITE_SAME

enum nfsstat4 {
NFS4_OK = 0,
--
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
2014-09-09 17:53:37 UTC
Permalink
Post by A***@public.gmane.org
It's cleaner to introduce everything at once and have the server reply
with "not supported" than it would be to introduce extra operations when
implementing a specific one in the middle of the list.
Looks good,

Reviewed-by: Christoph Hellwig <hch-***@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
A***@public.gmane.org
2014-09-02 17:30:38 UTC
Permalink
From: Anna Schumaker <Anna.Schumaker-HgOvQuBEEgTQT0dZR+***@public.gmane.org>

This patch adds server support for the NFS v4.2 operation SEEK, which
returns the position of the next hole or data segment in a file.

Signed-off-by: Anna Schumaker <Anna.Schumaker-ZwjVKphTwtPQT0dZR+***@public.gmane.org>
---
fs/nfsd/nfs4proc.c | 43 +++++++++++++++++++++++++++++++++++++++++++
fs/nfsd/nfs4xdr.c | 36 ++++++++++++++++++++++++++++++++++--
fs/nfsd/xdr4.h | 14 ++++++++++++++
include/linux/nfs4.h | 5 +++++
4 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 5e0dc52..aed5b88 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1013,6 +1013,43 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
return status;
}

+static __be32
+nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+ struct nfsd4_seek *seek)
+{
+ struct file *file;
+ __be32 status = nfs_ok;
+
+ status = nfs4_preprocess_stateid_op(SVC_NET(rqstp), cstate,
+ &seek->seek_stateid,
+ RD_STATE, &file);
+ if (status) {
+ dprintk("NFSD: nfsd4_seek: couldn't process stateid!\n");
+ return status;
+ }
+
+ switch (seek->seek_whence) {
+ case NFS4_CONTENT_DATA:
+ seek->seek_pos = vfs_llseek(file, seek->seek_offset, SEEK_DATA);
+ break;
+ case NFS4_CONTENT_HOLE:
+ seek->seek_pos = vfs_llseek(file, seek->seek_offset, SEEK_HOLE);
+ break;
+ default:
+ status = nfserr_union_notsupp;
+ goto out;
+ }
+
+ if (seek->seek_pos < 0)
+ status = nfserrno(seek->seek_pos);
+ else if (seek->seek_pos >= i_size_read(file_inode(file)))
+ seek->seek_eof = true;
+
+out:
+ fput(file);
+ return status;
+}
+
/* This routine never returns NFS_OK! If there are no other errors, it
* will return NFSERR_SAME or NFSERR_NOT_SAME depending on whether the
* attributes matched. VERIFY is implemented by mapping NFSERR_SAME
@@ -1881,6 +1918,12 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_get_currentstateid = (stateid_getter)nfsd4_get_freestateid,
.op_rsize_bop = (nfsd4op_rsize)nfsd4_only_status_rsize,
},
+
+ /* NFSv4.2 operations */
+ [OP_SEEK] = {
+ .op_func = (nfsd4op_func)nfsd4_seek,
+ .op_name = "OP_SEEK",
+ },
};

int nfsd4_max_reply(struct svc_rqst *rqstp, struct nfsd4_op *op)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 94dde7b..57b3973 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1521,6 +1521,22 @@ static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, str
}

static __be32
+nfsd4_decode_seek(struct nfsd4_compoundargs *argp, struct nfsd4_seek *seek)
+{
+ DECODE_HEAD;
+
+ status = nfsd4_decode_stateid(argp, &seek->seek_stateid);
+ if (status)
+ return status;
+
+ READ_BUF(12);
+ p = xdr_decode_hyper(p, &seek->seek_offset);
+ seek->seek_whence = be32_to_cpup(p);
+
+ DECODE_TAIL;
+}
+
+static __be32
nfsd4_decode_noop(struct nfsd4_compoundargs *argp, void *p)
{
return nfs_ok;
@@ -1605,7 +1621,7 @@ static nfsd4_dec nfsd4_dec_ops[] = {
[OP_OFFLOAD_CANCEL] = (nfsd4_dec)nfsd4_decode_notsupp,
[OP_OFFLOAD_STATUS] = (nfsd4_dec)nfsd4_decode_notsupp,
[OP_READ_PLUS] = (nfsd4_dec)nfsd4_decode_notsupp,
- [OP_SEEK] = (nfsd4_dec)nfsd4_decode_notsupp,
+ [OP_SEEK] = (nfsd4_dec)nfsd4_decode_seek,
[OP_WRITE_SAME] = (nfsd4_dec)nfsd4_decode_notsupp,
};

@@ -3765,6 +3781,22 @@ nfsd4_encode_test_stateid(struct nfsd4_compoundres *resp, __be32 nfserr,
}

static __be32
+nfsd4_encode_seek(struct nfsd4_compoundres *resp, __be32 nfserr,
+ struct nfsd4_seek *seek)
+{
+ __be32 *p;
+
+ if (nfserr)
+ return nfserr;
+
+ p = xdr_reserve_space(&resp->xdr, 12);
+ *p++ = cpu_to_be32(seek->seek_eof);
+ p = xdr_encode_hyper(p, seek->seek_pos);
+
+ return nfserr;
+}
+
+static __be32
nfsd4_encode_noop(struct nfsd4_compoundres *resp, __be32 nfserr, void *p)
{
return nfserr;
@@ -3848,7 +3880,7 @@ static nfsd4_enc nfsd4_enc_ops[] = {
[OP_OFFLOAD_CANCEL] = (nfsd4_enc)nfsd4_encode_noop,
[OP_OFFLOAD_STATUS] = (nfsd4_enc)nfsd4_encode_noop,
[OP_READ_PLUS] = (nfsd4_enc)nfsd4_encode_noop,
- [OP_SEEK] = (nfsd4_enc)nfsd4_encode_noop,
+ [OP_SEEK] = (nfsd4_enc)nfsd4_encode_seek,
[OP_WRITE_SAME] = (nfsd4_enc)nfsd4_encode_noop,
};

diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 465e779..5720e94 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -428,6 +428,17 @@ struct nfsd4_reclaim_complete {
u32 rca_one_fs;
};

+struct nfsd4_seek {
+ /* request */
+ stateid_t seek_stateid;
+ loff_t seek_offset;
+ u32 seek_whence;
+
+ /* response */
+ u32 seek_eof;
+ loff_t seek_pos;
+};
+
struct nfsd4_op {
int opnum;
__be32 status;
@@ -473,6 +484,9 @@ struct nfsd4_op {
struct nfsd4_reclaim_complete reclaim_complete;
struct nfsd4_test_stateid test_stateid;
struct nfsd4_free_stateid free_stateid;
+
+ /* NFSv4.2 */
+ struct nfsd4_seek seek;
} u;
struct nfs4_replay * replay;
};
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index cf38224..026b0c0 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -550,4 +550,9 @@ struct nfs4_deviceid {
char data[NFS4_DEVICEID4_SIZE];
};

+enum data_content4 {
+ NFS4_CONTENT_DATA = 0,
+ NFS4_CONTENT_HOLE = 1,
+};
+
#endif
--
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
2014-09-09 17:56:07 UTC
Permalink
Post by A***@public.gmane.org
+ switch (seek->seek_whence) {
+ seek->seek_pos = vfs_llseek(file, seek->seek_offset, SEEK_DATA);
+ break;
+ seek->seek_pos = vfs_llseek(file, seek->seek_offset, SEEK_HOLE);
+ break;
+ status = nfserr_union_notsupp;
+ goto out;
+ }
nipick: maybe just assign pos in the switch, and have a single
vfs_llseek call?

Also this might want a comment that vfs_llseek is changing file->f_pos,
but nothing in NFSD should ever rely on file->f_pos.
Post by A***@public.gmane.org
static __be32
+nfsd4_encode_seek(struct nfsd4_compoundres *resp, __be32 nfserr,
+ struct nfsd4_seek *seek)
+{
+ __be32 *p;
+
+ if (nfserr)
+ return nfserr;
+
+ p = xdr_reserve_space(&resp->xdr, 12);
nipick: can you replace the 12 by a "4 + 8"? I think having one
literal for each field later encoded helps reading the XDR code a lot.
And although it might not matter for a trivial encoder like this it
set standards for future copy and paste code.

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Anna Schumaker
2014-09-09 19:17:45 UTC
Permalink
Post by Christoph Hellwig
Post by A***@public.gmane.org
+ switch (seek->seek_whence) {
+ seek->seek_pos = vfs_llseek(file, seek->seek_offset, SEEK_DATA);
+ break;
+ seek->seek_pos = vfs_llseek(file, seek->seek_offset, SEEK_HOLE);
+ break;
+ status = nfserr_union_notsupp;
+ goto out;
+ }
nipick: maybe just assign pos in the switch, and have a single
vfs_llseek call?
Also this might want a comment that vfs_llseek is changing file->f_pos,
but nothing in NFSD should ever rely on file->f_pos.
Sure.
Post by Christoph Hellwig
Post by A***@public.gmane.org
static __be32
+nfsd4_encode_seek(struct nfsd4_compoundres *resp, __be32 nfserr,
+ struct nfsd4_seek *seek)
+{
+ __be32 *p;
+
+ if (nfserr)
+ return nfserr;
+
+ p = xdr_reserve_space(&resp->xdr, 12);
nipick: can you replace the 12 by a "4 + 8"? I think having one
literal for each field later encoded helps reading the XDR code a lot.
And although it might not matter for a trivial encoder like this it
set standards for future copy and paste code.
I can change that, too. Thanks for looking!

Anna
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
J. Bruce Fields
2014-09-02 19:10:43 UTC
Permalink
Post by A***@public.gmane.org
These patches add server support for the NFS v4.2 operation SEEK. The first
patch adds basic NFS v4.2 infrastructure that all future operations will rely
on. The second patch adds the SEEK operation.
- Remove CONFIG_NFSD_V4_2_SEEK.
- Change the call to preprocess_stateid_op() to only use RD_STATE instead of
RD_STATE | WR_STATE.
These patches and the corresponding client patch are avaliable in the [seek]
git://git.linux-nfs.org/projects/anna/linux-nfs.git
Questions? Comments? Thoughts?
They look OK to me. I'll probably merge them once there's evidence
they're likely to be merged on the client side too.

Also: has anyone done wireshark support for this yet?

--b.
Post by A***@public.gmane.org
Anna
NFSD: Add generic v4.2 infrastructure
NFSD: Implement SEEK
fs/nfsd/nfs4proc.c | 43 +++++++++++++++++++++++++++++++++++++
fs/nfsd/nfs4xdr.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++
fs/nfsd/xdr4.h | 14 ++++++++++++
include/linux/nfs4.h | 23 ++++++++++++++++++--
4 files changed, 138 insertions(+), 2 deletions(-)
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
J. Bruce Fields
2014-09-22 17:53:33 UTC
Permalink
Post by J. Bruce Fields
Post by A***@public.gmane.org
These patches add server support for the NFS v4.2 operation SEEK. The first
patch adds basic NFS v4.2 infrastructure that all future operations will rely
on. The second patch adds the SEEK operation.
- Remove CONFIG_NFSD_V4_2_SEEK.
- Change the call to preprocess_stateid_op() to only use RD_STATE instead of
RD_STATE | WR_STATE.
These patches and the corresponding client patch are avaliable in the [seek]
git://git.linux-nfs.org/projects/anna/linux-nfs.git
Questions? Comments? Thoughts?
They look OK to me. I'll probably merge them once there's evidence
they're likely to be merged on the client side too.
Also: has anyone done wireshark support for this yet?
Are you sending a new version of these?

(And ditto for the allocate patches.)

As far as I can tell review comments were mostly nits.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...