On Tue, May 11, 2021 at 03:59:00PM +0000, Chuck Lever III wrote:
> As Dave reported yesterday, this patch is unfinished and is probably
> junk. But any thoughts on how the tracepoints should be organized
> in this code would help.
>
> So I was thinking we probably want a tracepoint to fire for each
> case that is handled in this code (and in nfsd4_exchangeid).
> However, this comment in nfsd4_setclientid:
>
> /* Cases below refer to rfc 3530 section 14.2.33: */
>
> Is confusing.
>
> - RFC 3530 is superceded by RFC 7530, and the section numbers have changed.
>
> - The cases in this section in both RFCs aren't numbered, they are
> bullet points.
Honestly I think those particular comments should just go. The code
doesn't even follow those bullet points very closely any more.
--b.
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f47f72bc871f..2aa5d15b08ed 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3954,11 +3954,9 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
new = create_client(clname, rqstp, &clverifier);
if (new == NULL)
return nfserr_jukebox;
- /* Cases below refer to rfc 3530 section 14.2.33: */
spin_lock(&nn->client_lock);
conf = find_confirmed_client_by_name(&clname, nn);
if (conf && client_has_state(conf)) {
- /* case 0: */
status = nfserr_clid_inuse;
if (clp_used_exchangeid(conf))
goto out;
@@ -3970,7 +3968,6 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
unconf = find_unconfirmed_client_by_name(&clname, nn);
if (unconf)
unhash_client_locked(unconf);
- /* We need to handle only case 1: probable callback update */
if (conf && same_verf(&conf->cl_verifier, &clverifier)) {
copy_clid(new, conf);
gen_confirm(new, nn);
