[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
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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Currently, it will not correctly ignore any 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(-)
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
Signed-off-by: Benny Halevy <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <[email protected]>
---
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
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.
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.
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.