From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: linux-nfs-owner@vger.kernel.org Received: from fieldses.org ([174.143.236.118]:36891 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751239Ab2FEUla (ORCPT ); Tue, 5 Jun 2012 16:41:30 -0400 Date: Tue, 5 Jun 2012 16:41:28 -0400 To: Chuck Lever Cc: bfields@redhat.com, linux-nfs@vger.kernel.org Subject: Re: [PATCH] NFSD: TEST_STATEID should not return NFS4ERR_STALE_STATEID Message-ID: <20120605204128.GC27061@fieldses.org> References: <20120529175556.4472.63375.stgit@lebasque.1015granger.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20120529175556.4472.63375.stgit@lebasque.1015granger.net> From: "J. Bruce Fields" Sender: linux-nfs-owner@vger.kernel.org List-ID: Thanks, applying for 3.6. --b. On Tue, May 29, 2012 at 01:56:37PM -0400, Chuck Lever wrote: > According to RFC 5661, the TEST_STATEID operation is not allowed to > return NFS4ERR_STALE_STATEID. In addition, RFC 5661 says: > > 15.1.16.5. NFS4ERR_STALE_STATEID (Error Code 10023) > > A stateid generated by an earlier server instance was used. This > error is moot in NFSv4.1 because all operations that take a stateid > MUST be preceded by the SEQUENCE operation, and the earlier server > instance is detected by the session infrastructure that supports > SEQUENCE. > > I triggered NFS4ERR_STALE_STATEID while testing the Linux client's > NOGRACE recovery. Bruce suggested an additional test that could be > useful to client developers. > > Lastly, RFC 5661, section 18.48.3 has this: > > o Special stateids are always considered invalid (they result in the > error code NFS4ERR_BAD_STATEID). > > An explicit check is made for those state IDs to avoid printk noise. > > Signed-off-by: Chuck Lever > --- > > fs/nfsd/nfs4state.c | 22 ++++++++++++++++------ > fs/nfsd/state.h | 1 - > 2 files changed, 16 insertions(+), 7 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index 9235cfa..c6d44b4 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -38,6 +38,7 @@ > #include > #include > #include > +#include > #include > #include > #include "xdr4.h" > @@ -3359,18 +3360,26 @@ static __be32 check_stateid_generation(stateid_t *in, stateid_t *ref, > return nfserr_old_stateid; > } > > -__be32 nfs4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid) > +static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid) > { > struct nfs4_stid *s; > struct nfs4_ol_stateid *ols; > __be32 status; > > - if (STALE_STATEID(stateid)) > - return nfserr_stale_stateid; > - > + if (ZERO_STATEID(stateid) || ONE_STATEID(stateid)) > + return nfserr_bad_stateid; > + /* Client debugging aid. */ > + if (!same_clid(&stateid->si_opaque.so_clid, &cl->cl_clientid)) { > + char addr_str[INET6_ADDRSTRLEN]; > + rpc_ntop((struct sockaddr *)&cl->cl_addr, addr_str, > + sizeof(addr_str)); > + pr_warn_ratelimited("NFSD: client %s testing state ID " > + "with incorrect client ID\n", addr_str); > + return nfserr_bad_stateid; > + } > s = find_stateid(cl, stateid); > if (!s) > - return nfserr_stale_stateid; > + return nfserr_bad_stateid; > status = check_stateid_generation(stateid, &s->sc_stateid, 1); > if (status) > return status; > @@ -3489,7 +3498,8 @@ nfsd4_test_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > > nfs4_lock_state(); > list_for_each_entry(stateid, &test_stateid->ts_stateid_list, ts_id_list) > - stateid->ts_id_status = nfs4_validate_stateid(cl, &stateid->ts_id_stateid); > + stateid->ts_id_status = > + nfsd4_validate_stateid(cl, &stateid->ts_id_stateid); > nfs4_unlock_state(); > > return nfs_ok; > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h > index 89ab137..b6bd031 100644 > --- a/fs/nfsd/state.h > +++ b/fs/nfsd/state.h > @@ -476,7 +476,6 @@ extern __be32 nfs4_make_rec_clidname(char *clidname, struct xdr_netobj *clname); > extern int nfs4_client_to_reclaim(const char *name); > extern int nfs4_has_reclaimed_state(const char *name, bool use_exchange_id); > extern void release_session_client(struct nfsd4_session *); > -extern __be32 nfs4_validate_stateid(struct nfs4_client *, stateid_t *); > extern void nfsd4_purge_closed_stateid(struct nfs4_stateowner *); > > /* nfs4recover operations */ >