[PATCH 0/3] nfsd41: want delegation flags

[PATCH 0/3] nfsd41: want delegation flags

[Benny Halevy]

[PATCH 1/3] nfsd41: share_access_to_flags should consider only nfs4.x share_access flags
[PATCH 2/3] nfsd41: split out share_access want and signel flags while decoding
[PATCH 3/3] nfsd41: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT, why_no_deleg

[PATCH 1/3] nfsd41: share_access_to_flags should consider only nfs4.x share_access flags

[Benny Halevy]

Currently, it will not correctly ignore any 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 e8c98f0..a6cdbd5 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2633,7 +2633,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.5

[PATCH 2/3] nfsd41: split out share_access want and signel flags while decoding

[Benny Halevy]

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

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 896da74..a9040be 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -310,9 +310,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;
-
 	open->op_created = 0;
 	/*
 	 * RFC5661 18.51.3
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a6cdbd5..c165df7 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2633,8 +2633,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;
 }
 
@@ -3596,7 +3594,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 0ec5a1b..78d2bb8 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -638,14 +638,18 @@ 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;
+	*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_ACCESS_MASK) {
 	case NFS4_SHARE_ACCESS_READ:
 	case NFS4_SHARE_ACCESS_WRITE:
@@ -673,6 +677,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:
@@ -719,6 +726,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;
@@ -727,7 +735,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);
@@ -848,7 +858,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 2364747..1771a91 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -222,6 +222,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 */
@@ -245,8 +246,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.5

[PATCH 3/3] nfsd41: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT, why_no_deleg

[Benny Halevy]

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.

[nfsd41: add missing break when encoding op_why_no_deleg]
Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4state.c  |   60 ++++++++++++++++++++++++++++++++++++++++++++-----
 fs/nfsd/nfs4xdr.c    |   14 +++++++++++
 fs/nfsd/xdr4.h       |    1 +
 include/linux/nfs4.h |   15 +++++++++++-
 4 files changed, 82 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c165df7..ad21b06 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2862,7 +2862,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;
@@ -2903,11 +2903,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);
@@ -2977,20 +2998,45 @@ 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_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;
+		}
+	}
+
 	/*
 	* 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__,
 		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_UPGRADE;
+		}
+		/* 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 78d2bb8..c2cf857 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2862,6 +2862,20 @@ 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 */
+		switch (open->op_why_no_deleg) {
+		case WND4_CONTENTION:
+		case WND4_RESOURCE:
+			RESERVE_SPACE(8);
+			WRITE32(open->op_why_no_deleg);
+			WRITE32(0);	/* deleg signaling not supported yet */
+			break;
+		default:
+			RESERVE_SPACE(4);
+			WRITE32(open->op_why_no_deleg);
+		}
+		ADJUST_ARGS();
+		break;
 	default:
 		BUG();
 	}
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 1771a91..fea600f 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -212,6 +212,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 */
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.5

Re: [PATCH 3/3] nfsd41: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT, why_no_deleg

[J. Bruce Fields]

On Thu, Feb 16, 2012 at 08:57:17PM +0200, Benny Halevy wrote:
> 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.

These all look fine, just two nits:

> @@ -2903,11 +2903,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 */
> +				}
> +			}
> +		}
> +	}

This looks like a reasonably well-encapsulated bit of code--could you
move it into a helper function?

>  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_UPGRADE;
> +		}
> +		/* Otherwise the client must be confused wanting a delegation
> +		 * it already has, therefore we don't return
> +		 * NFS4_OPEN_DELEGATE_NONE_EXT and reason.
> +		 */
> +	}

Ditto for this.

I'll go ahead and apply the previous two patches pending some regression
tests.

--b.

Re: [PATCH 3/3] nfsd41: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT, why_no_deleg

[Benny Halevy]

On 2012-02-18 00:14, J. Bruce Fields wrote:
> On Thu, Feb 16, 2012 at 08:57:17PM +0200, Benny Halevy wrote:
>> 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.
> 
> These all look fine, just two nits:
> 
>> @@ -2903,11 +2903,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 */
>> +				}
>> +			}
>> +		}
>> +	}
> 
> This looks like a reasonably well-encapsulated bit of code--could you
> move it into a helper function?
> 
>>  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_UPGRADE;
>> +		}
>> +		/* Otherwise the client must be confused wanting a delegation
>> +		 * it already has, therefore we don't return
>> +		 * NFS4_OPEN_DELEGATE_NONE_EXT and reason.
>> +		 */
>> +	}
> 
> Ditto for this.
> 
> I'll go ahead and apply the previous two patches pending some regression
> tests.

Thanks!
I'm sending two additional patches that refactor these two helpers and add
some documentation to the commit message.

I think it might be helpful to keep them separated but let me know otherwise
and I'll send a squashed patch instead.

Benny

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

[PATCH 1/2] nfsd41: refactor nfs4_open_deleg_none_ext logic out of nfs4_open_delegation

[Benny Halevy]

When a 4.1 client asks for a delegation and the server returns none
op_delegate_type is set to NFS4_OPEN_DELEGATE_NONE_EXT
and op_why_no_deleg is set to either WND4_CONTENTION or WND4_RESOURCE.
Or, if the client sent a NFS4_SHARE_WANT_CANCEL (which it is not supposed
to ever do until our server supports delegations signaling),
op_why_no_deleg is set to WND4_CANCELLED.

Note that for WND4_CONTENTION and WND4_RESOURCE, the xdr layer is hard coded
at this time to encode boolean FALSE for ond_server_will_push_deleg /
ond_server_will_signal_avail.

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

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ad21b06..94daab4 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2853,6 +2853,27 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
 	return 0;
 }
 
+static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
+{
+	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 */
+		}
+	}
+}
+
 /*
  * Attempt to hand out a delegation.
  */
@@ -2909,25 +2930,9 @@ static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
 		    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 */
-				}
-			}
-		}
+		/* 4.1 client asking for a delegation? */
+		if (open->op_deleg_want)
+			nfsd4_open_deleg_none_ext(open, status);
 	}
 	return;
 out_free:
-- 
1.7.6.5

[PATCH 2/2] nfsd41: refactor nfsd4_deleg_xgrade_none_ext logic out of nfsd4_process_open2

[Benny Halevy]

Handle the case where the nfsv4.1 client asked to uprade or downgrade
its delegations and server returns no delegation.

In this case, op_delegate_type is set to NFS4_OPEN_DELEGATE_NONE_EXT
and op_why_no_deleg is set respectively to WND4_NOT_SUPP_{UP,DOWN}GRADE

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

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 94daab4..5bfc846 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2942,6 +2942,24 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
 	goto out;
 }
 
+static void nfsd4_deleg_xgrade_none_ext(struct nfsd4_open *open,
+					struct nfs4_delegation *dp)
+{
+	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_UPGRADE;
+	}
+	/* Otherwise the client must be confused wanting a delegation
+	 * it already has, therefore we don't return
+	 * NFS4_OPEN_DELEGATE_NONE_EXT and reason.
+	 */
+}
+
 /*
  * called with nfs4_lock_state() held.
  */
@@ -3026,21 +3044,8 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
 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_UPGRADE;
-		}
-		/* Otherwise the client must be confused wanting a delegation
-		 * it already has, therefore we don't return
-		 * NFS4_OPEN_DELEGATE_NONE_EXT and reason.
-		 */
-	}
+	    open->op_deleg_want)
+		nfsd4_deleg_xgrade_none_ext(open, dp);
 
 	if (fp)
 		put_nfs4_file(fp);
-- 
1.7.6.5

Re: [PATCH 3/3] nfsd41: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT, why_no_deleg

[J. Bruce Fields]

On Sat, Feb 18, 2012 at 10:02:56AM +0200, Benny Halevy wrote:
> On 2012-02-18 00:14, J. Bruce Fields wrote:
> > On Thu, Feb 16, 2012 at 08:57:17PM +0200, Benny Halevy wrote:
> >> 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.
> > 
> > These all look fine, just two nits:
> > 
> >> @@ -2903,11 +2903,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 */
> >> +				}
> >> +			}
> >> +		}
> >> +	}
> > 
> > This looks like a reasonably well-encapsulated bit of code--could you
> > move it into a helper function?
> > 
> >>  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_UPGRADE;
> >> +		}
> >> +		/* Otherwise the client must be confused wanting a delegation
> >> +		 * it already has, therefore we don't return
> >> +		 * NFS4_OPEN_DELEGATE_NONE_EXT and reason.
> >> +		 */
> >> +	}
> > 
> > Ditto for this.
> > 
> > I'll go ahead and apply the previous two patches pending some regression
> > tests.
> 
> Thanks!
> I'm sending two additional patches that refactor these two helpers and add
> some documentation to the commit message.
> 
> I think it might be helpful to keep them separated but let me know otherwise
> and I'll send a squashed patch instead.

Either way's fine.  What I've got so far is pushed out to

	git://linux-nfs.org/~bfields/linux.git for-3.4

--b.

Re: [PATCH 3/3] nfsd41: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT, why_no_deleg

[Benny Halevy]

On 2012-02-18 06:41, J. Bruce Fields wrote:
> On Sat, Feb 18, 2012 at 10:02:56AM +0200, Benny Halevy wrote:
>> On 2012-02-18 00:14, J. Bruce Fields wrote:
>>> On Thu, Feb 16, 2012 at 08:57:17PM +0200, Benny Halevy wrote:
>>>> 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.
>>>
>>> These all look fine, just two nits:
>>>
>>>> @@ -2903,11 +2903,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 */
>>>> +				}
>>>> +			}
>>>> +		}
>>>> +	}
>>>
>>> This looks like a reasonably well-encapsulated bit of code--could you
>>> move it into a helper function?
>>>
>>>>  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_UPGRADE;
>>>> +		}
>>>> +		/* Otherwise the client must be confused wanting a delegation
>>>> +		 * it already has, therefore we don't return
>>>> +		 * NFS4_OPEN_DELEGATE_NONE_EXT and reason.
>>>> +		 */
>>>> +	}
>>>
>>> Ditto for this.
>>>
>>> I'll go ahead and apply the previous two patches pending some regression
>>> tests.
>>
>> Thanks!
>> I'm sending two additional patches that refactor these two helpers and add
>> some documentation to the commit message.
>>
>> I think it might be helpful to keep them separated but let me know otherwise
>> and I'll send a squashed patch instead.
> 
> Either way's fine.  What I've got so far is pushed out to
> 
> 	git://linux-nfs.org/~bfields/linux.git for-3.4

Good. So the missing patches are:
http://marc.info/?l=linux-nfs&m=132941866004294&w=2
http://marc.info/?l=linux-nfs&m=132955306211255&w=2
http://marc.info/?l=linux-nfs&m=132955306211256&w=2

Benny

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

Re: [PATCH 3/3] nfsd41: implement NFS4_SHARE_WANT_NO_DELEG, NFS4_OPEN_DELEGATE_NONE_EXT, why_no_deleg

[J. Bruce Fields]

On Mon, Feb 20, 2012 at 07:47:19AM -0800, Benny Halevy wrote:
> On 2012-02-18 06:41, J. Bruce Fields wrote:
> > On Sat, Feb 18, 2012 at 10:02:56AM +0200, Benny Halevy wrote:
> >> I'm sending two additional patches that refactor these two helpers and add
> >> some documentation to the commit message.
> >>
> >> I think it might be helpful to keep them separated but let me know otherwise
> >> and I'll send a squashed patch instead.
> > 
> > Either way's fine.  What I've got so far is pushed out to
> > 
> > 	git://linux-nfs.org/~bfields/linux.git for-3.4
> 
> Good. So the missing patches are:
> http://marc.info/?l=linux-nfs&m=132941866004294&w=2
> http://marc.info/?l=linux-nfs&m=132955306211255&w=2
> http://marc.info/?l=linux-nfs&m=132955306211256&w=2

Sorry for the delay.  I've committed and pushed out everything I have
from you (to git://linux-nfs.org/~bfields/linux.git for-3.4).  Let me
know if I've overlooked anything.

--b.