* [PATCH 1/2] nfsd: Don't return NFS4ERR_STALE_STATEID for NFSv4.1+
@ 2014-03-29 18:43 Trond Myklebust
2014-03-29 18:43 ` [PATCH 2/2] nfsd: Don't return NFS4ERR_RESTOREFH " Trond Myklebust
2014-03-29 19:33 ` [PATCH 1/2] nfsd: Don't return NFS4ERR_STALE_STATEID " J. Bruce Fields
0 siblings, 2 replies; 7+ messages in thread
From: Trond Myklebust @ 2014-03-29 18:43 UTC (permalink / raw)
To: bfields; +Cc: Idan Kedar, linux-nfs
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 <idank@primarydata.com>
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/2] nfsd: Don't return NFS4ERR_RESTOREFH for NFSv4.1+
2014-03-29 18:43 [PATCH 1/2] nfsd: Don't return NFS4ERR_STALE_STATEID for NFSv4.1+ Trond Myklebust
@ 2014-03-29 18:43 ` Trond Myklebust
2014-03-29 19:34 ` J. Bruce Fields
2014-03-29 19:33 ` [PATCH 1/2] nfsd: Don't return NFS4ERR_STALE_STATEID " J. Bruce Fields
1 sibling, 1 reply; 7+ messages in thread
From: Trond Myklebust @ 2014-03-29 18:43 UTC (permalink / raw)
To: bfields; +Cc: Idan Kedar, linux-nfs
RFC5661 obsoletes NFS4ERR_RESTOREFH in favour of NFS4ERR_NOFILEHANDLE.
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] nfsd: Don't return NFS4ERR_RESTOREFH for NFSv4.1+
2014-03-29 18:43 ` [PATCH 2/2] nfsd: Don't return NFS4ERR_RESTOREFH " Trond Myklebust
@ 2014-03-29 19:34 ` J. Bruce Fields
2014-03-29 19:49 ` Trond Myklebust
0 siblings, 1 reply; 7+ messages in thread
From: J. Bruce Fields @ 2014-03-29 19:34 UTC (permalink / raw)
To: Trond Myklebust; +Cc: Idan Kedar, linux-nfs
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 <trond.myklebust@primarydata.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] nfsd: Don't return NFS4ERR_RESTOREFH for NFSv4.1+
2014-03-29 19:34 ` J. Bruce Fields
@ 2014-03-29 19:49 ` Trond Myklebust
2014-03-29 20:01 ` Trond Myklebust
0 siblings, 1 reply; 7+ messages in thread
From: Trond Myklebust @ 2014-03-29 19:49 UTC (permalink / raw)
To: Dr Fields James Bruce; +Cc: Idan Kedar, linux-nfs
On Mar 29, 2014, at 15:34, J. Bruce Fields <bfields@fieldses.org> 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 <trond.myklebust@primarydata.com>
>> ---
>> 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
trond.myklebust@primarydata.com
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] nfsd: Don't return NFS4ERR_RESTOREFH for NFSv4.1+
2014-03-29 19:49 ` Trond Myklebust
@ 2014-03-29 20:01 ` Trond Myklebust
2014-03-31 20:52 ` Dr Fields James Bruce
0 siblings, 1 reply; 7+ messages in thread
From: Trond Myklebust @ 2014-03-29 20:01 UTC (permalink / raw)
To: Dr Fields James Bruce; +Cc: Idan Kedar, Linux NFS Mailing List
On Mar 29, 2014, at 15:49, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
>
> On Mar 29, 2014, at 15:34, J. Bruce Fields <bfields@fieldses.org> 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
trond.myklebust@primarydata.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] nfsd: Don't return NFS4ERR_RESTOREFH for NFSv4.1+
2014-03-29 20:01 ` Trond Myklebust
@ 2014-03-31 20:52 ` Dr Fields James Bruce
0 siblings, 0 replies; 7+ messages in thread
From: Dr Fields James Bruce @ 2014-03-31 20:52 UTC (permalink / raw)
To: Trond Myklebust; +Cc: Idan Kedar, Linux NFS Mailing List
On Sat, Mar 29, 2014 at 04:01:15PM -0400, Trond Myklebust wrote:
>
> On Mar 29, 2014, at 15:49, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
>
> >
> > On Mar 29, 2014, at 15:34, J. Bruce Fields <bfields@fieldses.org> 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] nfsd: Don't return NFS4ERR_STALE_STATEID for NFSv4.1+
2014-03-29 18:43 [PATCH 1/2] nfsd: Don't return NFS4ERR_STALE_STATEID for NFSv4.1+ Trond Myklebust
2014-03-29 18:43 ` [PATCH 2/2] nfsd: Don't return NFS4ERR_RESTOREFH " Trond Myklebust
@ 2014-03-29 19:33 ` J. Bruce Fields
1 sibling, 0 replies; 7+ messages in thread
From: J. Bruce Fields @ 2014-03-29 19:33 UTC (permalink / raw)
To: Trond Myklebust; +Cc: Idan Kedar, linux-nfs
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 <idank@primarydata.com>
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-03-31 20:52 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-29 18:43 [PATCH 1/2] nfsd: Don't return NFS4ERR_STALE_STATEID for NFSv4.1+ Trond Myklebust
2014-03-29 18:43 ` [PATCH 2/2] nfsd: Don't return NFS4ERR_RESTOREFH " Trond Myklebust
2014-03-29 19:34 ` J. Bruce Fields
2014-03-29 19:49 ` Trond Myklebust
2014-03-29 20:01 ` Trond Myklebust
2014-03-31 20:52 ` Dr Fields James Bruce
2014-03-29 19:33 ` [PATCH 1/2] nfsd: Don't return NFS4ERR_STALE_STATEID " J. Bruce Fields
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).