Bakeathon server fixes

From: "J. Bruce Fields" <[email protected]>

Signed-off-by Benny Halevy <[email protected]>

---
 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(-)

fs/nfsd/nfs4proc.c +3 -12

index 5b192a2..e3fffab 100644
@@ -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",

fs/nfsd/nfs4state.c +8 -2

index 62aa91a..7da0748 100644
@@ -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);

fs/nfsd/nfs4xdr.c +13 -0

index 645a0a9..fdc09a5 100644
@@ -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;
 	}

include/linux/nfs4.h +4 -1

index b875b03..32345c2 100644
@@ -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 {

Signature · 1 line

1.7.6

From: Benny Halevy <[email protected]>

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

---
 fs/nfsd/nfs4state.c  |   10 ++++++++--
 fs/nfsd/nfs4xdr.c    |    3 +++
 include/linux/nfs4.h |   15 ++++++++++++++-
 3 files changed, 25 insertions(+), 3 deletions(-)

fs/nfsd/nfs4state.c +8 -2

index ec361bb..5c3377c 100644
@@ -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__,

fs/nfsd/nfs4xdr.c +3 -0

index 706ada1..1a419c0 100644
@@ -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();
 	}

include/linux/nfs4.h +14 -1

index 32345c2..8cdde4d 100644
@@ -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 {

Signature · 1 line

1.7.6

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.

Benny
-----Original Message-----
From: "J. Bruce Fields" <[email protected]>Date: Thu, 20 Oct 2011 07:52:24
To: Benny Halevy<[email protected]>
Cc: J. Bruce Fields<[email protected]>; <[email protected]>; Benny Halevy<[email protected]>
Subject: Re: [PATCH 5/7] nfsd4: implement NFS4_SHARE_WANT_NO_DELEG,
NFS4_OPEN_DELEGATE_NONE_EXT

Quoted 5 lines · Benny Halevy

From: Benny Halevy <[email protected]>

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.

Quoted 79 lines

Signed-off-by Benny Halevy <[email protected]>

---
 fs/nfsd/nfs4state.c  |   10 ++++++++--
 fs/nfsd/nfs4xdr.c    |    3 +++
 include/linux/nfs4.h |   15 ++++++++++++++-
 3 files changed, 25 insertions(+), 3 deletions(-)

fs/nfsd/nfs4state.c +8 -2

index ec361bb..5c3377c 100644
@@ -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__,

fs/nfsd/nfs4xdr.c +3 -0

index 706ada1..1a419c0 100644
@@ -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();
 	}

include/linux/nfs4.h +14 -1

index 32345c2..8cdde4d 100644
@@ -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 {

Signature · 1 line

1.7.6

Quoted 1 line · Benny Halevy

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?).

From ab9e79564fee05a0f3f369fc134bbace1543ac4d Mon Sep 17 00:00:00 2001

Quoted 5 lines · J. Bruce Fields

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

Quoted 2 lines

From ab9e79564fee05a0f3f369fc134bbace1543ac4d Mon Sep 17 00:00:00 2001

From: Benny Halevy <[email protected]>
Date: Thu, 20 Oct 2011 08:00:03 -0700
Subject: [PATCH] SQUASHME: nfsd4: implement why_no_deleg

Signed-off-by Benny Halevy <[email protected]>

---
 fs/nfsd/nfs4state.c |   50 +++++++++++++++++++++++++++++++++++++++++++++-----
 fs/nfsd/nfs4xdr.c   |    7 ++++++-
 fs/nfsd/xdr4.h      |    1 +
 3 files changed, 52 insertions(+), 6 deletions(-)

fs/nfsd/nfs4state.c +45 -5

index d00c246..2cdd2b3 100644
@@ -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)

fs/nfsd/nfs4xdr.c +6 -1

index 9b838e9..f2c1338 100644
@@ -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();

fs/nfsd/xdr4.h +1 -0

index 1e5ca95..d20d87e 100644
@@ -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 */

Signature · 1 line

1.7.6

From: Benny Halevy <[email protected]>

Signed-off-by Benny Halevy <[email protected]>

---
 fs/nfsd/nfs4xdr.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

fs/nfsd/nfs4xdr.c +1 -1

index 1a419c0..1a5cb7e 100644
@@ -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) {

Signature · 1 line

1.7.6

From: Benny Halevy <[email protected]>

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

Signed-off-by Benny Halevy <[email protected]>

---
 fs/nfsd/nfs4state.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

fs/nfsd/nfs4state.c +1 -1

index 5c3377c..a687e1a 100644
@@ -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;
 }

Signature · 1 line

1.7.6

Quoted 4 lines · Benny Halevy

From: Benny Halevy <[email protected]>

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.)

From 3eb3100dc8c8c3833ae24b09d356001245b7e691 Mon Sep 17 00:00:00 2001

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