Re: [PATCH 0/2 v4] nfs: return nfs4 compound header status on op header decoding error

[Benny Halevy <bhalevy@panasas.com>]

On Jul. 16, 2008, 15:57 +0300, Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
> On Wed, 2008-07-16 at 11:21 +0300, Benny Halevy wrote:
>> On Jul. 16, 2008, 0:57 +0300, Trond Myklebust <trond.myklebust@fys.uio.no> wrote: 
>>> IOW: something like
>>>
>>>
>>> @@static int decode_compound_hdr(
>>> 	p += XDR_QUADLEN(hdr->taglen);
>>> 	READ32(hdr->nops);
>>> +	if (hdr->nops < 1)
>>> +		return nfs4_stat_to_errno(hdr->status);
>>> 	return 0;
>>> }
>>>
>> This certainly provides a shortcut for the nops==0 case.
>> However, it doesn't solve the OP_ILLEGAL case all other
>> cases where xdr_inline_decode failed or opnum != expected,
>> we can easily handle the OP_ILLEGAL case explicitly in
>> decode_op_hdr.  Are you ok with the approach of
>> carrying the hdr.status in xdr->status for decode_op_hdr
>> use, or do you rather prefer to leave things as they are
>> for the invalid cases and explicitly handle only the
>> nops==0 and OP_ILLEGAL cases?
> 
> Why do we need to handle OP_ILLEGAL in the first place? This is the
> client; it isn't supposed to send illegal operations...

Right, but it helps in the development process when dealing with
a broken version of the server or the client to pass a less
generic error (-EOPNOTSUPP) up the stack rather than -EIO.

Benny

> 
> Trond
>