Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 20 Jan 2021 16:25:57 -0500
From:   "J. Bruce Fields" <bfields@redhat.com>
To:     Chuck Lever <chuck.lever@oracle.com>
Cc:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 2/8] nfsd: simplify process_lock
Message-ID: <20210120212557.GA167212@pick.fieldses.org>
References: <1611095729-31104-1-git-send-email-bfields@redhat.com>
 <1611095729-31104-3-git-send-email-bfields@redhat.com>
 <B0EA0C57-443B-4FB5-8E21-4CBA082EF8A6@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <B0EA0C57-443B-4FB5-8E21-4CBA082EF8A6@oracle.com>
Precedence: bulk

On Wed, Jan 20, 2021 at 09:01:53PM +0000, Chuck Lever wrote:
> 
> 
> > On Jan 19, 2021, at 5:35 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> > 
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > Similarly, this STALE_CLIENTID check is already handled inside
> > preprocess_confirmed_seqid_op().
> 
> I can't confirm this claim. Where is clid->cl_boot checked?


nfs4_preprocess_confirmed_seqid_op()->
	nfs4_preprocess_seqid_op()->
		nfsd4_lookup_stateid()->
			set_client()->
				STALE_CLIENTID()

> Did you mean nfs4_preprocess_confirmed_seqid_op() here?

Yeah.  But maybe it'd be better just to include that whole call stack in
the changelog.

--b.

> 
> 
> > (This may cause it to return a different error in some cases where
> > there are multiple things wrong; pynfs test SEQ10 regressed on this
> > commit because of that, but I think that's the test's fault, and I've
> > fixed it separately.)
> > 
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> > fs/nfsd/nfs4state.c | 4 ----
> > 1 file changed, 4 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index f9f89229dba6..7ea63d7cec4d 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -6697,10 +6697,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > 				&cstate->session->se_client->cl_clientid,
> > 				sizeof(clientid_t));
> > 
> > -		status = nfserr_stale_clientid;
> > -		if (STALE_CLIENTID(&lock->lk_new_clientid, nn))
> > -			goto out;
> > -
> > 		/* validate and update open stateid and open seqid */
> > 		status = nfs4_preprocess_confirmed_seqid_op(cstate,
> > 				        lock->lk_new_open_seqid,
> > -- 
> > 2.29.2
> > 
> 
> --
> Chuck Lever
> 
> 
>