[PATCH 0/7] Bakeathon server fixes

[PATCH 0/7] Bakeathon server fixes

[Benny Halevy]

Here's the list of patches I tested here in the Bakeathon.
They pass the Connectathon tests over nfsv4.1 (no pnfs)
against the linux client, but VMware reports a bug with
OPEN with share_access==0x401 (1025)
NFS4_SHARE_WANT_NO_DELEG | NFS4_SHARE_ACCESS_READ
return NFS4ERR_GRACE for some reason.
I haven't been able to track that down yet.

[PATCH 1/7] nfsd4: implement new 4.1 open reclaim types
[PATCH 2/7] nfsd41: use SEQ4_STATUS_BACKCHANNEL_FAULT when
[PATCH 3/7] nfsd4: seq->status_flags may be used unitialized
[PATCH 4/7] nfsd4: allow NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL |
[PATCH 5/7] nfsd4: implement NFS4_SHARE_WANT_NO_DELEG,
[PATCH 6/7] nfsd4: typo logical vs bitwise negate for want_mask
[PATCH 7/7] nfsd4: share_access_to_flags should consider only nfs4.x

[PATCH 1/7] nfsd4: implement new 4.1 open reclaim types

[Benny Halevy]

From: "J. Bruce Fields" <bfields@redhat.com>

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4proc.c   |   15 +++------------
 fs/nfsd/nfs4state.c  |   10 ++++++++--
 fs/nfsd/nfs4xdr.c    |   13 +++++++++++++
 include/linux/nfs4.h |    5 ++++-
 4 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 5b192a2..e3fffab 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -366,12 +366,6 @@ static __be32 nfsd_check_obj_isreg(struct svc_fh *fh)
 	switch (open->op_claim_type) {
 		case NFS4_OPEN_CLAIM_DELEGATE_CUR:
 		case NFS4_OPEN_CLAIM_NULL:
-			/*
-			 * (1) set CURRENT_FH to the file being opened,
-			 * creating it if necessary, (2) set open->op_cinfo,
-			 * (3) set open->op_truncate if the file is to be
-			 * truncated after opening, (4) do permission checking.
-			 */
 			status = do_open_lookup(rqstp, &cstate->current_fh,
 						open);
 			if (status)
@@ -379,17 +373,14 @@ static __be32 nfsd_check_obj_isreg(struct svc_fh *fh)
 			break;
 		case NFS4_OPEN_CLAIM_PREVIOUS:
 			open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
-			/*
-			 * The CURRENT_FH is already set to the file being
-			 * opened.  (1) set open->op_cinfo, (2) set
-			 * open->op_truncate if the file is to be truncated
-			 * after opening, (3) do permission checking.
-			*/
+		case NFS4_OPEN_CLAIM_FH:
+		case NFS4_OPEN_CLAIM_DELEG_CUR_FH:
 			status = do_open_fhandle(rqstp, &cstate->current_fh,
 						 open);
 			if (status)
 				goto out;
 			break;
+		case NFS4_OPEN_CLAIM_DELEG_PREV_FH:
              	case NFS4_OPEN_CLAIM_DELEGATE_PREV:
 			open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
 			dprintk("NFSD: unsupported OPEN claim type %d\n",
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 62aa91a..7da0748 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2562,6 +2562,12 @@ static int share_access_to_flags(u32 share_access)
 	return delegstateid(ret);
 }
 
+static bool nfsd4_is_deleg_cur(struct nfsd4_open *open)
+{
+	return open->op_claim_type == NFS4_OPEN_CLAIM_DELEGATE_CUR ||
+	       open->op_claim_type == NFS4_OPEN_CLAIM_DELEG_CUR_FH;
+}
+
 static __be32
 nfs4_check_deleg(struct nfs4_client *cl, struct nfs4_file *fp, struct nfsd4_open *open,
 		struct nfs4_delegation **dp)
@@ -2577,7 +2583,7 @@ static int share_access_to_flags(u32 share_access)
 	if (status)
 		*dp = NULL;
 out:
-	if (open->op_claim_type != NFS4_OPEN_CLAIM_DELEGATE_CUR)
+	if (!nfsd4_is_deleg_cur(open))
 		return nfs_ok;
 	if (status)
 		return status;
@@ -2882,7 +2888,7 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
 			goto out;
 	} else {
 		status = nfserr_bad_stateid;
-		if (open->op_claim_type == NFS4_OPEN_CLAIM_DELEGATE_CUR)
+		if (nfsd4_is_deleg_cur(open))
 			goto out;
 		status = nfserr_jukebox;
 		fp = alloc_init_file(ino);
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 645a0a9..fdc09a5 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -803,6 +803,19 @@ static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_ne
 		if ((status = check_filename(open->op_fname.data, open->op_fname.len, nfserr_inval)))
 			return status;
 		break;
+	case NFS4_OPEN_CLAIM_FH:
+	case NFS4_OPEN_CLAIM_DELEG_PREV_FH:
+		if (argp->minorversion < 1)
+			goto xdr_error;
+		/* void */
+		break;
+	case NFS4_OPEN_CLAIM_DELEG_CUR_FH:
+		if (argp->minorversion < 1)
+			goto xdr_error;
+		status = nfsd4_decode_stateid(argp, &open->op_delegate_stateid);
+		if (status)
+			return status;
+		break;
 	default:
 		goto xdr_error;
 	}
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index b875b03..32345c2 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -410,7 +410,10 @@ enum open_claim_type4 {
 	NFS4_OPEN_CLAIM_NULL = 0,
 	NFS4_OPEN_CLAIM_PREVIOUS = 1,
 	NFS4_OPEN_CLAIM_DELEGATE_CUR = 2,
-	NFS4_OPEN_CLAIM_DELEGATE_PREV = 3
+	NFS4_OPEN_CLAIM_DELEGATE_PREV = 3,
+	NFS4_OPEN_CLAIM_FH = 4, /* 4.1 */
+	NFS4_OPEN_CLAIM_DELEG_CUR_FH = 5, /* 4.1 */
+	NFS4_OPEN_CLAIM_DELEG_PREV_FH = 6, /* 4.1 */
 };
 
 enum opentype4 {
-- 
1.7.6

Re: [PATCH 1/7] nfsd4: implement new 4.1 open reclaim types

[J. Bruce Fields]

On Wed, Oct 19, 2011 at 07:12:48PM -0700, Benny Halevy wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> Signed-off-by: Benny Halevy <bhalevy@tonian.com>
> ---
>  fs/nfsd/nfs4proc.c   |   15 +++------------
>  fs/nfsd/nfs4state.c  |   10 ++++++++--
>  fs/nfsd/nfs4xdr.c    |   13 +++++++++++++
>  include/linux/nfs4.h |    5 ++++-
>  4 files changed, 28 insertions(+), 15 deletions(-)

Adding Mi Jinlong--is there a chance you might be able to write pynfs
tests for these?  I don't know if clients are using them yet so it would
be nice to know if they work before committing this.

--b.

> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 5b192a2..e3fffab 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -366,12 +366,6 @@ static __be32 nfsd_check_obj_isreg(struct svc_fh *fh)
>  	switch (open->op_claim_type) {
>  		case NFS4_OPEN_CLAIM_DELEGATE_CUR:
>  		case NFS4_OPEN_CLAIM_NULL:
> -			/*
> -			 * (1) set CURRENT_FH to the file being opened,
> -			 * creating it if necessary, (2) set open->op_cinfo,
> -			 * (3) set open->op_truncate if the file is to be
> -			 * truncated after opening, (4) do permission checking.
> -			 */
>  			status = do_open_lookup(rqstp, &cstate->current_fh,
>  						open);
>  			if (status)
> @@ -379,17 +373,14 @@ static __be32 nfsd_check_obj_isreg(struct svc_fh *fh)
>  			break;
>  		case NFS4_OPEN_CLAIM_PREVIOUS:
>  			open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
> -			/*
> -			 * The CURRENT_FH is already set to the file being
> -			 * opened.  (1) set open->op_cinfo, (2) set
> -			 * open->op_truncate if the file is to be truncated
> -			 * after opening, (3) do permission checking.
> -			*/
> +		case NFS4_OPEN_CLAIM_FH:
> +		case NFS4_OPEN_CLAIM_DELEG_CUR_FH:
>  			status = do_open_fhandle(rqstp, &cstate->current_fh,
>  						 open);
>  			if (status)
>  				goto out;
>  			break;
> +		case NFS4_OPEN_CLAIM_DELEG_PREV_FH:
>               	case NFS4_OPEN_CLAIM_DELEGATE_PREV:
>  			open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
>  			dprintk("NFSD: unsupported OPEN claim type %d\n",
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 62aa91a..7da0748 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2562,6 +2562,12 @@ static int share_access_to_flags(u32 share_access)
>  	return delegstateid(ret);
>  }
>  
> +static bool nfsd4_is_deleg_cur(struct nfsd4_open *open)
> +{
> +	return open->op_claim_type == NFS4_OPEN_CLAIM_DELEGATE_CUR ||
> +	       open->op_claim_type == NFS4_OPEN_CLAIM_DELEG_CUR_FH;
> +}
> +
>  static __be32
>  nfs4_check_deleg(struct nfs4_client *cl, struct nfs4_file *fp, struct nfsd4_open *open,
>  		struct nfs4_delegation **dp)
> @@ -2577,7 +2583,7 @@ static int share_access_to_flags(u32 share_access)
>  	if (status)
>  		*dp = NULL;
>  out:
> -	if (open->op_claim_type != NFS4_OPEN_CLAIM_DELEGATE_CUR)
> +	if (!nfsd4_is_deleg_cur(open))
>  		return nfs_ok;
>  	if (status)
>  		return status;
> @@ -2882,7 +2888,7 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
>  			goto out;
>  	} else {
>  		status = nfserr_bad_stateid;
> -		if (open->op_claim_type == NFS4_OPEN_CLAIM_DELEGATE_CUR)
> +		if (nfsd4_is_deleg_cur(open))
>  			goto out;
>  		status = nfserr_jukebox;
>  		fp = alloc_init_file(ino);
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 645a0a9..fdc09a5 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -803,6 +803,19 @@ static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_ne
>  		if ((status = check_filename(open->op_fname.data, open->op_fname.len, nfserr_inval)))
>  			return status;
>  		break;
> +	case NFS4_OPEN_CLAIM_FH:
> +	case NFS4_OPEN_CLAIM_DELEG_PREV_FH:
> +		if (argp->minorversion < 1)
> +			goto xdr_error;
> +		/* void */
> +		break;
> +	case NFS4_OPEN_CLAIM_DELEG_CUR_FH:
> +		if (argp->minorversion < 1)
> +			goto xdr_error;
> +		status = nfsd4_decode_stateid(argp, &open->op_delegate_stateid);
> +		if (status)
> +			return status;
> +		break;
>  	default:
>  		goto xdr_error;
>  	}
> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> index b875b03..32345c2 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -410,7 +410,10 @@ enum open_claim_type4 {
>  	NFS4_OPEN_CLAIM_NULL = 0,
>  	NFS4_OPEN_CLAIM_PREVIOUS = 1,
>  	NFS4_OPEN_CLAIM_DELEGATE_CUR = 2,
> -	NFS4_OPEN_CLAIM_DELEGATE_PREV = 3
> +	NFS4_OPEN_CLAIM_DELEGATE_PREV = 3,
> +	NFS4_OPEN_CLAIM_FH = 4, /* 4.1 */
> +	NFS4_OPEN_CLAIM_DELEG_CUR_FH = 5, /* 4.1 */
> +	NFS4_OPEN_CLAIM_DELEG_PREV_FH = 6, /* 4.1 */
>  };
>  
>  enum opentype4 {
> -- 
> 1.7.6
> 

[PATCH 2/7] nfsd41: use SEQ4_STATUS_BACKCHANNEL_FAULT when cb_sequence is invalid

[Benny Halevy]

From: Benny Halevy <bhalevy@panasas.com>

Signed-off-by: Benny Halevy <bhalevy@panasas.com>
---
 fs/nfsd/nfs4callback.c |   10 ++++++++++
 fs/nfsd/nfs4state.c    |    8 +++++++-
 fs/nfsd/state.h        |    1 +
 3 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index de018ec..7748d6a 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -39,6 +39,8 @@
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
 
+static void nfsd4_mark_cb_fault(struct nfs4_client *, int reason);
+
 #define NFSPROC4_CB_NULL 0
 #define NFSPROC4_CB_COMPOUND 1
 
@@ -460,6 +462,8 @@ static int decode_cb_sequence4resok(struct xdr_stream *xdr,
 	 */
 	status = 0;
 out:
+	if (status)
+		nfsd4_mark_cb_fault(cb->cb_clp, status);
 	return status;
 out_overflow:
 	print_overflow_msg(__func__, xdr);
@@ -686,6 +690,12 @@ static void nfsd4_mark_cb_down(struct nfs4_client *clp, int reason)
 	warn_no_callback_path(clp, reason);
 }
 
+static void nfsd4_mark_cb_fault(struct nfs4_client *clp, int reason)
+{
+	clp->cl_cb_state = NFSD4_CB_FAULT;
+	warn_no_callback_path(clp, reason);
+}
+
 static void nfsd4_cb_probe_done(struct rpc_task *task, void *calldata)
 {
 	struct nfs4_client *clp = container_of(calldata, struct nfs4_client, cl_cb_null);
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7da0748..ccdb9b2 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1929,8 +1929,14 @@ static bool nfsd4_request_too_big(struct svc_rqst *rqstp,
 
 		nfsd4_get_session(cstate->session);
 		atomic_inc(&clp->cl_refcount);
-		if (clp->cl_cb_state == NFSD4_CB_DOWN)
+		switch (clp->cl_cb_state) {
+		case NFSD4_CB_DOWN:
 			seq->status_flags |= SEQ4_STATUS_CB_PATH_DOWN;
+			break;
+		case NFSD4_CB_FAULT:
+			seq->status_flags |= SEQ4_STATUS_BACKCHANNEL_FAULT;
+			break;
+		}
 	}
 	kfree(conn);
 	spin_unlock(&client_lock);
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 87eecfd..c5b87a2 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -258,6 +258,7 @@ struct nfs4_client {
 #define NFSD4_CB_UP		0
 #define NFSD4_CB_UNKNOWN	1
 #define NFSD4_CB_DOWN		2
+#define NFSD4_CB_FAULT		3
 	int			cl_cb_state;
 	struct nfsd4_callback	cl_cb_null;
 	struct nfsd4_session	*cl_cb_session;
-- 
1.7.6

[PATCH 3/7] nfsd4: seq->status_flags may be used unitialized

[Benny Halevy]

From: Benny Halevy <bhalevy@tonian.com>

Reported-by: Gopala Suryanarayana <gsuryanarayana@vmware.com>
Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4state.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ccdb9b2..ec361bb 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1931,11 +1931,13 @@ static bool nfsd4_request_too_big(struct svc_rqst *rqstp,
 		atomic_inc(&clp->cl_refcount);
 		switch (clp->cl_cb_state) {
 		case NFSD4_CB_DOWN:
-			seq->status_flags |= SEQ4_STATUS_CB_PATH_DOWN;
+			seq->status_flags = SEQ4_STATUS_CB_PATH_DOWN;
 			break;
 		case NFSD4_CB_FAULT:
-			seq->status_flags |= SEQ4_STATUS_BACKCHANNEL_FAULT;
+			seq->status_flags = SEQ4_STATUS_BACKCHANNEL_FAULT;
 			break;
+		default:
+			seq->status_flags = 0;
 		}
 	}
 	kfree(conn);
-- 
1.7.6

[PATCH 4/7] nfsd4: allow NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL | NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED

[Benny Halevy]

From: Benny Halevy <bhalevy@tonian.com>

RFC5661 says:
   The client may set one or both of
   OPEN4_SHARE_ACCESS_WANT_SIGNAL_DELEG_WHEN_RESRC_AVAIL and
   OPEN4_SHARE_ACCESS_WANT_PUSH_DELEG_WHEN_UNCONTENDED.

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4xdr.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index fdc09a5..706ada1 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -677,6 +677,8 @@ static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *x)
 	switch (w) {
 	case NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL:
 	case NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED:
+	case (NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL |
+	      NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED):
 		return nfs_ok;
 	}
 xdr_error:
-- 
1.7.6

[PATCH 5/7] nfsd4: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT

[Benny Halevy]

From: Benny Halevy <bhalevy@tonian.com>

Respect client request for not getting a delegation in NFSv4.1
Appropriately return delegation "type" NFS4_OPEN_DELEGATE_NONE_EXT
and WND4_NOT_WANTED reason.

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4state.c  |   10 ++++++++--
 fs/nfsd/nfs4xdr.c    |    3 +++
 include/linux/nfs4.h |   15 ++++++++++++++-
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ec361bb..5c3377c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2931,15 +2931,21 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
 	update_stateid(&stp->st_stid.sc_stateid);
 	memcpy(&open->op_stateid, &stp->st_stid.sc_stateid, sizeof(stateid_t));
 
-	if (nfsd4_has_session(&resp->cstate))
+	if (nfsd4_has_session(&resp->cstate)) {
 		open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
 
+		if (open->op_share_access & NFS4_SHARE_WANT_NO_DELEG) {
+			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
+			goto nodeleg;
+		}
+	}
+
 	/*
 	* Attempt to hand out a delegation. No error return, because the
 	* OPEN succeeds even if we fail.
 	*/
 	nfs4_open_delegation(current_fh, open, stp);
-
+nodeleg:
 	status = nfs_ok;
 
 	dprintk("%s: stateid=" STATEID_FMT "\n", __func__,
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 706ada1..1a419c0 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2842,6 +2842,9 @@ static __be32 nfsd4_encode_bind_conn_to_session(struct nfsd4_compoundres *resp,
 		WRITE32(0);   /* XXX: is NULL principal ok? */
 		ADJUST_ARGS();
 		break;
+	case NFS4_OPEN_DELEGATE_NONE_EXT: /* 4.1 */
+		WRITE32(WND4_NOT_WANTED);	/* only reason for now */
+		break;
 	default:
 		BUG();
 	}
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 32345c2..8cdde4d 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -441,7 +441,20 @@ enum limit_by4 {
 enum open_delegation_type4 {
 	NFS4_OPEN_DELEGATE_NONE = 0,
 	NFS4_OPEN_DELEGATE_READ = 1,
-	NFS4_OPEN_DELEGATE_WRITE = 2
+	NFS4_OPEN_DELEGATE_WRITE = 2,
+	NFS4_OPEN_DELEGATE_NONE_EXT = 3, /* 4.1 */
+};
+
+enum why_no_delegation4 { /* new to v4.1 */
+	WND4_NOT_WANTED = 0,
+	WND4_CONTENTION = 1,
+	WND4_RESOURCE = 2,
+	WND4_NOT_SUPP_FTYPE = 3,
+	WND4_WRITE_DELEG_NOT_SUPP_FTYPE = 4,
+	WND4_NOT_SUPP_UPGRADE = 5,
+	WND4_NOT_SUPP_DOWNGRADE = 6,
+	WND4_CANCELLED = 7,
+	WND4_IS_DIR = 8,
 };
 
 enum lock_type4 {
-- 
1.7.6

Re: [PATCH 5/7] nfsd4: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT

[J. Bruce Fields]

On Wed, Oct 19, 2011 at 07:13:21PM -0700, Benny Halevy wrote:
> From: Benny Halevy <bhalevy@tonian.com>
> 
> Respect client request for not getting a delegation in NFSv4.1
> Appropriately return delegation "type" NFS4_OPEN_DELEGATE_NONE_EXT
> and WND4_NOT_WANTED reason.

As I read it, if we're going to implement part of this, then we have to
implement all of it.  ("If the server supports the new _WANT_ flags and
the client sends one or more of the new flags, then in the event the
server does not return a delegation, it MUST return a delegation type of
OPEN_DELEGATE_NONE_EXT.  The field ond_why in the reply indicates why no
delegation was returned and will be one of....")

As long as we don't implement any want flags, we escape that MUST.

So I'd rather put this aside till it's more complete.

--b.

> 
> Signed-off-by: Benny Halevy <bhalevy@tonian.com>
> ---
>  fs/nfsd/nfs4state.c  |   10 ++++++++--
>  fs/nfsd/nfs4xdr.c    |    3 +++
>  include/linux/nfs4.h |   15 ++++++++++++++-
>  3 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index ec361bb..5c3377c 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2931,15 +2931,21 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
>  	update_stateid(&stp->st_stid.sc_stateid);
>  	memcpy(&open->op_stateid, &stp->st_stid.sc_stateid, sizeof(stateid_t));
>  
> -	if (nfsd4_has_session(&resp->cstate))
> +	if (nfsd4_has_session(&resp->cstate)) {
>  		open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
>  
> +		if (open->op_share_access & NFS4_SHARE_WANT_NO_DELEG) {
> +			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
> +			goto nodeleg;
> +		}
> +	}
> +
>  	/*
>  	* Attempt to hand out a delegation. No error return, because the
>  	* OPEN succeeds even if we fail.
>  	*/
>  	nfs4_open_delegation(current_fh, open, stp);
> -
> +nodeleg:
>  	status = nfs_ok;
>  
>  	dprintk("%s: stateid=" STATEID_FMT "\n", __func__,
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 706ada1..1a419c0 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -2842,6 +2842,9 @@ static __be32 nfsd4_encode_bind_conn_to_session(struct nfsd4_compoundres *resp,
>  		WRITE32(0);   /* XXX: is NULL principal ok? */
>  		ADJUST_ARGS();
>  		break;
> +	case NFS4_OPEN_DELEGATE_NONE_EXT: /* 4.1 */
> +		WRITE32(WND4_NOT_WANTED);	/* only reason for now */
> +		break;
>  	default:
>  		BUG();
>  	}
> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> index 32345c2..8cdde4d 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -441,7 +441,20 @@ enum limit_by4 {
>  enum open_delegation_type4 {
>  	NFS4_OPEN_DELEGATE_NONE = 0,
>  	NFS4_OPEN_DELEGATE_READ = 1,
> -	NFS4_OPEN_DELEGATE_WRITE = 2
> +	NFS4_OPEN_DELEGATE_WRITE = 2,
> +	NFS4_OPEN_DELEGATE_NONE_EXT = 3, /* 4.1 */
> +};
> +
> +enum why_no_delegation4 { /* new to v4.1 */
> +	WND4_NOT_WANTED = 0,
> +	WND4_CONTENTION = 1,
> +	WND4_RESOURCE = 2,
> +	WND4_NOT_SUPP_FTYPE = 3,
> +	WND4_WRITE_DELEG_NOT_SUPP_FTYPE = 4,
> +	WND4_NOT_SUPP_UPGRADE = 5,
> +	WND4_NOT_SUPP_DOWNGRADE = 6,
> +	WND4_CANCELLED = 7,
> +	WND4_IS_DIR = 8,
>  };
>  
>  enum lock_type4 {
> -- 
> 1.7.6
> 

Re: [PATCH 5/7] nfsd4: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT

[Benny Halevy]

T0ssIEkgcmVhZCB5b3UuICBKdXN0IHBsZWFzZSB0YWtlIG5vdGUgdGhhdCBXQU5UX05PX0RFTEVH
IGZsYWcgaXMgaW1wb3J0YW50IHRvIHNpbXBsaWZpZWQgY2xpZW50cyB0aGF0IGRvIG5vdCBzdXBw
b3J0IGRlbGVnYXRpb25zLCBsaWtlIHRoZSBvbmUgSSB3YXMgdGVzdGluZyBhZ2FpbnN0IGluIHRo
ZSBCQVQsIGFuZCBpdCBpcyB0aGUgb25seSBvbmUgaW4gdXNlIHRoYXQgSSBjb3VsZCB0ZWxsLg0K
DQpCZW5ueQ0KLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCkZyb206ICJKLiBCcnVjZSBGaWVs
ZHMiIDxiZmllbGRzQGZpZWxkc2VzLm9yZz5EYXRlOiBUaHUsIDIwIE9jdCAyMDExIDA3OjUyOjI0
IA0KVG86IEJlbm55IEhhbGV2eTxiZW5ueUB0b25pYW4uY29tPg0KQ2M6IEouIEJydWNlIEZpZWxk
czxiZmllbGRzQHJlZGhhdC5jb20+OyA8bGludXgtbmZzQHZnZXIua2VybmVsLm9yZz47IEJlbm55
IEhhbGV2eTxiaGFsZXZ5QHRvbmlhbi5jb20+DQpTdWJqZWN0OiBSZTogW1BBVENIIDUvN10gbmZz
ZDQ6IGltcGxlbWVudCBORlM0X1NIQVJFX1dBTlRfTk9fREVMRUcsDQogTkZTNF9PUEVOX0RFTEVH
QVRFX05PTkVfRVhUDQoNCk9uIFdlZCwgT2N0IDE5LCAyMDExIGF0IDA3OjEzOjIxUE0gLTA3MDAs
IEJlbm55IEhhbGV2eSB3cm90ZToNCj4gRnJvbTogQmVubnkgSGFsZXZ5IDxiaGFsZXZ5QHRvbmlh
bi5jb20+DQo+IA0KPiBSZXNwZWN0IGNsaWVudCByZXF1ZXN0IGZvciBub3QgZ2V0dGluZyBhIGRl
bGVnYXRpb24gaW4gTkZTdjQuMQ0KPiBBcHByb3ByaWF0ZWx5IHJldHVybiBkZWxlZ2F0aW9uICJ0
eXBlIiBORlM0X09QRU5fREVMRUdBVEVfTk9ORV9FWFQNCj4gYW5kIFdORDRfTk9UX1dBTlRFRCBy
ZWFzb24uDQoNCkFzIEkgcmVhZCBpdCwgaWYgd2UncmUgZ29pbmcgdG8gaW1wbGVtZW50IHBhcnQg
b2YgdGhpcywgdGhlbiB3ZSBoYXZlIHRvDQppbXBsZW1lbnQgYWxsIG9mIGl0LiAgKCJJZiB0aGUg
c2VydmVyIHN1cHBvcnRzIHRoZSBuZXcgX1dBTlRfIGZsYWdzIGFuZA0KdGhlIGNsaWVudCBzZW5k
cyBvbmUgb3IgbW9yZSBvZiB0aGUgbmV3IGZsYWdzLCB0aGVuIGluIHRoZSBldmVudCB0aGUNCnNl
cnZlciBkb2VzIG5vdCByZXR1cm4gYSBkZWxlZ2F0aW9uLCBpdCBNVVNUIHJldHVybiBhIGRlbGVn
YXRpb24gdHlwZSBvZg0KT1BFTl9ERUxFR0FURV9OT05FX0VYVC4gIFRoZSBmaWVsZCBvbmRfd2h5
IGluIHRoZSByZXBseSBpbmRpY2F0ZXMgd2h5IG5vDQpkZWxlZ2F0aW9uIHdhcyByZXR1cm5lZCBh
bmQgd2lsbCBiZSBvbmUgb2YuLi4uIikNCg0KQXMgbG9uZyBhcyB3ZSBkb24ndCBpbXBsZW1lbnQg
YW55IHdhbnQgZmxhZ3MsIHdlIGVzY2FwZSB0aGF0IE1VU1QuDQoNClNvIEknZCByYXRoZXIgcHV0
IHRoaXMgYXNpZGUgdGlsbCBpdCdzIG1vcmUgY29tcGxldGUuDQoNCi0tYi4NCg0KPiANCj4gU2ln
bmVkLW9mZi1ieTogQmVubnkgSGFsZXZ5IDxiaGFsZXZ5QHRvbmlhbi5jb20+DQo+IC0tLQ0KPiAg
ZnMvbmZzZC9uZnM0c3RhdGUuYyAgfCAgIDEwICsrKysrKysrLS0NCj4gIGZzL25mc2QvbmZzNHhk
ci5jICAgIHwgICAgMyArKysNCj4gIGluY2x1ZGUvbGludXgvbmZzNC5oIHwgICAxNSArKysrKysr
KysrKysrKy0NCj4gIDMgZmlsZXMgY2hhbmdlZCwgMjUgaW5zZXJ0aW9ucygrKSwgMyBkZWxldGlv
bnMoLSkNCj4gDQo+IGRpZmYgLS1naXQgYS9mcy9uZnNkL25mczRzdGF0ZS5jIGIvZnMvbmZzZC9u
ZnM0c3RhdGUuYw0KPiBpbmRleCBlYzM2MWJiLi41YzMzNzdjIDEwMDY0NA0KPiAtLS0gYS9mcy9u
ZnNkL25mczRzdGF0ZS5jDQo+ICsrKyBiL2ZzL25mc2QvbmZzNHN0YXRlLmMNCj4gQEAgLTI5MzEs
MTUgKzI5MzEsMjEgQEAgc3RhdGljIGludCBuZnM0X3NldF9kZWxlZ2F0aW9uKHN0cnVjdCBuZnM0
X2RlbGVnYXRpb24gKmRwLCBpbnQgZmxhZykNCj4gIAl1cGRhdGVfc3RhdGVpZCgmc3RwLT5zdF9z
dGlkLnNjX3N0YXRlaWQpOw0KPiAgCW1lbWNweSgmb3Blbi0+b3Bfc3RhdGVpZCwgJnN0cC0+c3Rf
c3RpZC5zY19zdGF0ZWlkLCBzaXplb2Yoc3RhdGVpZF90KSk7DQo+ICANCj4gLQlpZiAobmZzZDRf
aGFzX3Nlc3Npb24oJnJlc3AtPmNzdGF0ZSkpDQo+ICsJaWYgKG5mc2Q0X2hhc19zZXNzaW9uKCZy
ZXNwLT5jc3RhdGUpKSB7DQo+ICAJCW9wZW4tPm9wX29wZW5vd25lci0+b29fZmxhZ3MgfD0gTkZT
NF9PT19DT05GSVJNRUQ7DQo+ICANCj4gKwkJaWYgKG9wZW4tPm9wX3NoYXJlX2FjY2VzcyAmIE5G
UzRfU0hBUkVfV0FOVF9OT19ERUxFRykgew0KPiArCQkJb3Blbi0+b3BfZGVsZWdhdGVfdHlwZSA9
IE5GUzRfT1BFTl9ERUxFR0FURV9OT05FX0VYVDsNCj4gKwkJCWdvdG8gbm9kZWxlZzsNCj4gKwkJ
fQ0KPiArCX0NCj4gKw0KPiAgCS8qDQo+ICAJKiBBdHRlbXB0IHRvIGhhbmQgb3V0IGEgZGVsZWdh
dGlvbi4gTm8gZXJyb3IgcmV0dXJuLCBiZWNhdXNlIHRoZQ0KPiAgCSogT1BFTiBzdWNjZWVkcyBl
dmVuIGlmIHdlIGZhaWwuDQo+ICAJKi8NCj4gIAluZnM0X29wZW5fZGVsZWdhdGlvbihjdXJyZW50
X2ZoLCBvcGVuLCBzdHApOw0KPiAtDQo+ICtub2RlbGVnOg0KPiAgCXN0YXR1cyA9IG5mc19vazsN
Cj4gIA0KPiAgCWRwcmludGsoIiVzOiBzdGF0ZWlkPSIgU1RBVEVJRF9GTVQgIlxuIiwgX19mdW5j
X18sDQo+IGRpZmYgLS1naXQgYS9mcy9uZnNkL25mczR4ZHIuYyBiL2ZzL25mc2QvbmZzNHhkci5j
DQo+IGluZGV4IDcwNmFkYTEuLjFhNDE5YzAgMTAwNjQ0DQo+IC0tLSBhL2ZzL25mc2QvbmZzNHhk
ci5jDQo+ICsrKyBiL2ZzL25mc2QvbmZzNHhkci5jDQo+IEBAIC0yODQyLDYgKzI4NDIsOSBAQCBz
dGF0aWMgX19iZTMyIG5mc2Q0X2VuY29kZV9iaW5kX2Nvbm5fdG9fc2Vzc2lvbihzdHJ1Y3QgbmZz
ZDRfY29tcG91bmRyZXMgKnJlc3AsDQo+ICAJCVdSSVRFMzIoMCk7ICAgLyogWFhYOiBpcyBOVUxM
IHByaW5jaXBhbCBvaz8gKi8NCj4gIAkJQURKVVNUX0FSR1MoKTsNCj4gIAkJYnJlYWs7DQo+ICsJ
Y2FzZSBORlM0X09QRU5fREVMRUdBVEVfTk9ORV9FWFQ6IC8qIDQuMSAqLw0KPiArCQlXUklURTMy
KFdORDRfTk9UX1dBTlRFRCk7CS8qIG9ubHkgcmVhc29uIGZvciBub3cgKi8NCj4gKwkJYnJlYWs7
DQo+ICAJZGVmYXVsdDoNCj4gIAkJQlVHKCk7DQo+ICAJfQ0KPiBkaWZmIC0tZ2l0IGEvaW5jbHVk
ZS9saW51eC9uZnM0LmggYi9pbmNsdWRlL2xpbnV4L25mczQuaA0KPiBpbmRleCAzMjM0NWMyLi44
Y2RkZTRkIDEwMDY0NA0KPiAtLS0gYS9pbmNsdWRlL2xpbnV4L25mczQuaA0KPiArKysgYi9pbmNs
dWRlL2xpbnV4L25mczQuaA0KPiBAQCAtNDQxLDcgKzQ0MSwyMCBAQCBlbnVtIGxpbWl0X2J5NCB7
DQo+ICBlbnVtIG9wZW5fZGVsZWdhdGlvbl90eXBlNCB7DQo+ICAJTkZTNF9PUEVOX0RFTEVHQVRF
X05PTkUgPSAwLA0KPiAgCU5GUzRfT1BFTl9ERUxFR0FURV9SRUFEID0gMSwNCj4gLQlORlM0X09Q
RU5fREVMRUdBVEVfV1JJVEUgPSAyDQo+ICsJTkZTNF9PUEVOX0RFTEVHQVRFX1dSSVRFID0gMiwN
Cj4gKwlORlM0X09QRU5fREVMRUdBVEVfTk9ORV9FWFQgPSAzLCAvKiA0LjEgKi8NCj4gK307DQo+
ICsNCj4gK2VudW0gd2h5X25vX2RlbGVnYXRpb240IHsgLyogbmV3IHRvIHY0LjEgKi8NCj4gKwlX
TkQ0X05PVF9XQU5URUQgPSAwLA0KPiArCVdORDRfQ09OVEVOVElPTiA9IDEsDQo+ICsJV05ENF9S
RVNPVVJDRSA9IDIsDQo+ICsJV05ENF9OT1RfU1VQUF9GVFlQRSA9IDMsDQo+ICsJV05ENF9XUklU
RV9ERUxFR19OT1RfU1VQUF9GVFlQRSA9IDQsDQo+ICsJV05ENF9OT1RfU1VQUF9VUEdSQURFID0g
NSwNCj4gKwlXTkQ0X05PVF9TVVBQX0RPV05HUkFERSA9IDYsDQo+ICsJV05ENF9DQU5DRUxMRUQg
PSA3LA0KPiArCVdORDRfSVNfRElSID0gOCwNCj4gIH07DQo+ICANCj4gIGVudW0gbG9ja190eXBl
NCB7DQo+IC0tIA0KPiAxLjcuNg0KPiANCg==

Re: [PATCH 5/7] nfsd4: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT

[J. Bruce Fields]

On Thu, Oct 20, 2011 at 12:36:25PM +0000, Benny Halevy wrote:
> OK, I read you.  Just please take note that WANT_NO_DELEG flag is important to simplified clients that do not support delegations, like the one I was testing against in the BAT, and it is the only one in use that I could tell.

If you can suggest an implementation that meets the minimum required by
the spec, I'd happily take it....

It might not be much harder: it's probably just a matter of returrning
the right OPEN_DELEGATE_NONE_EXT?

Though unfortunately there's not a "why" for "I just didn't feel like
giving you one", so we probably would have to make an effort to detect
the different case.

Also, I wonder if it's wise for a client to depend on this--it really is
an optional feature as far as I can tell.  It would be safer to teach
the client just to handle delegations in the simplest way possible
(possibly just return them immediately?).

--b.

> 
> Benny
> -----Original Message-----
> From: "J. Bruce Fields" <bfields@fieldses.org>Date: Thu, 20 Oct 2011 07:52:24 
> To: Benny Halevy<benny@tonian.com>
> Cc: J. Bruce Fields<bfields@redhat.com>; <linux-nfs@vger.kernel.org>; Benny Halevy<bhalevy@tonian.com>
> Subject: Re: [PATCH 5/7] nfsd4: implement NFS4_SHARE_WANT_NO_DELEG,
>  NFS4_OPEN_DELEGATE_NONE_EXT
> 
> On Wed, Oct 19, 2011 at 07:13:21PM -0700, Benny Halevy wrote:
> > From: Benny Halevy <bhalevy@tonian.com>
> > 
> > Respect client request for not getting a delegation in NFSv4.1
> > Appropriately return delegation "type" NFS4_OPEN_DELEGATE_NONE_EXT
> > and WND4_NOT_WANTED reason.
> 
> As I read it, if we're going to implement part of this, then we have to
> implement all of it.  ("If the server supports the new _WANT_ flags and
> the client sends one or more of the new flags, then in the event the
> server does not return a delegation, it MUST return a delegation type of
> OPEN_DELEGATE_NONE_EXT.  The field ond_why in the reply indicates why no
> delegation was returned and will be one of....")
> 
> As long as we don't implement any want flags, we escape that MUST.
> 
> So I'd rather put this aside till it's more complete.
> 
> --b.
> 
> > 
> > Signed-off-by: Benny Halevy <bhalevy@tonian.com>
> > ---
> >  fs/nfsd/nfs4state.c  |   10 ++++++++--
> >  fs/nfsd/nfs4xdr.c    |    3 +++
> >  include/linux/nfs4.h |   15 ++++++++++++++-
> >  3 files changed, 25 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index ec361bb..5c3377c 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -2931,15 +2931,21 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
> >  	update_stateid(&stp->st_stid.sc_stateid);
> >  	memcpy(&open->op_stateid, &stp->st_stid.sc_stateid, sizeof(stateid_t));
> >  
> > -	if (nfsd4_has_session(&resp->cstate))
> > +	if (nfsd4_has_session(&resp->cstate)) {
> >  		open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
> >  
> > +		if (open->op_share_access & NFS4_SHARE_WANT_NO_DELEG) {
> > +			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
> > +			goto nodeleg;
> > +		}
> > +	}
> > +
> >  	/*
> >  	* Attempt to hand out a delegation. No error return, because the
> >  	* OPEN succeeds even if we fail.
> >  	*/
> >  	nfs4_open_delegation(current_fh, open, stp);
> > -
> > +nodeleg:
> >  	status = nfs_ok;
> >  
> >  	dprintk("%s: stateid=" STATEID_FMT "\n", __func__,
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index 706ada1..1a419c0 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -2842,6 +2842,9 @@ static __be32 nfsd4_encode_bind_conn_to_session(struct nfsd4_compoundres *resp,
> >  		WRITE32(0);   /* XXX: is NULL principal ok? */
> >  		ADJUST_ARGS();
> >  		break;
> > +	case NFS4_OPEN_DELEGATE_NONE_EXT: /* 4.1 */
> > +		WRITE32(WND4_NOT_WANTED);	/* only reason for now */
> > +		break;
> >  	default:
> >  		BUG();
> >  	}
> > diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> > index 32345c2..8cdde4d 100644
> > --- a/include/linux/nfs4.h
> > +++ b/include/linux/nfs4.h
> > @@ -441,7 +441,20 @@ enum limit_by4 {
> >  enum open_delegation_type4 {
> >  	NFS4_OPEN_DELEGATE_NONE = 0,
> >  	NFS4_OPEN_DELEGATE_READ = 1,
> > -	NFS4_OPEN_DELEGATE_WRITE = 2
> > +	NFS4_OPEN_DELEGATE_WRITE = 2,
> > +	NFS4_OPEN_DELEGATE_NONE_EXT = 3, /* 4.1 */
> > +};
> > +
> > +enum why_no_delegation4 { /* new to v4.1 */
> > +	WND4_NOT_WANTED = 0,
> > +	WND4_CONTENTION = 1,
> > +	WND4_RESOURCE = 2,
> > +	WND4_NOT_SUPP_FTYPE = 3,
> > +	WND4_WRITE_DELEG_NOT_SUPP_FTYPE = 4,
> > +	WND4_NOT_SUPP_UPGRADE = 5,
> > +	WND4_NOT_SUPP_DOWNGRADE = 6,
> > +	WND4_CANCELLED = 7,
> > +	WND4_IS_DIR = 8,
> >  };
> >  
> >  enum lock_type4 {
> > -- 
> > 1.7.6
> > 

Re: [PATCH 5/7] nfsd4: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT

[Benny Halevy]

On 2011-10-20 08:47, J. Bruce Fields wrote:
> On Thu, Oct 20, 2011 at 12:36:25PM +0000, Benny Halevy wrote:
>> OK, I read you.  Just please take note that WANT_NO_DELEG flag is important to simplified clients that do not support delegations, like the one I was testing against in the BAT, and it is the only one in use that I could tell.
> 
> If you can suggest an implementation that meets the minimum required by
> the spec, I'd happily take it....
> 
> It might not be much harder: it's probably just a matter of returrning
> the right OPEN_DELEGATE_NONE_EXT?
> 
> Though unfortunately there's not a "why" for "I just didn't feel like
> giving you one", so we probably would have to make an effort to detect
> the different case.

Agreed.  It could have been a good to have such a reason.

> 
> Also, I wonder if it's wise for a client to depend on this--it really is
> an optional feature as far as I can tell.  It would be safer to teach
> the client just to handle delegations in the simplest way possible
> (possibly just return them immediately?).
> 

That's true.

How about the following? (untested squashme over this patch)

>From ab9e79564fee05a0f3f369fc134bbace1543ac4d Mon Sep 17 00:00:00 2001
From: Benny Halevy <bhalevy@tonian.com>
Date: Thu, 20 Oct 2011 08:00:03 -0700
Subject: [PATCH] SQUASHME: nfsd4: implement why_no_deleg

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4state.c |   50 +++++++++++++++++++++++++++++++++++++++++++++-----
 fs/nfsd/nfs4xdr.c   |    7 ++++++-
 fs/nfsd/xdr4.h      |    1 +
 3 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d00c246..2cdd2b3 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2914,7 +2914,7 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
 	struct nfs4_delegation *dp;
 	struct nfs4_openowner *oo = container_of(stp->st_stateowner, struct nfs4_openowner, oo_owner);
 	int cb_up;
-	int status, flag = 0;
+	int status = 0, flag = 0;

 	cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
 	flag = NFS4_OPEN_DELEGATE_NONE;
@@ -2955,11 +2955,32 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
 	dprintk("NFSD: delegation stateid=" STATEID_FMT "\n",
 		STATEID_VAL(&dp->dl_stid.sc_stateid));
 out:
-	if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS
-			&& flag == NFS4_OPEN_DELEGATE_NONE
-			&& open->op_delegate_type != NFS4_OPEN_DELEGATE_NONE)
-		dprintk("NFSD: WARNING: refusing delegation reclaim\n");
 	open->op_delegate_type = flag;
+	if (flag == NFS4_OPEN_DELEGATE_NONE) {
+		if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS &&
+		    open->op_delegate_type != NFS4_OPEN_DELEGATE_NONE)
+			dprintk("NFSD: WARNING: refusing delegation reclaim\n");
+
+		if (open->op_deleg_want) {
+			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
+			if (status == -EAGAIN)
+				open->op_why_no_deleg = WND4_CONTENTION;
+			else {
+				open->op_why_no_deleg = WND4_RESOURCE;
+				switch (open->op_deleg_want) {
+				case NFS4_SHARE_WANT_READ_DELEG:
+				case NFS4_SHARE_WANT_WRITE_DELEG:
+				case NFS4_SHARE_WANT_ANY_DELEG:
+					break;
+				case NFS4_SHARE_WANT_CANCEL:
+					open->op_why_no_deleg = WND4_CANCELLED;
+					break;
+				case NFS4_SHARE_WANT_NO_DELEG:
+					BUG();	/* not supposed to get here */
+				}
+			}
+		}
+	}
 	return;
 out_free:
 	nfs4_put_delegation(dp);
@@ -3036,6 +3057,7 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)

 		if (open->op_deleg_want & NFS4_SHARE_WANT_NO_DELEG) {
 			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
+			open->op_why_no_deleg = WND4_NOT_WANTED;
 			goto nodeleg;
 		}
 	}
@@ -3051,6 +3073,24 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
 	dprintk("%s: stateid=" STATEID_FMT "\n", __func__,
 		STATEID_VAL(&stp->st_stid.sc_stateid));
 out:
+	/* 4.1 client trying to upgrade/downgrade delegation? */
+	if (open->op_delegate_type == NFS4_OPEN_DELEGATE_NONE && dp &&
+	    open->op_deleg_want) {
+		if (open->op_deleg_want == NFS4_SHARE_WANT_READ_DELEG &&
+		    dp->dl_type == NFS4_OPEN_DELEGATE_WRITE) {
+			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
+			open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
+		} else if (open->op_deleg_want == NFS4_SHARE_WANT_WRITE_DELEG &&
+			   dp->dl_type == NFS4_OPEN_DELEGATE_WRITE) {
+			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
+			open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
+		}
+		/* Otherwise the client must be confused wanting a delegation
+		 * it already has, therefore we don't return
+		 * NFS4_OPEN_DELEGATE_NONE_EXT and reason.
+		 */
+	}
+
 	if (fp)
 		put_nfs4_file(fp);
 	if (status == 0 && open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 9b838e9..f2c1338 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3035,7 +3035,12 @@ static __be32 nfsd4_encode_bind_conn_to_session(struct nfsd4_compoundres *resp,
 		ADJUST_ARGS();
 		break;
 	case NFS4_OPEN_DELEGATE_NONE_EXT: /* 4.1 */
-		WRITE32(WND4_NOT_WANTED);	/* only reason for now */
+		WRITE32(open->op_why_no_deleg);
+		switch (open->op_why_no_deleg) {
+		case WND4_CONTENTION:
+		case WND4_RESOURCE:
+			WRITE32(0);	/* deleg signaling not supported yet */
+		}
 		break;
 	default:
 		BUG();
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 1e5ca95..d20d87e 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -214,6 +214,7 @@ struct nfsd4_open {
 	struct xdr_netobj op_fname;	    /* request - everything but CLAIM_PREV */
 	u32		op_delegate_type;   /* request - CLAIM_PREV only */
 	stateid_t       op_delegate_stateid; /* request - response */
+	u32		op_why_no_deleg;    /* response - DELEG_NONE_EXT only */
 	u32		op_create;     	    /* request */
 	u32		op_createmode;      /* request */
 	u32		op_bmval[3];        /* request */
-- 
1.7.6

Re: [PATCH 5/7] nfsd4: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT

[J. Bruce Fields]

On Thu, Oct 20, 2011 at 03:16:05PM -0400, Benny Halevy wrote:
> On 2011-10-20 08:47, J. Bruce Fields wrote:
> > Also, I wonder if it's wise for a client to depend on this--it really is
> > an optional feature as far as I can tell.  It would be safer to teach
> > the client just to handle delegations in the simplest way possible
> > (possibly just return them immediately?).
> > 
> 
> That's true.
> 
> How about the following? (untested squashme over this patch)

>From a very quick glance--yep, that's sort of what I was thinking.  It
doesn't look too bad.  Nit: I might split some of this into helper
functions  like maybe:

	if (flag == NFS4_OPEN_DELEGATE_NONE)
		nfsd4_set_nodeleg_why(open);

But, squash this with the previous patch, add a changelog, maybe give us
some idea how to test this (some pynfs tests exercising one or two of
these cases would be nice)....  And we'd be done with that.

--b.

> 
> >From ab9e79564fee05a0f3f369fc134bbace1543ac4d Mon Sep 17 00:00:00 2001
> From: Benny Halevy <bhalevy@tonian.com>
> Date: Thu, 20 Oct 2011 08:00:03 -0700
> Subject: [PATCH] SQUASHME: nfsd4: implement why_no_deleg
> 
> Signed-off-by: Benny Halevy <bhalevy@tonian.com>
> ---
>  fs/nfsd/nfs4state.c |   50 +++++++++++++++++++++++++++++++++++++++++++++-----
>  fs/nfsd/nfs4xdr.c   |    7 ++++++-
>  fs/nfsd/xdr4.h      |    1 +
>  3 files changed, 52 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index d00c246..2cdd2b3 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2914,7 +2914,7 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
>  	struct nfs4_delegation *dp;
>  	struct nfs4_openowner *oo = container_of(stp->st_stateowner, struct nfs4_openowner, oo_owner);
>  	int cb_up;
> -	int status, flag = 0;
> +	int status = 0, flag = 0;
> 
>  	cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
>  	flag = NFS4_OPEN_DELEGATE_NONE;
> @@ -2955,11 +2955,32 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
>  	dprintk("NFSD: delegation stateid=" STATEID_FMT "\n",
>  		STATEID_VAL(&dp->dl_stid.sc_stateid));
>  out:
> -	if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS
> -			&& flag == NFS4_OPEN_DELEGATE_NONE
> -			&& open->op_delegate_type != NFS4_OPEN_DELEGATE_NONE)
> -		dprintk("NFSD: WARNING: refusing delegation reclaim\n");
>  	open->op_delegate_type = flag;
> +	if (flag == NFS4_OPEN_DELEGATE_NONE) {
> +		if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS &&
> +		    open->op_delegate_type != NFS4_OPEN_DELEGATE_NONE)
> +			dprintk("NFSD: WARNING: refusing delegation reclaim\n");
> +
> +		if (open->op_deleg_want) {
> +			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
> +			if (status == -EAGAIN)
> +				open->op_why_no_deleg = WND4_CONTENTION;
> +			else {
> +				open->op_why_no_deleg = WND4_RESOURCE;
> +				switch (open->op_deleg_want) {
> +				case NFS4_SHARE_WANT_READ_DELEG:
> +				case NFS4_SHARE_WANT_WRITE_DELEG:
> +				case NFS4_SHARE_WANT_ANY_DELEG:
> +					break;
> +				case NFS4_SHARE_WANT_CANCEL:
> +					open->op_why_no_deleg = WND4_CANCELLED;
> +					break;
> +				case NFS4_SHARE_WANT_NO_DELEG:
> +					BUG();	/* not supposed to get here */
> +				}
> +			}
> +		}
> +	}
>  	return;
>  out_free:
>  	nfs4_put_delegation(dp);
> @@ -3036,6 +3057,7 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
> 
>  		if (open->op_deleg_want & NFS4_SHARE_WANT_NO_DELEG) {
>  			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
> +			open->op_why_no_deleg = WND4_NOT_WANTED;
>  			goto nodeleg;
>  		}
>  	}
> @@ -3051,6 +3073,24 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
>  	dprintk("%s: stateid=" STATEID_FMT "\n", __func__,
>  		STATEID_VAL(&stp->st_stid.sc_stateid));
>  out:
> +	/* 4.1 client trying to upgrade/downgrade delegation? */
> +	if (open->op_delegate_type == NFS4_OPEN_DELEGATE_NONE && dp &&
> +	    open->op_deleg_want) {
> +		if (open->op_deleg_want == NFS4_SHARE_WANT_READ_DELEG &&
> +		    dp->dl_type == NFS4_OPEN_DELEGATE_WRITE) {
> +			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
> +			open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
> +		} else if (open->op_deleg_want == NFS4_SHARE_WANT_WRITE_DELEG &&
> +			   dp->dl_type == NFS4_OPEN_DELEGATE_WRITE) {
> +			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
> +			open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
> +		}
> +		/* Otherwise the client must be confused wanting a delegation
> +		 * it already has, therefore we don't return
> +		 * NFS4_OPEN_DELEGATE_NONE_EXT and reason.
> +		 */
> +	}
> +
>  	if (fp)
>  		put_nfs4_file(fp);
>  	if (status == 0 && open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS)
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 9b838e9..f2c1338 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -3035,7 +3035,12 @@ static __be32 nfsd4_encode_bind_conn_to_session(struct nfsd4_compoundres *resp,
>  		ADJUST_ARGS();
>  		break;
>  	case NFS4_OPEN_DELEGATE_NONE_EXT: /* 4.1 */
> -		WRITE32(WND4_NOT_WANTED);	/* only reason for now */
> +		WRITE32(open->op_why_no_deleg);
> +		switch (open->op_why_no_deleg) {
> +		case WND4_CONTENTION:
> +		case WND4_RESOURCE:
> +			WRITE32(0);	/* deleg signaling not supported yet */
> +		}
>  		break;
>  	default:
>  		BUG();
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index 1e5ca95..d20d87e 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -214,6 +214,7 @@ struct nfsd4_open {
>  	struct xdr_netobj op_fname;	    /* request - everything but CLAIM_PREV */
>  	u32		op_delegate_type;   /* request - CLAIM_PREV only */
>  	stateid_t       op_delegate_stateid; /* request - response */
> +	u32		op_why_no_deleg;    /* response - DELEG_NONE_EXT only */
>  	u32		op_create;     	    /* request */
>  	u32		op_createmode;      /* request */
>  	u32		op_bmval[3];        /* request */
> -- 
> 1.7.6
> 

[PATCH 6/7] nfsd4: typo logical vs bitwise negate for want_mask

[Benny Halevy]

From: Benny Halevy <bhalevy@tonian.com>

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4xdr.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 1a419c0..1a5cb7e 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -671,7 +671,7 @@ static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *x)
 	default:
 		return nfserr_bad_xdr;
 	}
-	w &= !NFS4_SHARE_WANT_MASK;
+	w &= ~NFS4_SHARE_WANT_MASK;
 	if (!w)
 		return nfs_ok;
 	switch (w) {
-- 
1.7.6

Re: [PATCH 6/7] nfsd4: typo logical vs bitwise negate for want_mask

[J. Bruce Fields]

Yep, bad mistake on my part--but someone else already caught this, and
it's already committed to my 3.2 branch.--b.

On Wed, Oct 19, 2011 at 07:13:29PM -0700, Benny Halevy wrote:
> From: Benny Halevy <bhalevy@tonian.com>
> 
> Signed-off-by: Benny Halevy <bhalevy@tonian.com>
> ---
>  fs/nfsd/nfs4xdr.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 1a419c0..1a5cb7e 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -671,7 +671,7 @@ static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *x)
>  	default:
>  		return nfserr_bad_xdr;
>  	}
> -	w &= !NFS4_SHARE_WANT_MASK;
> +	w &= ~NFS4_SHARE_WANT_MASK;
>  	if (!w)
>  		return nfs_ok;
>  	switch (w) {
> -- 
> 1.7.6
> 

[PATCH 7/7] nfsd4: share_access_to_flags should consider only nfs4.x share_access flags

[Benny Halevy]

From: Benny Halevy <bhalevy@tonian.com>

Currently, it will not correctl ignore and nfsv4.1 signal flags if the client
sends them.

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4state.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 5c3377c..a687e1a 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2555,7 +2555,7 @@ static __be32 nfsd4_check_seqid(struct nfsd4_compound_state *cstate, struct nfs4
 
 static int share_access_to_flags(u32 share_access)
 {
-	share_access &= ~NFS4_SHARE_WANT_MASK;
+	share_access &= NFS4_SHARE_ACCESS_MASK;
 
 	return share_access == NFS4_SHARE_ACCESS_READ ? RD_STATE : WR_STATE;
 }
-- 
1.7.6

Re: [PATCH 7/7] nfsd4: share_access_to_flags should consider only nfs4.x share_access flags

[J. Bruce Fields]

On Wed, Oct 19, 2011 at 07:13:37PM -0700, Benny Halevy wrote:
> From: Benny Halevy <bhalevy@tonian.com>
> 
> Currently, it will not correctl ignore and nfsv4.1 signal flags if the client
> sends them.

Good catch, but since b6d2f1ca3c1162f51098969e9c52fd099720416a "nfsd4:
more robust ignoring of WANT bits in OPEN" we're actually doing this
right from the start, and this is redundant.

(And remembering to mask out the other bits on every use is so
error-prone, I'm wondering if we should actually put the other bits in a
separate field entirely.  Either that or maybe provide a little helper
function to get the access bits and ensure they're never accessed any
other way.)

--b.

> 
> Signed-off-by: Benny Halevy <bhalevy@tonian.com>
> ---
>  fs/nfsd/nfs4state.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 5c3377c..a687e1a 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2555,7 +2555,7 @@ static __be32 nfsd4_check_seqid(struct nfsd4_compound_state *cstate, struct nfs4
>  
>  static int share_access_to_flags(u32 share_access)
>  {
> -	share_access &= ~NFS4_SHARE_WANT_MASK;
> +	share_access &= NFS4_SHARE_ACCESS_MASK;
>  
>  	return share_access == NFS4_SHARE_ACCESS_READ ? RD_STATE : WR_STATE;
>  }
> -- 
> 1.7.6
> 

Re: [PATCH 7/7] nfsd4: share_access_to_flags should consider only nfs4.x share_access flags

[Benny Halevy]

On 2011-10-20 04:56, J. Bruce Fields wrote:
> On Wed, Oct 19, 2011 at 07:13:37PM -0700, Benny Halevy wrote:
>> From: Benny Halevy <bhalevy@tonian.com>
>>
>> Currently, it will not correctl ignore and nfsv4.1 signal flags if the client
>> sends them.
> 
> Good catch, but since b6d2f1ca3c1162f51098969e9c52fd099720416a "nfsd4:
> more robust ignoring of WANT bits in OPEN" we're actually doing this
> right from the start, and this is redundant.
> 
> (And remembering to mask out the other bits on every use is so
> error-prone, I'm wondering if we should actually put the other bits in a
> separate field entirely.  Either that or maybe provide a little helper
> function to get the access bits and ensure they're never accessed any
> other way.)

Agreed, something along these lines? (untested)

>From 3eb3100dc8c8c3833ae24b09d356001245b7e691 Mon Sep 17 00:00:00 2001
From: Benny Halevy <bhalevy@tonian.com>
Date: Thu, 20 Oct 2011 07:12:47 -0700
Subject: [PATCH] nfsd4: split out share_access want and signel flags while decoding

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4proc.c  |    3 ---
 fs/nfsd/nfs4state.c |    8 ++++----
 fs/nfsd/nfs4xdr.c   |   25 +++++++++++++++++++------
 fs/nfsd/xdr4.h      |    6 ++++--
 4 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 2f87ea5..65f6c86 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -319,9 +319,6 @@ static __be32 nfsd_check_obj_isreg(struct svc_fh *fh)
 	if (open->op_create && open->op_claim_type != NFS4_OPEN_CLAIM_NULL)
 		return nfserr_inval;

-	/* We don't yet support WANT bits: */
-	open->op_share_access &= NFS4_SHARE_ACCESS_MASK;
-
 	/*
 	 * RFC5661 18.51.3
 	 * Before RECLAIM_COMPLETE done, server should deny new lock
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 9701d71..d00c246 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2657,8 +2657,6 @@ static __be32 nfsd4_check_seqid(struct nfsd4_compound_state *cstate, struct nfs4

 static int share_access_to_flags(u32 share_access)
 {
-	share_access &= NFS4_SHARE_ACCESS_MASK;
-
 	return share_access == NFS4_SHARE_ACCESS_READ ? RD_STATE : WR_STATE;
 }

@@ -3036,7 +3034,7 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
 	if (nfsd4_has_session(&resp->cstate)) {
 		open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;

-		if (open->op_share_access & NFS4_SHARE_WANT_NO_DELEG) {
+		if (open->op_deleg_want & NFS4_SHARE_WANT_NO_DELEG) {
 			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
 			goto nodeleg;
 		}
@@ -3654,7 +3652,9 @@ static inline void nfs4_stateid_downgrade(struct nfs4_ol_stateid *stp, u32 to_ac
 			cstate->current_fh.fh_dentry->d_name.name);

 	/* We don't yet support WANT bits: */
-	od->od_share_access &= NFS4_SHARE_ACCESS_MASK;
+	if (od->od_deleg_want)
+		dprintk("NFSD: %s: od_deleg_want=0x%x ignored\n", __func__,
+			od->od_deleg_want);

 	nfs4_lock_state();
 	status = nfs4_preprocess_confirmed_seqid_op(cstate, od->od_seqid,
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 79cb105..9b838e9 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -644,15 +644,19 @@ static __be32 nfsd4_decode_bind_conn_to_session(struct nfsd4_compoundargs *argp,
 	DECODE_TAIL;
 }

-static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *x)
+static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *share_access, u32 *deleg_want, u32 *deleg_when)
 {
 	__be32 *p;
 	u32 w;

 	READ_BUF(4);
 	READ32(w);
-	*x = w;
-	switch (w & NFS4_SHARE_ACCESS_MASK) {
+	*share_access = w & NFS4_SHARE_ACCESS_MASK;
+	*deleg_want = w & NFS4_SHARE_WANT_MASK;
+	if (deleg_when)
+		*deleg_when = w & NFS4_SHARE_WHEN_MASK;
+
+	switch (w & NFS4_SHARE_WHEN_MASK) {
 	case NFS4_SHARE_ACCESS_READ:
 	case NFS4_SHARE_ACCESS_WRITE:
 	case NFS4_SHARE_ACCESS_BOTH:
@@ -660,11 +664,13 @@ static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *x)
 	default:
 		return nfserr_bad_xdr;
 	}
-	w &= !NFS4_SHARE_ACCESS_MASK;
+	w &= ~NFS4_SHARE_ACCESS_MASK;
 	if (!w)
 		return nfs_ok;
+
 	if (!argp->minorversion)
 		return nfserr_bad_xdr;
+
 	switch (w & NFS4_SHARE_WANT_MASK) {
 	case NFS4_SHARE_WANT_NO_PREFERENCE:
 	case NFS4_SHARE_WANT_READ_DELEG:
@@ -679,6 +685,9 @@ static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *x)
 	w &= ~NFS4_SHARE_WANT_MASK;
 	if (!w)
 		return nfs_ok;
+
+	if (!deleg_when)	/* open_downgrade */
+		return nfserr_inval;
 	switch (w) {
 	case NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL:
 	case NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED:
@@ -725,6 +734,7 @@ static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_ne
 nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
 {
 	DECODE_HEAD;
+	u32 dummy;

 	memset(open->op_bmval, 0, sizeof(open->op_bmval));
 	open->op_iattr.ia_valid = 0;
@@ -733,7 +743,9 @@ static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_ne
 	/* seqid, share_access, share_deny, clientid, ownerlen */
 	READ_BUF(4);
 	READ32(open->op_seqid);
-	status = nfsd4_decode_share_access(argp, &open->op_share_access);
+	/* decode, yet ignore deleg_when until supported */
+	status = nfsd4_decode_share_access(argp, &open->op_share_access,
+					   &open->op_deleg_want, &dummy);
 	if (status)
 		goto xdr_error;
 	status = nfsd4_decode_share_deny(argp, &open->op_share_deny);
@@ -854,7 +866,8 @@ static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_ne
 		return status;
 	READ_BUF(4);
 	READ32(open_down->od_seqid);
-	status = nfsd4_decode_share_access(argp, &open_down->od_share_access);
+	status = nfsd4_decode_share_access(argp, &open_down->od_share_access,
+					   &open_down->od_deleg_want, NULL);
 	if (status)
 		return status;
 	status = nfsd4_decode_share_deny(argp, &open_down->od_share_deny);
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index bf4c9e7..1e5ca95 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -224,6 +224,7 @@ struct nfsd4_open {
 	u32		op_seqid;           /* request */
 	u32		op_share_access;    /* request */
 	u32		op_share_deny;      /* request */
+	u32		op_deleg_want;      /* request */
 	stateid_t	op_stateid;         /* response */
 	u32		op_recall;          /* recall */
 	struct nfsd4_change_info  op_cinfo; /* response */
@@ -244,8 +245,9 @@ struct nfsd4_open_confirm {
 struct nfsd4_open_downgrade {
 	stateid_t       od_stateid;
 	u32             od_seqid;
-	u32             od_share_access;
-	u32             od_share_deny;
+	u32             od_share_access;	/* request */
+	u32		od_deleg_want;		/* request */
+	u32             od_share_deny;		/* request */
 };


-- 
1.7.6

Re: [PATCH 0/7] Bakeathon server fixes

[J. Bruce Fields]

On Wed, Oct 19, 2011 at 07:10:04PM -0700, Benny Halevy wrote:
> Here's the list of patches I tested here in the Bakeathon.

Thanks, Benny!  I need to run some tests of my own, and then I intend to
apply the first four (see individual replies for comments on the last
three).

--b.

> They pass the Connectathon tests over nfsv4.1 (no pnfs)
> against the linux client, but VMware reports a bug with
> OPEN with share_access==0x401 (1025)
> NFS4_SHARE_WANT_NO_DELEG | NFS4_SHARE_ACCESS_READ
> return NFS4ERR_GRACE for some reason.
> I haven't been able to track that down yet.
> 
> [PATCH 1/7] nfsd4: implement new 4.1 open reclaim types
> [PATCH 2/7] nfsd41: use SEQ4_STATUS_BACKCHANNEL_FAULT when
> [PATCH 3/7] nfsd4: seq->status_flags may be used unitialized
> [PATCH 4/7] nfsd4: allow NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL |
> [PATCH 5/7] nfsd4: implement NFS4_SHARE_WANT_NO_DELEG,
> [PATCH 6/7] nfsd4: typo logical vs bitwise negate for want_mask
> [PATCH 7/7] nfsd4: share_access_to_flags should consider only nfs4.x