[PATCH] Don't convert NFS4ERR_RESOURCE to EREMOTEIO

[PATCH] Don't convert NFS4ERR_RESOURCE to EREMOTEIO

[Benjamin Coddington]

Fixes a test in nfs4_proc_setclientid_confirm() which allows the client 
to retry an operation when the server returns NFS4ERR_RESOURCE, instead 
of returning EREMOTEIO to the user.

---
fs/nfs/nfs4xdr.c |    1 -
  1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index d1e4c8f..61dda13 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4522,7 +4522,6 @@ static struct {
         { NFS4ERR_SERVERFAULT,  -ESERVERFAULT   },
         { NFS4ERR_BADTYPE,      -EBADTYPE       },
         { NFS4ERR_LOCKED,       -EAGAIN         },
-       { NFS4ERR_RESOURCE,     -EREMOTEIO      },
         { NFS4ERR_SYMLINK,      -ELOOP          },
         { NFS4ERR_OP_ILLEGAL,   -EOPNOTSUPP     },
         { NFS4ERR_DEADLOCK,     -EDEADLK        },

Re: [PATCH] Don't convert NFS4ERR_RESOURCE to EREMOTEIO

[Trond Myklebust]

On Wed, 2009-03-04 at 13:42 -0500, Benjamin Coddington wrote:
> Fixes a test in nfs4_proc_setclientid_confirm() which allows the client 
> to retry an operation when the server returns NFS4ERR_RESOURCE, instead 
> of returning EREMOTEIO to the user.
> 
> ---
> fs/nfs/nfs4xdr.c |    1 -
>   1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index d1e4c8f..61dda13 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -4522,7 +4522,6 @@ static struct {
>          { NFS4ERR_SERVERFAULT,  -ESERVERFAULT   },
>          { NFS4ERR_BADTYPE,      -EBADTYPE       },
>          { NFS4ERR_LOCKED,       -EAGAIN         },
> -       { NFS4ERR_RESOURCE,     -EREMOTEIO      },
>          { NFS4ERR_SYMLINK,      -ELOOP          },
>          { NFS4ERR_OP_ILLEGAL,   -EOPNOTSUPP     },
>          { NFS4ERR_DEADLOCK,     -EDEADLK        },

Big NACK.

Firstly, if you want to return the unmapped NFS4ERR_RESOURCE to
setclientid, then you should turn off the mapping of the error in
decode_setclientid instead of breaking all the other cases.

Secondly. The definition of NFS4ERR_RESOURCE is:
                For the processing of the COMPOUND procedure, the server
                may exhaust available resources and can not continue
                processing operations within the COMPOUND procedure.
                This error will be returned from the server in those
                instances of resource exhaustion related to the
                processing of the COMPOUND procedure.

There is nothing there that states this is a temporary condition, and
that the client should retry (which is why NFSv4.1 gets rid of it
altogether). If servers want the client to retry, they should be using
NFS4ERR_DELAY.

  Trond

Re: [PATCH] Don't convert NFS4ERR_RESOURCE to EREMOTEIO

[J. Bruce Fields]

On Wed, Mar 04, 2009 at 01:42:30PM -0500, Benjamin Coddington wrote:
> Fixes a test in nfs4_proc_setclientid_confirm() which allows the client  
> to retry an operation when the server returns NFS4ERR_RESOURCE, instead  
> of returning EREMOTEIO to the user.

I remember being confused as to whether NFS4ERR_RESOURCE is transitory
(hence worth being retried) or permanent (in which case retrying the
identical compound won't help).  The latter might happen if, for
example, the particular sequence of compounds sent by the client was
just too long or complicated for the server to handle.

But rfc3530 doesn't explicitly limit NFS4ERR_RESOURCE to that case, and
it appears that other clients:

	http://www.nfsv4.org/nfsv4-wg-archive-feb-03-feb-05/0747.html

retry.  That discussion isn't conclusive, though.  Hm, and there's
further discussion from Mike Eisler here:

	http://www.nfsv4-editor.org/cgi-bin/roundup/nfsv4/file7/comp-res.txt

which suggests it was intended as a permanent error.

What was the specific case where you hit this?

--b.

>
> ---
> fs/nfs/nfs4xdr.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index d1e4c8f..61dda13 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -4522,7 +4522,6 @@ static struct {
>         { NFS4ERR_SERVERFAULT,  -ESERVERFAULT   },
>         { NFS4ERR_BADTYPE,      -EBADTYPE       },
>         { NFS4ERR_LOCKED,       -EAGAIN         },
> -       { NFS4ERR_RESOURCE,     -EREMOTEIO      },
>         { NFS4ERR_SYMLINK,      -ELOOP          },
>         { NFS4ERR_OP_ILLEGAL,   -EOPNOTSUPP     },
>         { NFS4ERR_DEADLOCK,     -EDEADLK        },
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Don't convert NFS4ERR_RESOURCE to EREMOTEIO

[Benjamin Coddington]

We have webserver clients on an AIX 5300-08 server and run into brief 
periods where the server returns NFS4ERR_RESOURCE on almost every 
operation.  The webservers don't handle EREMOTEIO very well, and it 
would be preferable for us to retry.

It appeared that the retry, and no seqid increment was already set up, 
but never used because of the conversion.

Ben

J. Bruce Fields wrote:
> On Wed, Mar 04, 2009 at 01:42:30PM -0500, Benjamin Coddington wrote:
>> Fixes a test in nfs4_proc_setclientid_confirm() which allows the client  
>> to retry an operation when the server returns NFS4ERR_RESOURCE, instead  
>> of returning EREMOTEIO to the user.
> 
> I remember being confused as to whether NFS4ERR_RESOURCE is transitory
> (hence worth being retried) or permanent (in which case retrying the
> identical compound won't help).  The latter might happen if, for
> example, the particular sequence of compounds sent by the client was
> just too long or complicated for the server to handle.
> 
> But rfc3530 doesn't explicitly limit NFS4ERR_RESOURCE to that case, and
> it appears that other clients:
> 
> 	http://www.nfsv4.org/nfsv4-wg-archive-feb-03-feb-05/0747.html
> 
> retry.  That discussion isn't conclusive, though.  Hm, and there's
> further discussion from Mike Eisler here:
> 
> 	http://www.nfsv4-editor.org/cgi-bin/roundup/nfsv4/file7/comp-res.txt
> 
> which suggests it was intended as a permanent error.
> 
> What was the specific case where you hit this?
> 
> --b.
> 
>> ---
>> fs/nfs/nfs4xdr.c |    1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>> index d1e4c8f..61dda13 100644
>> --- a/fs/nfs/nfs4xdr.c
>> +++ b/fs/nfs/nfs4xdr.c
>> @@ -4522,7 +4522,6 @@ static struct {
>>         { NFS4ERR_SERVERFAULT,  -ESERVERFAULT   },
>>         { NFS4ERR_BADTYPE,      -EBADTYPE       },
>>         { NFS4ERR_LOCKED,       -EAGAIN         },
>> -       { NFS4ERR_RESOURCE,     -EREMOTEIO      },
>>         { NFS4ERR_SYMLINK,      -ELOOP          },
>>         { NFS4ERR_OP_ILLEGAL,   -EOPNOTSUPP     },
>>         { NFS4ERR_DEADLOCK,     -EDEADLK        },
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] Don't convert NFS4ERR_RESOURCE to EREMOTEIO

[J. Bruce Fields]

On Wed, Mar 04, 2009 at 02:25:27PM -0500, Benjamin Coddington wrote:
> We have webserver clients on an AIX 5300-08 server and run into brief  
> periods where the server returns NFS4ERR_RESOURCE on almost every  
> operation.  The webservers don't handle EREMOTEIO very well, and it  
> would be preferable for us to retry.

I think that's an AIX bug--they should probably be returning
NFS4ERR_DELAY--but rfc 3530 wasn't as clear about this as it should have
been.

--b.

>
> It appeared that the retry, and no seqid increment was already set up,  
> but never used because of the conversion.
>
> Ben
>
> J. Bruce Fields wrote:
>> On Wed, Mar 04, 2009 at 01:42:30PM -0500, Benjamin Coddington wrote:
>>> Fixes a test in nfs4_proc_setclientid_confirm() which allows the 
>>> client  to retry an operation when the server returns 
>>> NFS4ERR_RESOURCE, instead  of returning EREMOTEIO to the user.
>>
>> I remember being confused as to whether NFS4ERR_RESOURCE is transitory
>> (hence worth being retried) or permanent (in which case retrying the
>> identical compound won't help).  The latter might happen if, for
>> example, the particular sequence of compounds sent by the client was
>> just too long or complicated for the server to handle.
>>
>> But rfc3530 doesn't explicitly limit NFS4ERR_RESOURCE to that case, and
>> it appears that other clients:
>>
>> 	http://www.nfsv4.org/nfsv4-wg-archive-feb-03-feb-05/0747.html
>>
>> retry.  That discussion isn't conclusive, though.  Hm, and there's
>> further discussion from Mike Eisler here:
>>
>> 	http://www.nfsv4-editor.org/cgi-bin/roundup/nfsv4/file7/comp-res.txt
>>
>> which suggests it was intended as a permanent error.
>>
>> What was the specific case where you hit this?
>>
>> --b.
>>
>>> ---
>>> fs/nfs/nfs4xdr.c |    1 -
>>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>>> index d1e4c8f..61dda13 100644
>>> --- a/fs/nfs/nfs4xdr.c
>>> +++ b/fs/nfs/nfs4xdr.c
>>> @@ -4522,7 +4522,6 @@ static struct {
>>>         { NFS4ERR_SERVERFAULT,  -ESERVERFAULT   },
>>>         { NFS4ERR_BADTYPE,      -EBADTYPE       },
>>>         { NFS4ERR_LOCKED,       -EAGAIN         },
>>> -       { NFS4ERR_RESOURCE,     -EREMOTEIO      },
>>>         { NFS4ERR_SYMLINK,      -ELOOP          },
>>>         { NFS4ERR_OP_ILLEGAL,   -EOPNOTSUPP     },
>>>         { NFS4ERR_DEADLOCK,     -EDEADLK        },
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

Re: [PATCH] Don't convert NFS4ERR_RESOURCE to EREMOTEIO

[Trond Myklebust]

On Wed, 2009-03-04 at 14:01 -0500, J. Bruce Fields wrote:
> On Wed, Mar 04, 2009 at 01:42:30PM -0500, Benjamin Coddington wrote:
> > Fixes a test in nfs4_proc_setclientid_confirm() which allows the client  
> > to retry an operation when the server returns NFS4ERR_RESOURCE, instead  
> > of returning EREMOTEIO to the user.
> 
> I remember being confused as to whether NFS4ERR_RESOURCE is transitory
> (hence worth being retried) or permanent (in which case retrying the
> identical compound won't help).  The latter might happen if, for
> example, the particular sequence of compounds sent by the client was
> just too long or complicated for the server to handle.
> 
> But rfc3530 doesn't explicitly limit NFS4ERR_RESOURCE to that case, and
> it appears that other clients:
> 
> 	http://www.nfsv4.org/nfsv4-wg-archive-feb-03-feb-05/0747.html
> 
> retry.  That discussion isn't conclusive, though.  Hm, and there's
> further discussion from Mike Eisler here:
> 
> 	http://www.nfsv4-editor.org/cgi-bin/roundup/nfsv4/file7/comp-res.txt
> 
> which suggests it was intended as a permanent error.
>
> What was the specific case where you hit this?

He says it was in SETCLIENTID_CONFIRM.

AIX used to abuse the NFS4ERR_RESOURCE error in order to delay clients
while the server was doing some preparations for state recovery. They
argues that they couldn't use NFS4ERR_DELAY since it isn't listed as a
legal error for SETCLIENTID_CONFIRM in RFC3530, and so they picked
NFS4ERR_RESOURCE. That would be a server bug...

Trond

Re: [PATCH] Don't convert NFS4ERR_RESOURCE to EREMOTEIO

[J. Bruce Fields]

On Wed, Mar 04, 2009 at 02:30:10PM -0500, Trond Myklebust wrote:
> On Wed, 2009-03-04 at 14:01 -0500, J. Bruce Fields wrote:
> > On Wed, Mar 04, 2009 at 01:42:30PM -0500, Benjamin Coddington wrote:
> > > Fixes a test in nfs4_proc_setclientid_confirm() which allows the client  
> > > to retry an operation when the server returns NFS4ERR_RESOURCE, instead  
> > > of returning EREMOTEIO to the user.
> > 
> > I remember being confused as to whether NFS4ERR_RESOURCE is transitory
> > (hence worth being retried) or permanent (in which case retrying the
> > identical compound won't help).  The latter might happen if, for
> > example, the particular sequence of compounds sent by the client was
> > just too long or complicated for the server to handle.
> > 
> > But rfc3530 doesn't explicitly limit NFS4ERR_RESOURCE to that case, and
> > it appears that other clients:
> > 
> > 	http://www.nfsv4.org/nfsv4-wg-archive-feb-03-feb-05/0747.html
> > 
> > retry.  That discussion isn't conclusive, though.  Hm, and there's
> > further discussion from Mike Eisler here:
> > 
> > 	http://www.nfsv4-editor.org/cgi-bin/roundup/nfsv4/file7/comp-res.txt
> > 
> > which suggests it was intended as a permanent error.
> >
> > What was the specific case where you hit this?
> 
> He says it was in SETCLIENTID_CONFIRM.
> 
> AIX used to abuse the NFS4ERR_RESOURCE error in order to delay clients
> while the server was doing some preparations for state recovery. They
> argues that they couldn't use NFS4ERR_DELAY since it isn't listed as a
> legal error for SETCLIENTID_CONFIRM in RFC3530,

Oh, good grief, I'd forgotten that story.  OK!--b.

> and so they picked
> NFS4ERR_RESOURCE. That would be a server bug...

Re: [PATCH] Don't convert NFS4ERR_RESOURCE to EREMOTEIO

[Benjamin Coddington]

In that case, maybe the unused test should be cleaned up to avoid 
fooling any other administrators.

Shall I send along a patch removing the test in setclientid_confirm? 
(Its also used in increment_seqid)

Ben

J. Bruce Fields wrote:
> On Wed, Mar 04, 2009 at 02:30:10PM -0500, Trond Myklebust wrote:
>> On Wed, 2009-03-04 at 14:01 -0500, J. Bruce Fields wrote:
>>> On Wed, Mar 04, 2009 at 01:42:30PM -0500, Benjamin Coddington wrote:
>>>> Fixes a test in nfs4_proc_setclientid_confirm() which allows the client  
>>>> to retry an operation when the server returns NFS4ERR_RESOURCE, instead  
>>>> of returning EREMOTEIO to the user.
>>> I remember being confused as to whether NFS4ERR_RESOURCE is transitory
>>> (hence worth being retried) or permanent (in which case retrying the
>>> identical compound won't help).  The latter might happen if, for
>>> example, the particular sequence of compounds sent by the client was
>>> just too long or complicated for the server to handle.
>>>
>>> But rfc3530 doesn't explicitly limit NFS4ERR_RESOURCE to that case, and
>>> it appears that other clients:
>>>
>>> 	http://www.nfsv4.org/nfsv4-wg-archive-feb-03-feb-05/0747.html
>>>
>>> retry.  That discussion isn't conclusive, though.  Hm, and there's
>>> further discussion from Mike Eisler here:
>>>
>>> 	http://www.nfsv4-editor.org/cgi-bin/roundup/nfsv4/file7/comp-res.txt
>>>
>>> which suggests it was intended as a permanent error.
>>>
>>> What was the specific case where you hit this?
>> He says it was in SETCLIENTID_CONFIRM.
>>
>> AIX used to abuse the NFS4ERR_RESOURCE error in order to delay clients
>> while the server was doing some preparations for state recovery. They
>> argues that they couldn't use NFS4ERR_DELAY since it isn't listed as a
>> legal error for SETCLIENTID_CONFIRM in RFC3530,
> 
> Oh, good grief, I'd forgotten that story.  OK!--b.
> 
>> and so they picked
>> NFS4ERR_RESOURCE. That would be a server bug...
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>