Re: [PATCH 1/1 v2] nfs41: pass state recovery error back to caller

[Benny Halevy <bhalevy@panasas.com>]

On Sep. 02, 2009, 20:52 +0300, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
> On Wed, 2009-09-02 at 10:48 +0300, Benny Halevy wrote:
>> Currently the error returned from create_session
>> is ignored by nfs4_check_client_ready and mis-translated to
>> -EPROTONOSUPPORT if the client has a session.
>> Record the error returned from create_session to the state manager
>> in cl_cons_state via nfs_mark_client_ready and pass it upstream
>> in nfs4_recover_expired_lease.
>>
>> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
>> ---
> 
> Firstly, if you're out to save 4 bytes by sharing storage with an object
> of an entirely different type, then please use an explicit union. Then
> use a special state NFS4CLNT_LEASE_RECLAIM_FAILED in order to clearly
> label what is being stored in that union.
> 

OK.  Just to make sure, will it be acceptable by you to add a field
to struct nfs_client to explicitly keep this status or would you prefer to
save these 4 bytes using the union and extra state?

> Secondly, I'd say that it is more natural to share storage with the
> client id, cl_ex_clid, rather than using the lease time. The latter is
> read via an entirely separate RPC call _after_ you are done establishing
> the lease and the first session.
> 

I (ab?)used cl_lease_time for this reason as nobody cares about its
value at the session establishment phase.

(cl_ex_clid is defined under #ifdef CONFIG_NFS_V4_1
so using it for this purpose will be cumbersome...)

Benny