2014-08-27 15:18:03

by Anna Schumaker

[permalink] [raw]
Subject: [PATCH 0/3] NFSD: Add v4.2 SEEK support

From: Anna Schumaker <[email protected]>

These patches add server support for the NFS v4.2 operation SEEK. The first
two patches are error cleanups and infrastructure setup to prepare for the
addition of new operations.

My biggest concern is that these patches freeze the v4.2 opcodes to their
current values, which makes any future spec changes to add or remove
operations more difficult to accomidate.

Questions? Comments? Thoughts?

Anna


Anna Schumaker (3):
NFSD: Update error codes
NFSD: Create nfs v4.2 decode ops
NFSD: Implement SEEK

fs/nfsd/Kconfig | 12 +++++++++
fs/nfsd/nfs4proc.c | 47 +++++++++++++++++++++++++++++++++
fs/nfsd/nfs4xdr.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
fs/nfsd/nfsd.h | 2 +-
fs/nfsd/xdr4.h | 14 ++++++++++
include/linux/nfs4.h | 26 ++++++++++++++++---
6 files changed, 170 insertions(+), 4 deletions(-)

--
2.1.0



2014-08-27 19:29:52

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 3/3] NFSD: Implement SEEK

On Wed, Aug 27, 2014 at 03:21:22PM -0400, Anna Schumaker wrote:
> On 08/27/2014 03:09 PM, J. Bruce Fields wrote:
> > On Wed, Aug 27, 2014 at 11:17:58AM -0400, [email protected] wrote:
> >> From: Anna Schumaker <[email protected]>
> >>
> >> 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. This
> >> operation is off by default, and needs CONFIG_NFSD_V4_2_SEEK=y to be
> >> compiled.
> >>
> >> Signed-off-by: Anna Schumaker <[email protected]>
> >> ---
> >> fs/nfsd/Kconfig | 12 ++++++++++++
> >> fs/nfsd/nfs4proc.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> >> fs/nfsd/nfs4xdr.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> >> fs/nfsd/xdr4.h | 14 ++++++++++++++
> >> include/linux/nfs4.h | 5 +++++
> >> 5 files changed, 123 insertions(+)
> >>
> >> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> >> index f994e75..804b20a 100644
> >> --- a/fs/nfsd/Kconfig
> >> +++ b/fs/nfsd/Kconfig
> >> @@ -81,6 +81,18 @@ config NFSD_V4
> >>
> >> If unsure, say N.
> >>
> >> +config NFSD_V4_2_SEEK
> >> + bool "Enable SEEK support for the NFS v4.2 server"
> >> + depends on NFSD_V4
> >> + help
> >> + Say Y here if you want to enable support for the NFS v4.2 operation
> >> + SEEK, which adds in SEEK_HOLE and SEEK_DATA support.
> >> +
> >> + WARNING: there is still a chance of backwards-incompatible protocol
> >> + changes. This feature is targeted at developers and testers only.
> >
> > I think now that we should instead confirm with the working group that
> > backwards-incompatible changes are done, and skip this warning.
> >
> > I'll remove it in the labeled NFS case, which we've decided is "done"
> > even if the rest of the 4.2 draft isn't.
> >
> > I also wonder if we should ditch these configuration options, or keep
> > them only temporarily. It's not much code, so I think the only reason
> > to allow configuring it out is to temporarily protect people from
> > immature code.
>
> I was expecting them to be temporary, and to get removed when the draft becomes an RFC.
>
> >
> >> +
> >> + If unsure, say N.
> >> +
> >> config NFSD_V4_SECURITY_LABEL
> >> bool "Provide Security Label support for NFSv4 server"
> >> depends on NFSD_V4 && SECURITY
> >> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> >> index 5e0dc52..f555eb2 100644
> >> --- a/fs/nfsd/nfs4proc.c
> >> +++ b/fs/nfsd/nfs4proc.c
> >> @@ -1013,6 +1013,45 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> >> return status;
> >> }
> >>
> >> +#ifdef CONFIG_NFSD_V4_2_SEEK
> >> +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 | WR_STATE, &file);
> >
> > I think that should be just RD_STATE.
>
> I was trying to cover the case where an application does:
>
> open(WRITE);
> seek(HOLE);
> write("blah");
>
> I can change the code if that's not the way people use SEEK_HOLE / SEEK_DATA ...

Note nfsd4_read() for example passes RD_STATE here. It means "I'm doing
a read-like operation", not "I can only use a read open".

--b.

2014-08-27 17:42:34

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 2/3] NFSD: Create nfs v4.2 decode ops

On Wed, Aug 27, 2014 at 11:17:57AM -0400, [email protected] wrote:
> From: Anna Schumaker <[email protected]>
>
> 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.

What prevents a client from calling the new operations on a 4.1 session?


2014-08-27 18:04:21

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 2/3] NFSD: Create nfs v4.2 decode ops

On Wed, Aug 27, 2014 at 10:42:31AM -0700, Christoph Hellwig wrote:
> On Wed, Aug 27, 2014 at 11:17:57AM -0400, [email protected] wrote:
> > From: Anna Schumaker <[email protected]>
> >
> > 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.
>
> What prevents a client from calling the new operations on a 4.1 session?

fs/nfsd/nfs4xdr.c:nfsd4_opnum_in_range(), which just uses minorversion
and the LAST_NFS4?_OP constants.

--b.

2014-08-27 15:18:04

by Anna Schumaker

[permalink] [raw]
Subject: [PATCH 1/3] NFSD: Update error codes

From: Anna Schumaker <[email protected]>

Recent NFS v4.2 drafts have removed NFS4ERR_METADATA_NOTSUPP and
reassigned the error code to NFS4ERR_UNION_NOTSUPP.

I also add in the NFS4ERR_OFFLOAD_NO_REQS error code.

Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfsd/nfsd.h | 2 +-
include/linux/nfs4.h | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 847daf3..747f3b95 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -251,7 +251,7 @@ void nfsd_lockd_shutdown(void);
#define nfserr_deleg_revoked cpu_to_be32(NFS4ERR_DELEG_REVOKED)
#define nfserr_partner_notsupp cpu_to_be32(NFS4ERR_PARTNER_NOTSUPP)
#define nfserr_partner_no_auth cpu_to_be32(NFS4ERR_PARTNER_NO_AUTH)
-#define nfserr_metadata_notsupp cpu_to_be32(NFS4ERR_METADATA_NOTSUPP)
+#define nfserr_union_notsupp cpu_to_be32(NFS4ERR_UNION_NOTSUPP)
#define nfserr_offload_denied cpu_to_be32(NFS4ERR_OFFLOAD_DENIED)
#define nfserr_wrong_lfs cpu_to_be32(NFS4ERR_WRONG_LFS)
#define nfserr_badlabel cpu_to_be32(NFS4ERR_BADLABEL)
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index a1e3064..79b2a0f 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -235,10 +235,11 @@ enum nfsstat4 {
/* nfs42 */
NFS4ERR_PARTNER_NOTSUPP = 10088,
NFS4ERR_PARTNER_NO_AUTH = 10089,
- NFS4ERR_METADATA_NOTSUPP = 10090,
+ NFS4ERR_UNION_NOTSUPP = 10090,
NFS4ERR_OFFLOAD_DENIED = 10091,
NFS4ERR_WRONG_LFS = 10092,
NFS4ERR_BADLABEL = 10093,
+ NFS4ERR_OFFLOAD_NO_REQS = 10094,
};

static inline bool seqid_mutating_err(u32 err)
--
2.1.0


2014-08-27 15:18:06

by Anna Schumaker

[permalink] [raw]
Subject: [PATCH 2/3] NFSD: Create nfs v4.2 decode ops

From: Anna Schumaker <[email protected]>

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 <[email protected]>
---
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


2014-08-27 19:21:32

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH 3/3] NFSD: Implement SEEK

On 08/27/2014 03:09 PM, J. Bruce Fields wrote:
> On Wed, Aug 27, 2014 at 11:17:58AM -0400, [email protected] wrote:
>> From: Anna Schumaker <[email protected]>
>>
>> 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. This
>> operation is off by default, and needs CONFIG_NFSD_V4_2_SEEK=y to be
>> compiled.
>>
>> Signed-off-by: Anna Schumaker <[email protected]>
>> ---
>> fs/nfsd/Kconfig | 12 ++++++++++++
>> fs/nfsd/nfs4proc.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>> fs/nfsd/nfs4xdr.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>> fs/nfsd/xdr4.h | 14 ++++++++++++++
>> include/linux/nfs4.h | 5 +++++
>> 5 files changed, 123 insertions(+)
>>
>> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
>> index f994e75..804b20a 100644
>> --- a/fs/nfsd/Kconfig
>> +++ b/fs/nfsd/Kconfig
>> @@ -81,6 +81,18 @@ config NFSD_V4
>>
>> If unsure, say N.
>>
>> +config NFSD_V4_2_SEEK
>> + bool "Enable SEEK support for the NFS v4.2 server"
>> + depends on NFSD_V4
>> + help
>> + Say Y here if you want to enable support for the NFS v4.2 operation
>> + SEEK, which adds in SEEK_HOLE and SEEK_DATA support.
>> +
>> + WARNING: there is still a chance of backwards-incompatible protocol
>> + changes. This feature is targeted at developers and testers only.
>
> I think now that we should instead confirm with the working group that
> backwards-incompatible changes are done, and skip this warning.
>
> I'll remove it in the labeled NFS case, which we've decided is "done"
> even if the rest of the 4.2 draft isn't.
>
> I also wonder if we should ditch these configuration options, or keep
> them only temporarily. It's not much code, so I think the only reason
> to allow configuring it out is to temporarily protect people from
> immature code.

I was expecting them to be temporary, and to get removed when the draft becomes an RFC.

>
>> +
>> + If unsure, say N.
>> +
>> config NFSD_V4_SECURITY_LABEL
>> bool "Provide Security Label support for NFSv4 server"
>> depends on NFSD_V4 && SECURITY
>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>> index 5e0dc52..f555eb2 100644
>> --- a/fs/nfsd/nfs4proc.c
>> +++ b/fs/nfsd/nfs4proc.c
>> @@ -1013,6 +1013,45 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>> return status;
>> }
>>
>> +#ifdef CONFIG_NFSD_V4_2_SEEK
>> +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 | WR_STATE, &file);
>
> I think that should be just RD_STATE.

I was trying to cover the case where an application does:

open(WRITE);
seek(HOLE);
write("blah");

I can change the code if that's not the way people use SEEK_HOLE / SEEK_DATA ...

Anna

>
> --b.
>
>
>> + 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;
>> +}
>> +#endif /* CONFIG_NFSD_V4_2_SEEK */
>> +
>> /* 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 +1920,14 @@ 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 */
>> +#ifdef CONFIG_NFSD_V4_2_SEEK
>> + [OP_SEEK] = {
>> + .op_func = (nfsd4op_func)nfsd4_seek,
>> + .op_name = "OP_SEEK",
>> + },
>> +#endif /* CONFIG_NFSD_V4_2_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..5bedeb3 100644
>> --- a/fs/nfsd/nfs4xdr.c
>> +++ b/fs/nfsd/nfs4xdr.c
>> @@ -1520,6 +1520,24 @@ static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, str
>> DECODE_TAIL;
>> }
>>
>> +#ifdef CONFIG_NFSD_V4_2_SEEK
>> +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;
>> +}
>> +#endif /* CONFIG_NFSD_V4_2_SEEK */
>> +
>> static __be32
>> nfsd4_decode_noop(struct nfsd4_compoundargs *argp, void *p)
>> {
>> @@ -1605,7 +1623,11 @@ 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,
>> +#ifdef CONFIG_NFSD_V4_2_SEEK
>> + [OP_SEEK] = (nfsd4_dec)nfsd4_decode_seek,
>> +#else
>> [OP_SEEK] = (nfsd4_dec)nfsd4_decode_notsupp,
>> +#endif /* CONFIG_NFSD_V4_2_SEEK */
>> [OP_WRITE_SAME] = (nfsd4_dec)nfsd4_decode_notsupp,
>> };
>>
>> @@ -3764,6 +3786,24 @@ nfsd4_encode_test_stateid(struct nfsd4_compoundres *resp, __be32 nfserr,
>> return nfserr;
>> }
>>
>> +#ifdef CONFIG_NFSD_V4_2_SEEK
>> +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;
>> +}
>> +#endif /* CONFIG_NFSD_V4_2_SEEK */
>> +
>> static __be32
>> nfsd4_encode_noop(struct nfsd4_compoundres *resp, __be32 nfserr, void *p)
>> {
>> @@ -3848,8 +3888,13 @@ 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,
>> +#ifdef CONFIG_NFSD_V4_2_SEEK
>> + [OP_SEEK] = (nfsd4_enc)nfsd4_encode_seek,
>> +#else
>> [OP_SEEK] = (nfsd4_enc)nfsd4_encode_noop,
>> +#endif /* CONFIG_NFSD_V4_2_SEEK */
>> [OP_WRITE_SAME] = (nfsd4_enc)nfsd4_encode_noop,
>> + [OP_IO_ADVISE] = (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
>>


2014-08-27 19:39:19

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 0/3] NFSD: Add v4.2 SEEK support

On Wed, Aug 27, 2014 at 11:17:55AM -0400, [email protected] wrote:
> From: Anna Schumaker <[email protected]>
>
> These patches add server support for the NFS v4.2 operation SEEK. The first
> two patches are error cleanups and infrastructure setup to prepare for the
> addition of new operations.
>
> My biggest concern is that these patches freeze the v4.2 opcodes to their
> current values, which makes any future spec changes to add or remove
> operations more difficult to accomidate.

I don't think it's so bad--worst case we end up with a hole in the list
of operation numbers, but I don't think that's a big deal.

So I'd drop the warning in the config text.

Anyway, the code's simple enough. Assuming the client patches are
ready, I'll be inclined to merge (revised versions of) these for 3.18.

--b.

>
> Questions? Comments? Thoughts?
>
> Anna
>
>
> Anna Schumaker (3):
> NFSD: Update error codes
> NFSD: Create nfs v4.2 decode ops
> NFSD: Implement SEEK
>
> fs/nfsd/Kconfig | 12 +++++++++
> fs/nfsd/nfs4proc.c | 47 +++++++++++++++++++++++++++++++++
> fs/nfsd/nfs4xdr.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/nfsd.h | 2 +-
> fs/nfsd/xdr4.h | 14 ++++++++++
> include/linux/nfs4.h | 26 ++++++++++++++++---
> 6 files changed, 170 insertions(+), 4 deletions(-)
>
> --
> 2.1.0
>

2014-08-27 17:59:23

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 1/3] NFSD: Update error codes

On Wed, Aug 27, 2014 at 11:17:56AM -0400, [email protected] wrote:
> From: Anna Schumaker <[email protected]>
>
> Recent NFS v4.2 drafts have removed NFS4ERR_METADATA_NOTSUPP and
> reassigned the error code to NFS4ERR_UNION_NOTSUPP.

And these have no users, so fine. Applying.

--b.

>
> I also add in the NFS4ERR_OFFLOAD_NO_REQS error code.
>
> Signed-off-by: Anna Schumaker <[email protected]>
> ---
> fs/nfsd/nfsd.h | 2 +-
> include/linux/nfs4.h | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 847daf3..747f3b95 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -251,7 +251,7 @@ void nfsd_lockd_shutdown(void);
> #define nfserr_deleg_revoked cpu_to_be32(NFS4ERR_DELEG_REVOKED)
> #define nfserr_partner_notsupp cpu_to_be32(NFS4ERR_PARTNER_NOTSUPP)
> #define nfserr_partner_no_auth cpu_to_be32(NFS4ERR_PARTNER_NO_AUTH)
> -#define nfserr_metadata_notsupp cpu_to_be32(NFS4ERR_METADATA_NOTSUPP)
> +#define nfserr_union_notsupp cpu_to_be32(NFS4ERR_UNION_NOTSUPP)
> #define nfserr_offload_denied cpu_to_be32(NFS4ERR_OFFLOAD_DENIED)
> #define nfserr_wrong_lfs cpu_to_be32(NFS4ERR_WRONG_LFS)
> #define nfserr_badlabel cpu_to_be32(NFS4ERR_BADLABEL)
> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> index a1e3064..79b2a0f 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -235,10 +235,11 @@ enum nfsstat4 {
> /* nfs42 */
> NFS4ERR_PARTNER_NOTSUPP = 10088,
> NFS4ERR_PARTNER_NO_AUTH = 10089,
> - NFS4ERR_METADATA_NOTSUPP = 10090,
> + NFS4ERR_UNION_NOTSUPP = 10090,
> NFS4ERR_OFFLOAD_DENIED = 10091,
> NFS4ERR_WRONG_LFS = 10092,
> NFS4ERR_BADLABEL = 10093,
> + NFS4ERR_OFFLOAD_NO_REQS = 10094,
> };
>
> static inline bool seqid_mutating_err(u32 err)
> --
> 2.1.0
>

2014-08-27 19:35:17

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 3/3] NFSD: Implement SEEK

On Wed, Aug 27, 2014 at 03:21:22PM -0400, Anna Schumaker wrote:
> On 08/27/2014 03:09 PM, J. Bruce Fields wrote:
> > On Wed, Aug 27, 2014 at 11:17:58AM -0400, [email protected] wrote:
> >> From: Anna Schumaker <[email protected]>
> >>
> >> 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. This
> >> operation is off by default, and needs CONFIG_NFSD_V4_2_SEEK=y to be
> >> compiled.
> >>
> >> Signed-off-by: Anna Schumaker <[email protected]>
> >> ---
> >> fs/nfsd/Kconfig | 12 ++++++++++++
> >> fs/nfsd/nfs4proc.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> >> fs/nfsd/nfs4xdr.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> >> fs/nfsd/xdr4.h | 14 ++++++++++++++
> >> include/linux/nfs4.h | 5 +++++
> >> 5 files changed, 123 insertions(+)
> >>
> >> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> >> index f994e75..804b20a 100644
> >> --- a/fs/nfsd/Kconfig
> >> +++ b/fs/nfsd/Kconfig
> >> @@ -81,6 +81,18 @@ config NFSD_V4
> >>
> >> If unsure, say N.
> >>
> >> +config NFSD_V4_2_SEEK
> >> + bool "Enable SEEK support for the NFS v4.2 server"
> >> + depends on NFSD_V4
> >> + help
> >> + Say Y here if you want to enable support for the NFS v4.2 operation
> >> + SEEK, which adds in SEEK_HOLE and SEEK_DATA support.
> >> +
> >> + WARNING: there is still a chance of backwards-incompatible protocol
> >> + changes. This feature is targeted at developers and testers only.
> >
> > I think now that we should instead confirm with the working group that
> > backwards-incompatible changes are done, and skip this warning.
> >
> > I'll remove it in the labeled NFS case, which we've decided is "done"
> > even if the rest of the 4.2 draft isn't.
> >
> > I also wonder if we should ditch these configuration options, or keep
> > them only temporarily. It's not much code, so I think the only reason
> > to allow configuring it out is to temporarily protect people from
> > immature code.
>
> I was expecting them to be temporary, and to get removed when the draft becomes an RFC.

OK.

In the security label case I guess there's one other issue, which is
that removing CONFIG_NFSD_V4_SECURITY_LABEL would mean adding a
previously unnecessary dependency on CONFIG_SECURITY.

--b.

>
> >
> >> +
> >> + If unsure, say N.
> >> +
> >> config NFSD_V4_SECURITY_LABEL
> >> bool "Provide Security Label support for NFSv4 server"
> >> depends on NFSD_V4 && SECURITY
> >> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> >> index 5e0dc52..f555eb2 100644
> >> --- a/fs/nfsd/nfs4proc.c
> >> +++ b/fs/nfsd/nfs4proc.c
> >> @@ -1013,6 +1013,45 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> >> return status;
> >> }
> >>
> >> +#ifdef CONFIG_NFSD_V4_2_SEEK
> >> +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 | WR_STATE, &file);
> >
> > I think that should be just RD_STATE.
>
> I was trying to cover the case where an application does:
>
> open(WRITE);
> seek(HOLE);
> write("blah");
>
> I can change the code if that's not the way people use SEEK_HOLE / SEEK_DATA ...
>
> Anna
>
> >
> > --b.
> >
> >
> >> + 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;
> >> +}
> >> +#endif /* CONFIG_NFSD_V4_2_SEEK */
> >> +
> >> /* 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 +1920,14 @@ 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 */
> >> +#ifdef CONFIG_NFSD_V4_2_SEEK
> >> + [OP_SEEK] = {
> >> + .op_func = (nfsd4op_func)nfsd4_seek,
> >> + .op_name = "OP_SEEK",
> >> + },
> >> +#endif /* CONFIG_NFSD_V4_2_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..5bedeb3 100644
> >> --- a/fs/nfsd/nfs4xdr.c
> >> +++ b/fs/nfsd/nfs4xdr.c
> >> @@ -1520,6 +1520,24 @@ static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, str
> >> DECODE_TAIL;
> >> }
> >>
> >> +#ifdef CONFIG_NFSD_V4_2_SEEK
> >> +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;
> >> +}
> >> +#endif /* CONFIG_NFSD_V4_2_SEEK */
> >> +
> >> static __be32
> >> nfsd4_decode_noop(struct nfsd4_compoundargs *argp, void *p)
> >> {
> >> @@ -1605,7 +1623,11 @@ 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,
> >> +#ifdef CONFIG_NFSD_V4_2_SEEK
> >> + [OP_SEEK] = (nfsd4_dec)nfsd4_decode_seek,
> >> +#else
> >> [OP_SEEK] = (nfsd4_dec)nfsd4_decode_notsupp,
> >> +#endif /* CONFIG_NFSD_V4_2_SEEK */
> >> [OP_WRITE_SAME] = (nfsd4_dec)nfsd4_decode_notsupp,
> >> };
> >>
> >> @@ -3764,6 +3786,24 @@ nfsd4_encode_test_stateid(struct nfsd4_compoundres *resp, __be32 nfserr,
> >> return nfserr;
> >> }
> >>
> >> +#ifdef CONFIG_NFSD_V4_2_SEEK
> >> +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;
> >> +}
> >> +#endif /* CONFIG_NFSD_V4_2_SEEK */
> >> +
> >> static __be32
> >> nfsd4_encode_noop(struct nfsd4_compoundres *resp, __be32 nfserr, void *p)
> >> {
> >> @@ -3848,8 +3888,13 @@ 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,
> >> +#ifdef CONFIG_NFSD_V4_2_SEEK
> >> + [OP_SEEK] = (nfsd4_enc)nfsd4_encode_seek,
> >> +#else
> >> [OP_SEEK] = (nfsd4_enc)nfsd4_encode_noop,
> >> +#endif /* CONFIG_NFSD_V4_2_SEEK */
> >> [OP_WRITE_SAME] = (nfsd4_enc)nfsd4_encode_noop,
> >> + [OP_IO_ADVISE] = (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
> >>
>

2014-08-27 15:18:07

by Anna Schumaker

[permalink] [raw]
Subject: [PATCH 3/3] NFSD: Implement SEEK

From: Anna Schumaker <[email protected]>

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. This
operation is off by default, and needs CONFIG_NFSD_V4_2_SEEK=y to be
compiled.

Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfsd/Kconfig | 12 ++++++++++++
fs/nfsd/nfs4proc.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
fs/nfsd/nfs4xdr.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
fs/nfsd/xdr4.h | 14 ++++++++++++++
include/linux/nfs4.h | 5 +++++
5 files changed, 123 insertions(+)

diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
index f994e75..804b20a 100644
--- a/fs/nfsd/Kconfig
+++ b/fs/nfsd/Kconfig
@@ -81,6 +81,18 @@ config NFSD_V4

If unsure, say N.

+config NFSD_V4_2_SEEK
+ bool "Enable SEEK support for the NFS v4.2 server"
+ depends on NFSD_V4
+ help
+ Say Y here if you want to enable support for the NFS v4.2 operation
+ SEEK, which adds in SEEK_HOLE and SEEK_DATA support.
+
+ WARNING: there is still a chance of backwards-incompatible protocol
+ changes. This feature is targeted at developers and testers only.
+
+ If unsure, say N.
+
config NFSD_V4_SECURITY_LABEL
bool "Provide Security Label support for NFSv4 server"
depends on NFSD_V4 && SECURITY
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 5e0dc52..f555eb2 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1013,6 +1013,45 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
return status;
}

+#ifdef CONFIG_NFSD_V4_2_SEEK
+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 | WR_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;
+}
+#endif /* CONFIG_NFSD_V4_2_SEEK */
+
/* 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 +1920,14 @@ 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 */
+#ifdef CONFIG_NFSD_V4_2_SEEK
+ [OP_SEEK] = {
+ .op_func = (nfsd4op_func)nfsd4_seek,
+ .op_name = "OP_SEEK",
+ },
+#endif /* CONFIG_NFSD_V4_2_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..5bedeb3 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1520,6 +1520,24 @@ static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, str
DECODE_TAIL;
}

+#ifdef CONFIG_NFSD_V4_2_SEEK
+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;
+}
+#endif /* CONFIG_NFSD_V4_2_SEEK */
+
static __be32
nfsd4_decode_noop(struct nfsd4_compoundargs *argp, void *p)
{
@@ -1605,7 +1623,11 @@ 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,
+#ifdef CONFIG_NFSD_V4_2_SEEK
+ [OP_SEEK] = (nfsd4_dec)nfsd4_decode_seek,
+#else
[OP_SEEK] = (nfsd4_dec)nfsd4_decode_notsupp,
+#endif /* CONFIG_NFSD_V4_2_SEEK */
[OP_WRITE_SAME] = (nfsd4_dec)nfsd4_decode_notsupp,
};

@@ -3764,6 +3786,24 @@ nfsd4_encode_test_stateid(struct nfsd4_compoundres *resp, __be32 nfserr,
return nfserr;
}

+#ifdef CONFIG_NFSD_V4_2_SEEK
+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;
+}
+#endif /* CONFIG_NFSD_V4_2_SEEK */
+
static __be32
nfsd4_encode_noop(struct nfsd4_compoundres *resp, __be32 nfserr, void *p)
{
@@ -3848,8 +3888,13 @@ 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,
+#ifdef CONFIG_NFSD_V4_2_SEEK
+ [OP_SEEK] = (nfsd4_enc)nfsd4_encode_seek,
+#else
[OP_SEEK] = (nfsd4_enc)nfsd4_encode_noop,
+#endif /* CONFIG_NFSD_V4_2_SEEK */
[OP_WRITE_SAME] = (nfsd4_enc)nfsd4_encode_noop,
+ [OP_IO_ADVISE] = (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


2014-08-27 19:09:44

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 3/3] NFSD: Implement SEEK

On Wed, Aug 27, 2014 at 11:17:58AM -0400, [email protected] wrote:
> From: Anna Schumaker <[email protected]>
>
> 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. This
> operation is off by default, and needs CONFIG_NFSD_V4_2_SEEK=y to be
> compiled.
>
> Signed-off-by: Anna Schumaker <[email protected]>
> ---
> fs/nfsd/Kconfig | 12 ++++++++++++
> fs/nfsd/nfs4proc.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/nfs4xdr.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/xdr4.h | 14 ++++++++++++++
> include/linux/nfs4.h | 5 +++++
> 5 files changed, 123 insertions(+)
>
> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> index f994e75..804b20a 100644
> --- a/fs/nfsd/Kconfig
> +++ b/fs/nfsd/Kconfig
> @@ -81,6 +81,18 @@ config NFSD_V4
>
> If unsure, say N.
>
> +config NFSD_V4_2_SEEK
> + bool "Enable SEEK support for the NFS v4.2 server"
> + depends on NFSD_V4
> + help
> + Say Y here if you want to enable support for the NFS v4.2 operation
> + SEEK, which adds in SEEK_HOLE and SEEK_DATA support.
> +
> + WARNING: there is still a chance of backwards-incompatible protocol
> + changes. This feature is targeted at developers and testers only.

I think now that we should instead confirm with the working group that
backwards-incompatible changes are done, and skip this warning.

I'll remove it in the labeled NFS case, which we've decided is "done"
even if the rest of the 4.2 draft isn't.

I also wonder if we should ditch these configuration options, or keep
them only temporarily. It's not much code, so I think the only reason
to allow configuring it out is to temporarily protect people from
immature code.

> +
> + If unsure, say N.
> +
> config NFSD_V4_SECURITY_LABEL
> bool "Provide Security Label support for NFSv4 server"
> depends on NFSD_V4 && SECURITY
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 5e0dc52..f555eb2 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1013,6 +1013,45 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> return status;
> }
>
> +#ifdef CONFIG_NFSD_V4_2_SEEK
> +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 | WR_STATE, &file);

I think that should be just RD_STATE.

--b.


> + 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;
> +}
> +#endif /* CONFIG_NFSD_V4_2_SEEK */
> +
> /* 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 +1920,14 @@ 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 */
> +#ifdef CONFIG_NFSD_V4_2_SEEK
> + [OP_SEEK] = {
> + .op_func = (nfsd4op_func)nfsd4_seek,
> + .op_name = "OP_SEEK",
> + },
> +#endif /* CONFIG_NFSD_V4_2_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..5bedeb3 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1520,6 +1520,24 @@ static __be32 nfsd4_decode_reclaim_complete(struct nfsd4_compoundargs *argp, str
> DECODE_TAIL;
> }
>
> +#ifdef CONFIG_NFSD_V4_2_SEEK
> +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;
> +}
> +#endif /* CONFIG_NFSD_V4_2_SEEK */
> +
> static __be32
> nfsd4_decode_noop(struct nfsd4_compoundargs *argp, void *p)
> {
> @@ -1605,7 +1623,11 @@ 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,
> +#ifdef CONFIG_NFSD_V4_2_SEEK
> + [OP_SEEK] = (nfsd4_dec)nfsd4_decode_seek,
> +#else
> [OP_SEEK] = (nfsd4_dec)nfsd4_decode_notsupp,
> +#endif /* CONFIG_NFSD_V4_2_SEEK */
> [OP_WRITE_SAME] = (nfsd4_dec)nfsd4_decode_notsupp,
> };
>
> @@ -3764,6 +3786,24 @@ nfsd4_encode_test_stateid(struct nfsd4_compoundres *resp, __be32 nfserr,
> return nfserr;
> }
>
> +#ifdef CONFIG_NFSD_V4_2_SEEK
> +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;
> +}
> +#endif /* CONFIG_NFSD_V4_2_SEEK */
> +
> static __be32
> nfsd4_encode_noop(struct nfsd4_compoundres *resp, __be32 nfserr, void *p)
> {
> @@ -3848,8 +3888,13 @@ 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,
> +#ifdef CONFIG_NFSD_V4_2_SEEK
> + [OP_SEEK] = (nfsd4_enc)nfsd4_encode_seek,
> +#else
> [OP_SEEK] = (nfsd4_enc)nfsd4_encode_noop,
> +#endif /* CONFIG_NFSD_V4_2_SEEK */
> [OP_WRITE_SAME] = (nfsd4_enc)nfsd4_encode_noop,
> + [OP_IO_ADVISE] = (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
>