Re: [PATCH] Revert "nfsd4: distinguish expired from stale stateids"

[Bian Naimeng <biannm@cn.fujitsu.com>]

J. Bruce Fields wrote:
> On Wed, Sep 22, 2010 at 03:39:33PM -0400, J. Bruce Fields wrote:
>> On Tue, Aug 03, 2010 at 05:12:06PM +0800, Bian Naimeng wrote:
>>>   As i see, this question is difficult to slove. Freebsd looks like nerver return 
>>>   the NFSERR_EXPIRED error.  And solaris more than happy to return NFSERR_EXPIRED 
>>>   rather than NFSERR_BAD_STATEID when the stateid is not found at server.
>>>
>>>   I mean it's difficult to distinguish expired_stateid and bad_stateid(they are not
>>>   exist at server), so maybe we have not a exactly solution to solve it.
>>>
>>>   Maybe we should choose which error we are more needed, NFSERR_EXPIRED  or NFSERR_BAD_STATEID?
>> Yes, that would probably be simplest.
> 
> So, like this?

  Hi bruce,

     I'm sorry to reply it so later, it looks good to me. ^_^

  Best Regards
    Bian

> 
> --b.
> 
> commit 3d5bdf44f73f2e4136d27fe7bd45b9ca6674e590
> Author: J. Bruce Fields <bfields@redhat.com>
> Date:   Sat Oct 2 18:42:39 2010 -0400
> 
>     nfsd4: return expired on unfound stateid's
>     
>     Commit 78155ed75f470710f2aecb3e75e3d97107ba8374 "nfsd4: distinguish
>     expired from stale stateids" attempted to distinguish expired and stale
>     stateid's using time information that may not have been completely
>     reliable, so I reverted it.
>     
>     That was throwing out the baby with the bathwater; we still do want to
>     return expired, but let's do that using the simpler approach of just
>     assuming any stateid is expired if it looks like it was given out by the
>     current server instance, but we can't find it any more.
>     
>     This may help clients that are recovering from network partitions.
>     
>     Reported-by: Bian Naimeng <biannm@cn.fujitsu.com>
>     Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 596702e..02c23b7 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -3046,7 +3046,11 @@ nfs4_preprocess_stateid_op(struct nfsd4_compound_state *cstate,
>  	if (STALE_STATEID(stateid)) 
>  		goto out;
>  
> -	status = nfserr_bad_stateid;
> +	/*
> +	 * We assume that any stateid that has the current boot time,
> +	 * but that we can't find, is expired:
> +	 */
> +	status = nfserr_expired;
>  	if (is_delegation_stateid(stateid)) {
>  		dp = find_delegation_stateid(ino, stateid);
>  		if (!dp)
> @@ -3066,6 +3070,7 @@ nfs4_preprocess_stateid_op(struct nfsd4_compound_state *cstate,
>  		stp = find_stateid(stateid, flags);
>  		if (!stp)
>  			goto out;
> +		status = nfserr_bad_stateid;
>  		if (nfs4_check_fh(current_fh, stp))
>  			goto out;
>  		if (!stp->st_stateowner->so_confirmed)
> @@ -3140,8 +3145,9 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
>  		 * a replayed close:
>  		 */
>  		sop = search_close_lru(stateid->si_stateownerid, flags);
> +		/* It's not stale; let's assume it's expired: */
>  		if (sop == NULL)
> -			return nfserr_bad_stateid;
> +			return nfserr_expired;
>  		*sopp = sop;
>  		goto check_replay;
>  	}
> @@ -3406,6 +3412,7 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	status = nfserr_bad_stateid;
>  	if (!is_delegation_stateid(stateid))
>  		goto out;
> +	status = nfserr_expired;
>  	dp = find_delegation_stateid(inode, stateid);
>  	if (!dp)
>  		goto out;
> 
> 
> 

-- 
Regards
Bian Naimeng