RFC5661 obsoletes NFS4ERR_STALE_STATEID in favour of NFS4ERR_BAD_STATEID.
Note that because nfsd encodes the clientid boot time in the stateid, we
can hit this error case in certain scenarios where the Linux client
state management thread exits early, before it has finished recovering
all state.
Reported-by: Idan Kedar <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d5d070fbeb35..dd32f3746e4d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3627,8 +3627,11 @@ static __be32 nfsd4_lookup_stateid(stateid_t *stateid, unsigned char typemask,
return nfserr_bad_stateid;
status = lookup_clientid(&stateid->si_opaque.so_clid, sessions,
nn, &cl);
- if (status == nfserr_stale_clientid)
+ if (status == nfserr_stale_clientid) {
+ if (sessions)
+ return nfserr_bad_stateid;
return nfserr_stale_stateid;
+ }
if (status)
return status;
*s = find_stateid_by_type(cl, stateid, typemask);
--
1.9.0
On Mar 29, 2014, at 15:49, Trond Myklebust <[email protected]> wrote:
>
> On Mar 29, 2014, at 15:34, J. Bruce Fields <[email protected]> wrote:
>
>> On Sat, Mar 29, 2014 at 02:43:39PM -0400, Trond Myklebust wrote:
>>> RFC5661 obsoletes NFS4ERR_RESTOREFH in favour of NFS4ERR_NOFILEHANDLE.
>>
>> Looks right. Any objection to just making this nfserr_restorefh in the
>> 4.0 case as well? Hard to imagine how that could cause a 4.0 client any
>> problem.
>
> You mean make both cases return nfserr_nofilehandle (as per RFC5661), right? So, I agree that most clients should handle that, but the problem is that RFC3530bis does not allow it.
Either way, this is not a performance critical issue. Any time we get into this situation, it is because the client is utterly screwed up in the first place.
The NFS4ERR_STALE_STATEID is the critical one that really needs to be applied...
_________________________________
Trond Myklebust
Linux NFS client maintainer, PrimaryData
[email protected]
On Sat, Mar 29, 2014 at 04:01:15PM -0400, Trond Myklebust wrote:
>
> On Mar 29, 2014, at 15:49, Trond Myklebust <[email protected]> wrote:
>
> >
> > On Mar 29, 2014, at 15:34, J. Bruce Fields <[email protected]> wrote:
> >
> >> On Sat, Mar 29, 2014 at 02:43:39PM -0400, Trond Myklebust wrote:
> >>> RFC5661 obsoletes NFS4ERR_RESTOREFH in favour of NFS4ERR_NOFILEHANDLE.
> >>
> >> Looks right. Any objection to just making this nfserr_restorefh in the
> >> 4.0 case as well? Hard to imagine how that could cause a 4.0 client any
> >> problem.
> >
> > You mean make both cases return nfserr_nofilehandle (as per RFC5661), right? So, I agree that most clients should handle that, but the problem is that RFC3530bis does not allow it.
>
> Either way, this is not a performance critical issue. Any time we get into this situation, it is because the client is utterly screwed up in the first place.
Yeah. I don't really care much, applied.
> The NFS4ERR_STALE_STATEID is the critical one that really needs to be applied...
Already applied, thanks.
--b.
On Sat, Mar 29, 2014 at 02:43:39PM -0400, Trond Myklebust wrote:
> RFC5661 obsoletes NFS4ERR_RESTOREFH in favour of NFS4ERR_NOFILEHANDLE.
Looks right. Any objection to just making this nfserr_restorefh in the
4.0 case as well? Hard to imagine how that could cause a 4.0 client any
problem.
--b.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfsd/nfs4proc.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 82189b208af3..eeee4418d44a 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -536,8 +536,11 @@ static __be32
> nfsd4_restorefh(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> void *arg)
> {
> - if (!cstate->save_fh.fh_dentry)
> + if (!cstate->save_fh.fh_dentry) {
> + if (nfsd4_has_session(cstate))
> + return nfserr_nofilehandle;
> return nfserr_restorefh;
> + }
>
> fh_dup2(&cstate->current_fh, &cstate->save_fh);
> if (HAS_STATE_ID(cstate, SAVED_STATE_ID_FLAG)) {
> --
> 1.9.0
>
On Mar 29, 2014, at 15:34, J. Bruce Fields <[email protected]> wrote:
> On Sat, Mar 29, 2014 at 02:43:39PM -0400, Trond Myklebust wrote:
>> RFC5661 obsoletes NFS4ERR_RESTOREFH in favour of NFS4ERR_NOFILEHANDLE.
>
> Looks right. Any objection to just making this nfserr_restorefh in the
> 4.0 case as well? Hard to imagine how that could cause a 4.0 client any
> problem.
You mean make both cases return nfserr_nofilehandle (as per RFC5661), right? So, I agree that most clients should handle that, but the problem is that RFC3530bis does not allow it.
>
> --b.
>
>>
>> Signed-off-by: Trond Myklebust <[email protected]>
>> ---
>> fs/nfsd/nfs4proc.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>> index 82189b208af3..eeee4418d44a 100644
>> --- a/fs/nfsd/nfs4proc.c
>> +++ b/fs/nfsd/nfs4proc.c
>> @@ -536,8 +536,11 @@ static __be32
>> nfsd4_restorefh(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>> void *arg)
>> {
>> - if (!cstate->save_fh.fh_dentry)
>> + if (!cstate->save_fh.fh_dentry) {
>> + if (nfsd4_has_session(cstate))
>> + return nfserr_nofilehandle;
>> return nfserr_restorefh;
>> + }
>>
>> fh_dup2(&cstate->current_fh, &cstate->save_fh);
>> if (HAS_STATE_ID(cstate, SAVED_STATE_ID_FLAG)) {
>> --
>> 1.9.0
>>
_________________________________
Trond Myklebust
Linux NFS client maintainer, PrimaryData
[email protected]
On Sat, Mar 29, 2014 at 02:43:38PM -0400, Trond Myklebust wrote:
> RFC5661 obsoletes NFS4ERR_STALE_STATEID in favour of NFS4ERR_BAD_STATEID.
>
> Note that because nfsd encodes the clientid boot time in the stateid, we
> can hit this error case in certain scenarios where the Linux client
> state management thread exits early, before it has finished recovering
> all state.
Thanks, applying.
--b.
>
> Reported-by: Idan Kedar <[email protected]>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index d5d070fbeb35..dd32f3746e4d 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -3627,8 +3627,11 @@ static __be32 nfsd4_lookup_stateid(stateid_t *stateid, unsigned char typemask,
> return nfserr_bad_stateid;
> status = lookup_clientid(&stateid->si_opaque.so_clid, sessions,
> nn, &cl);
> - if (status == nfserr_stale_clientid)
> + if (status == nfserr_stale_clientid) {
> + if (sessions)
> + return nfserr_bad_stateid;
> return nfserr_stale_stateid;
> + }
> if (status)
> return status;
> *s = find_stateid_by_type(cl, stateid, typemask);
> --
> 1.9.0
>
RFC5661 obsoletes NFS4ERR_RESTOREFH in favour of NFS4ERR_NOFILEHANDLE.
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4proc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 82189b208af3..eeee4418d44a 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -536,8 +536,11 @@ static __be32
nfsd4_restorefh(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
void *arg)
{
- if (!cstate->save_fh.fh_dentry)
+ if (!cstate->save_fh.fh_dentry) {
+ if (nfsd4_has_session(cstate))
+ return nfserr_nofilehandle;
return nfserr_restorefh;
+ }
fh_dup2(&cstate->current_fh, &cstate->save_fh);
if (HAS_STATE_ID(cstate, SAVED_STATE_ID_FLAG)) {
--
1.9.0