From: J. Bruce Fields <[email protected]>
This reverts commit 78155ed75f470710f2aecb3e75e3d97107ba8374.
We're depending here on the boot time that we use to generate the
stateid being monotonic, but get_seconds() is not necessarily.
We still depend at least on boot_time being different every time, but
that is a safer bet.
We have a few reports of errors that might be explained by this problem,
though we haven't been able to confirm any of them.
But the minor gain of distinguishing expired from stale errors seems not
worth the risk.
Conflicts:
fs/nfsd/nfs4state.c
Cc: Bian Naimeng <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 56 ++++++++++-----------------------------------------
1 files changed, 11 insertions(+), 45 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 296eded..12f7109 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -190,7 +190,7 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_stateid *stp, struct svc_f
dp->dl_vfs_file = stp->st_vfs_file;
dp->dl_type = type;
dp->dl_ident = cb->cb_ident;
- dp->dl_stateid.si_boot = get_seconds();
+ dp->dl_stateid.si_boot = boot_time;
dp->dl_stateid.si_stateownerid = current_delegid++;
dp->dl_stateid.si_fileid = 0;
dp->dl_stateid.si_generation = 0;
@@ -1884,7 +1884,7 @@ init_stateid(struct nfs4_stateid *stp, struct nfs4_file *fp, struct nfsd4_open *
stp->st_stateowner = sop;
get_nfs4_file(fp);
stp->st_file = fp;
- stp->st_stateid.si_boot = get_seconds();
+ stp->st_stateid.si_boot = boot_time;
stp->st_stateid.si_stateownerid = sop->so_id;
stp->st_stateid.si_fileid = fp->fi_id;
stp->st_stateid.si_generation = 0;
@@ -2733,39 +2733,11 @@ nfs4_check_fh(struct svc_fh *fhp, struct nfs4_stateid *stp)
static int
STALE_STATEID(stateid_t *stateid)
{
- if (time_after((unsigned long)boot_time,
- (unsigned long)stateid->si_boot)) {
- dprintk("NFSD: stale stateid " STATEID_FMT "!\n",
- STATEID_VAL(stateid));
- return 1;
- }
- return 0;
-}
-
-static int
-EXPIRED_STATEID(stateid_t *stateid)
-{
- if (time_before((unsigned long)boot_time,
- ((unsigned long)stateid->si_boot)) &&
- time_before((unsigned long)(stateid->si_boot + nfsd4_lease), get_seconds())) {
- dprintk("NFSD: expired stateid " STATEID_FMT "!\n",
- STATEID_VAL(stateid));
- return 1;
- }
- return 0;
-}
-
-static __be32
-stateid_error_map(stateid_t *stateid)
-{
- if (STALE_STATEID(stateid))
- return nfserr_stale_stateid;
- if (EXPIRED_STATEID(stateid))
- return nfserr_expired;
-
- dprintk("NFSD: bad stateid " STATEID_FMT "!\n",
+ if (stateid->si_boot == boot_time)
+ return 0;
+ dprintk("NFSD: stale stateid " STATEID_FMT "!\n",
STATEID_VAL(stateid));
- return nfserr_bad_stateid;
+ return 1;
}
static inline int
@@ -2889,10 +2861,8 @@ nfs4_preprocess_stateid_op(struct nfsd4_compound_state *cstate,
status = nfserr_bad_stateid;
if (is_delegation_stateid(stateid)) {
dp = find_delegation_stateid(ino, stateid);
- if (!dp) {
- status = stateid_error_map(stateid);
+ if (!dp)
goto out;
- }
status = check_stateid_generation(stateid, &dp->dl_stateid,
flags);
if (status)
@@ -2905,10 +2875,8 @@ nfs4_preprocess_stateid_op(struct nfsd4_compound_state *cstate,
*filpp = dp->dl_vfs_file;
} else { /* open or lock stateid */
stp = find_stateid(stateid, flags);
- if (!stp) {
- status = stateid_error_map(stateid);
+ if (!stp)
goto out;
- }
if (nfs4_check_fh(current_fh, stp))
goto out;
if (!stp->st_stateowner->so_confirmed)
@@ -2980,7 +2948,7 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
*/
sop = search_close_lru(stateid->si_stateownerid, flags);
if (sop == NULL)
- return stateid_error_map(stateid);
+ return nfserr_bad_stateid;
*sopp = sop;
goto check_replay;
}
@@ -3247,10 +3215,8 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (!is_delegation_stateid(stateid))
goto out;
dp = find_delegation_stateid(inode, stateid);
- if (!dp) {
- status = stateid_error_map(stateid);
+ if (!dp)
goto out;
- }
status = check_stateid_generation(stateid, &dp->dl_stateid, flags);
if (status)
goto out;
@@ -3476,7 +3442,7 @@ alloc_init_lock_stateid(struct nfs4_stateowner *sop, struct nfs4_file *fp, struc
stp->st_stateowner = sop;
get_nfs4_file(fp);
stp->st_file = fp;
- stp->st_stateid.si_boot = get_seconds();
+ stp->st_stateid.si_boot = boot_time;
stp->st_stateid.si_stateownerid = sop->so_id;
stp->st_stateid.si_fileid = fp->fi_id;
stp->st_stateid.si_generation = 0;
--
1.7.0.4
On Mon, Aug 02, 2010 at 11:35:01AM +0800, Bian Naimeng wrote:
> > From: J. Bruce Fields <[email protected]>
> >
> > This reverts commit 78155ed75f470710f2aecb3e75e3d97107ba8374.
> >
> > We're depending here on the boot time that we use to generate the
> > stateid being monotonic, but get_seconds() is not necessarily.
> >
> > We still depend at least on boot_time being different every time, but
> > that is a safer bet.
> >
> > We have a few reports of errors that might be explained by this problem,
> > though we haven't been able to confirm any of them.
> >
> > But the minor gain of distinguishing expired from stale errors seems not
> > worth the risk.
> >
>
> Hi bruce, if remove this patch, some my test will fail. So what's your opinion
> for those test case.
>
> STEP1: open the file, and get a open stateid (STATEID).
> STEP2: shutdown the network between client and server
> STEP3: keep the network partition lease_time(90s by default) seconds
> STEP4: recovery network
> STEP5: do some IO operation, such as LOCK.
>
> If i use the patch 78155ed75f470710f2aecb3e75e3d97107ba8374, this case will OK
> at STEP5, however, it's will fail when remove this patch.
How does it fail, exactly?
>
> So i think it's no good for the network recovery, what do you think about it,
> or give me some suggestions, thanks very much.
The theoretical problem with the patch is that time changes could cause
the server to return spurious errors when the client hands it state that
should still be good.
We might be able to solve that by using a different time source?
--b.
> On Mon, Aug 02, 2010 at 11:35:01AM +0800, Bian Naimeng wrote:
>>> From: J. Bruce Fields <[email protected]>
>>>
... snip ...
>> If i use the patch 78155ed75f470710f2aecb3e75e3d97107ba8374, this case will OK
>> at STEP5, however, it's will fail when remove this patch.
>
> How does it fail, exactly?
>
>> So i think it's no good for the network recovery, what do you think about it,
>> or give me some suggestions, thanks very much.
>
> The theoretical problem with the patch is that time changes could cause
> the server to return spurious errors when the client hands it state that
> should still be good.
>
> We might be able to solve that by using a different time source?
>
Hi bruce, Have you got a idea to solve it?
--
Regards
Bian Naimeng
> From: J. Bruce Fields <[email protected]>
>
> This reverts commit 78155ed75f470710f2aecb3e75e3d97107ba8374.
>
> We're depending here on the boot time that we use to generate the
> stateid being monotonic, but get_seconds() is not necessarily.
>
> We still depend at least on boot_time being different every time, but
> that is a safer bet.
>
> We have a few reports of errors that might be explained by this problem,
> though we haven't been able to confirm any of them.
>
> But the minor gain of distinguishing expired from stale errors seems not
> worth the risk.
>
Hi bruce, if remove this patch, some my test will fail. So what's your opinion
for those test case.
STEP1: open the file, and get a open stateid (STATEID).
STEP2: shutdown the network between client and server
STEP3: keep the network partition lease_time(90s by default) seconds
STEP4: recovery network
STEP5: do some IO operation, such as LOCK.
If i use the patch 78155ed75f470710f2aecb3e75e3d97107ba8374, this case will OK
at STEP5, however, it's will fail when remove this patch.
So i think it's no good for the network recovery, what do you think about it,
or give me some suggestions, thanks very much.
> Conflicts:
>
> fs/nfsd/nfs4state.c
>
> Cc: Bian Naimeng <[email protected]>
> Signed-off-by: J. Bruce Fields <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 56 ++++++++++-----------------------------------------
> 1 files changed, 11 insertions(+), 45 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 296eded..12f7109 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -190,7 +190,7 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_stateid *stp, struct svc_f
> dp->dl_vfs_file = stp->st_vfs_file;
> dp->dl_type = type;
> dp->dl_ident = cb->cb_ident;
> - dp->dl_stateid.si_boot = get_seconds();
> + dp->dl_stateid.si_boot = boot_time;
> dp->dl_stateid.si_stateownerid = current_delegid++;
> dp->dl_stateid.si_fileid = 0;
> dp->dl_stateid.si_generation = 0;
> @@ -1884,7 +1884,7 @@ init_stateid(struct nfs4_stateid *stp, struct nfs4_file *fp, struct nfsd4_open *
> stp->st_stateowner = sop;
> get_nfs4_file(fp);
> stp->st_file = fp;
> - stp->st_stateid.si_boot = get_seconds();
> + stp->st_stateid.si_boot = boot_time;
> stp->st_stateid.si_stateownerid = sop->so_id;
> stp->st_stateid.si_fileid = fp->fi_id;
> stp->st_stateid.si_generation = 0;
> @@ -2733,39 +2733,11 @@ nfs4_check_fh(struct svc_fh *fhp, struct nfs4_stateid *stp)
> static int
> STALE_STATEID(stateid_t *stateid)
> {
> - if (time_after((unsigned long)boot_time,
> - (unsigned long)stateid->si_boot)) {
> - dprintk("NFSD: stale stateid " STATEID_FMT "!\n",
> - STATEID_VAL(stateid));
> - return 1;
> - }
> - return 0;
> -}
> -
> -static int
> -EXPIRED_STATEID(stateid_t *stateid)
> -{
> - if (time_before((unsigned long)boot_time,
> - ((unsigned long)stateid->si_boot)) &&
> - time_before((unsigned long)(stateid->si_boot + nfsd4_lease), get_seconds())) {
> - dprintk("NFSD: expired stateid " STATEID_FMT "!\n",
> - STATEID_VAL(stateid));
> - return 1;
> - }
> - return 0;
> -}
> -
> -static __be32
> -stateid_error_map(stateid_t *stateid)
> -{
> - if (STALE_STATEID(stateid))
> - return nfserr_stale_stateid;
> - if (EXPIRED_STATEID(stateid))
> - return nfserr_expired;
> -
> - dprintk("NFSD: bad stateid " STATEID_FMT "!\n",
> + if (stateid->si_boot == boot_time)
> + return 0;
> + dprintk("NFSD: stale stateid " STATEID_FMT "!\n",
> STATEID_VAL(stateid));
> - return nfserr_bad_stateid;
> + return 1;
> }
>
> static inline int
> @@ -2889,10 +2861,8 @@ nfs4_preprocess_stateid_op(struct nfsd4_compound_state *cstate,
> status = nfserr_bad_stateid;
> if (is_delegation_stateid(stateid)) {
> dp = find_delegation_stateid(ino, stateid);
> - if (!dp) {
> - status = stateid_error_map(stateid);
> + if (!dp)
> goto out;
> - }
> status = check_stateid_generation(stateid, &dp->dl_stateid,
> flags);
> if (status)
> @@ -2905,10 +2875,8 @@ nfs4_preprocess_stateid_op(struct nfsd4_compound_state *cstate,
> *filpp = dp->dl_vfs_file;
> } else { /* open or lock stateid */
> stp = find_stateid(stateid, flags);
> - if (!stp) {
> - status = stateid_error_map(stateid);
> + if (!stp)
> goto out;
> - }
> if (nfs4_check_fh(current_fh, stp))
> goto out;
> if (!stp->st_stateowner->so_confirmed)
> @@ -2980,7 +2948,7 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
> */
> sop = search_close_lru(stateid->si_stateownerid, flags);
> if (sop == NULL)
> - return stateid_error_map(stateid);
> + return nfserr_bad_stateid;
> *sopp = sop;
> goto check_replay;
> }
> @@ -3247,10 +3215,8 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> if (!is_delegation_stateid(stateid))
> goto out;
> dp = find_delegation_stateid(inode, stateid);
> - if (!dp) {
> - status = stateid_error_map(stateid);
> + if (!dp)
> goto out;
> - }
> status = check_stateid_generation(stateid, &dp->dl_stateid, flags);
> if (status)
> goto out;
> @@ -3476,7 +3442,7 @@ alloc_init_lock_stateid(struct nfs4_stateowner *sop, struct nfs4_file *fp, struc
> stp->st_stateowner = sop;
> get_nfs4_file(fp);
> stp->st_file = fp;
> - stp->st_stateid.si_boot = get_seconds();
> + stp->st_stateid.si_boot = boot_time;
> stp->st_stateid.si_stateownerid = sop->so_id;
> stp->st_stateid.si_fileid = fp->fi_id;
> stp->st_stateid.si_generation = 0;
--
Regards
Bian Naimeng
> On Mon, Aug 02, 2010 at 11:35:01AM +0800, Bian Naimeng wrote:
>>> From: J. Bruce Fields <[email protected]>
>>>
>>> This reverts commit 78155ed75f470710f2aecb3e75e3d97107ba8374.
... snip ...
>>>
>> Hi bruce, if remove this patch, some my test will fail. So what's your opinion
>> for those test case.
>>
>> STEP1: open the file, and get a open stateid (STATEID).
>> STEP2: shutdown the network between client and server
>> STEP3: keep the network partition lease_time(90s by default) seconds
>> STEP4: recovery network
>> STEP5: do some IO operation, such as LOCK.
>>
>> If i use the patch 78155ed75f470710f2aecb3e75e3d97107ba8374, this case will OK
>> at STEP5, however, it's will fail when remove this patch.
>
> How does it fail, exactly?
>
My client is linux-2.6.32, server is linux-2.6.35.
At step5, client will use the old open stateid to send lock request, and server
return NFS4ERR_BAD_STATEID because this stateid have be released,
then client's kernel return EIO to userspace.
If i apply this patch, server will return NFS4ERR_EXPIRED at step5, then client
will start recovery procedure.
>> So i think it's no good for the network recovery, what do you think about it,
>> or give me some suggestions, thanks very much.
>
> The theoretical problem with the patch is that time changes could cause
> the server to return spurious errors when the client hands it state that
> should still be good.
>
En... Would you give me a example?
> We might be able to solve that by using a different time source?
>
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?
--
Regards
Bian Naimeng
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.
--b.
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?
--b.
commit 3d5bdf44f73f2e4136d27fe7bd45b9ca6674e590
Author: J. Bruce Fields <[email protected]>
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 <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
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;
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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: J. Bruce Fields <[email protected]>
>
> 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
On Mon, Nov 08, 2010 at 04:52:36PM +0800, Bian Naimeng wrote:
>
>
> 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. ^_^
OK, thanks. This is upstream now.
--b.
>
> Best Regards
> Bian
>
> >
> > --b.
> >
> > commit 3d5bdf44f73f2e4136d27fe7bd45b9ca6674e590
> > Author: J. Bruce Fields <[email protected]>
> > 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 <[email protected]>
> > Signed-off-by: J. Bruce Fields <[email protected]>
> >
> > 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html