> On May 11, 2021, at 1:38 PM, Bruce Fields <bfields@xxxxxxxxxxxx> wrote:
>
> 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.
OK, I'll squash this in, and include similar changes to
nfsd4_setclientid_confirm().
> --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);
--
Chuck Lever
