* [PATCH 1/5] nfsd4: fix exchange_id to return confirm flag
2012-05-30 18:36 exchange_id/create_session bugfixes J. Bruce Fields
@ 2012-05-30 18:36 ` J. Bruce Fields
2012-05-30 18:36 ` [PATCH 2/5] nfsd4: return "real" sequence id in confirmed case J. Bruce Fields
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2012-05-30 18:36 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
Otherwise nfsd4_set_ex_flags writes over the return flags.
Reported-by: Bryan Schumaker <bjschuma@netapp.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfsd/nfs4state.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index be1fc97..d53ad72 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1617,7 +1617,7 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
goto expire_client;
}
if (verfs_match) { /* case 2 */
- exid->flags |= EXCHGID4_FLAG_CONFIRMED_R;
+ conf->cl_exchange_flags |= EXCHGID4_FLAG_CONFIRMED_R;
new = conf;
goto out_copy;
}
--
1.7.10.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/5] nfsd4: return "real" sequence id in confirmed case
2012-05-30 18:36 exchange_id/create_session bugfixes J. Bruce Fields
2012-05-30 18:36 ` [PATCH 1/5] nfsd4: fix exchange_id to return confirm flag J. Bruce Fields
@ 2012-05-30 18:36 ` J. Bruce Fields
2012-05-30 18:36 ` [PATCH 3/5] nfsd4: remove some dprintk's and a comment J. Bruce Fields
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2012-05-30 18:36 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
The client should ignore the returned sequence_id in the case where the
CONFIRMED flag is set on an exchange_id reply--and in the unconfirmed
case "1" is always the right response. So it shouldn't actually matter
what we return here.
We could continue returning 1 just to catch clients ignoring the spec
here, but I'd rather be generous. Other things equal, returning the
existing sequence_id seems more informative.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfsd/nfs4state.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d53ad72..bcb4b50 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1650,7 +1650,7 @@ out_copy:
exid->clientid.cl_boot = new->cl_clientid.cl_boot;
exid->clientid.cl_id = new->cl_clientid.cl_id;
- exid->seqid = 1;
+ exid->seqid = new->cl_cs_slot.sl_seqid + 1;
nfsd4_set_ex_flags(new, exid);
dprintk("nfsd4_exchange_id seqid %d flags %x\n",
--
1.7.10.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/5] nfsd4: remove some dprintk's and a comment
2012-05-30 18:36 exchange_id/create_session bugfixes J. Bruce Fields
2012-05-30 18:36 ` [PATCH 1/5] nfsd4: fix exchange_id to return confirm flag J. Bruce Fields
2012-05-30 18:36 ` [PATCH 2/5] nfsd4: return "real" sequence id in confirmed case J. Bruce Fields
@ 2012-05-30 18:36 ` J. Bruce Fields
2012-05-30 18:36 ` [PATCH 4/5] nfsd4: don't remove rebooted client record until confirmation J. Bruce Fields
2012-05-30 18:36 ` [PATCH 5/5] nfsd4: fix, consolidate client_has_state J. Bruce Fields
4 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2012-05-30 18:36 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
The comment is redundant, and if we really want dprintk's here they'd
probably be better in the common (check-slot_seqid) code.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfsd/nfs4state.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index bcb4b50..c03c951 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1750,16 +1750,10 @@ nfsd4_create_session(struct svc_rqst *rqstp,
cs_slot = &conf->cl_cs_slot;
status = check_slot_seqid(cr_ses->seqid, cs_slot->sl_seqid, 0);
if (status == nfserr_replay_cache) {
- dprintk("Got a create_session replay! seqid= %d\n",
- cs_slot->sl_seqid);
- /* Return the cached reply status */
status = nfsd4_replay_create_session(cr_ses, cs_slot);
goto out;
} else if (cr_ses->seqid != cs_slot->sl_seqid + 1) {
status = nfserr_seq_misordered;
- dprintk("Sequence misordered!\n");
- dprintk("Expected seqid= %d but got seqid= %d\n",
- cs_slot->sl_seqid, cr_ses->seqid);
goto out;
}
} else if (unconf) {
@@ -1768,7 +1762,6 @@ nfsd4_create_session(struct svc_rqst *rqstp,
status = nfserr_clid_inuse;
goto out;
}
-
cs_slot = &unconf->cl_cs_slot;
status = check_slot_seqid(cr_ses->seqid, cs_slot->sl_seqid, 0);
if (status) {
@@ -1776,7 +1769,6 @@ nfsd4_create_session(struct svc_rqst *rqstp,
status = nfserr_seq_misordered;
goto out;
}
-
confirm_me = true;
conf = unconf;
} else {
--
1.7.10.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/5] nfsd4: don't remove rebooted client record until confirmation
2012-05-30 18:36 exchange_id/create_session bugfixes J. Bruce Fields
` (2 preceding siblings ...)
2012-05-30 18:36 ` [PATCH 3/5] nfsd4: remove some dprintk's and a comment J. Bruce Fields
@ 2012-05-30 18:36 ` J. Bruce Fields
2012-05-30 18:36 ` [PATCH 5/5] nfsd4: fix, consolidate client_has_state J. Bruce Fields
4 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2012-05-30 18:36 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
In the NFSv4.1 client-reboot case we're currently removing the client's
previous state in exchange_id. That's wrong--we should be waiting till
the confirming create_session.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfsd/nfs4state.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c03c951..4bd5e4d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1614,7 +1614,8 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
status = nfserr_clid_inuse;
goto out;
}
- goto expire_client;
+ expire_client(conf);
+ goto out_new;
}
if (verfs_match) { /* case 2 */
conf->cl_exchange_flags |= EXCHGID4_FLAG_CONFIRMED_R;
@@ -1622,8 +1623,6 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
goto out_copy;
}
/* case 5, client reboot */
-expire_client:
- expire_client(conf);
goto out_new;
}
@@ -1805,8 +1804,14 @@ nfsd4_create_session(struct svc_rqst *rqstp,
/* cache solo and embedded create sessions under the state lock */
nfsd4_cache_create_session(cr_ses, cs_slot, status);
- if (confirm_me)
+ if (confirm_me) {
+ unsigned int hash = clientstr_hashval(unconf->cl_recdir);
+ struct nfs4_client *old =
+ find_confirmed_client_by_str(conf->cl_recdir, hash);
+ if (old)
+ expire_client(old);
move_to_confirmed(conf);
+ }
out:
nfs4_unlock_state();
dprintk("%s returns %d\n", __func__, ntohl(status));
--
1.7.10.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 5/5] nfsd4: fix, consolidate client_has_state
2012-05-30 18:36 exchange_id/create_session bugfixes J. Bruce Fields
` (3 preceding siblings ...)
2012-05-30 18:36 ` [PATCH 4/5] nfsd4: don't remove rebooted client record until confirmation J. Bruce Fields
@ 2012-05-30 18:36 ` J. Bruce Fields
4 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2012-05-30 18:36 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
Whoops: first, I reimplemented the already-existing has_resources
without noticing; second, I got the test backwards. I did pick a better
name, though. Combine the two....
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfsd/nfs4state.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 4bd5e4d..c8da5c8 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1537,9 +1537,9 @@ static bool client_has_state(struct nfs4_client *clp)
*
* Also note we should probably be using this in 4.0 case too.
*/
- return list_empty(&clp->cl_openowners)
- && list_empty(&clp->cl_delegations)
- && list_empty(&clp->cl_sessions);
+ return !list_empty(&clp->cl_openowners)
+ || !list_empty(&clp->cl_delegations)
+ || !list_empty(&clp->cl_sessions);
}
__be32
@@ -2069,13 +2069,6 @@ out:
return status;
}
-static inline bool has_resources(struct nfs4_client *clp)
-{
- return !list_empty(&clp->cl_openowners)
- || !list_empty(&clp->cl_delegations)
- || !list_empty(&clp->cl_sessions);
-}
-
__be32
nfsd4_destroy_clientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, struct nfsd4_destroy_clientid *dc)
{
@@ -2089,7 +2082,7 @@ nfsd4_destroy_clientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *csta
if (conf) {
clp = conf;
- if (!is_client_expired(conf) && has_resources(conf)) {
+ if (!is_client_expired(conf) && client_has_state(conf)) {
status = nfserr_clientid_busy;
goto out;
}
--
1.7.10.2
^ permalink raw reply related [flat|nested] 6+ messages in thread