Re: [PATCH 1/2] nfs4.1: add BIND_CONN_TO_SESSION operation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On May 24, 2012, at 1:06 PM, Chuck Lever wrote:

> 
> On May 24, 2012, at 12:49 PM, Adamson, Dros wrote:
> 
>> 
>> On May 24, 2012, at 12:46 PM, Chuck Lever wrote:
>> 
>>> 
>>> On May 24, 2012, at 12:26 PM, Weston Andros Adamson wrote:
>>> 
>>>> This patch adds the BIND_CONN_TO_SESSION operation which is needed for
>>>> upcoming SP4_MACH_CRED work and useful for recovering from broken connections
>>>> without destroying the session.
>>>> 
>>>> Signed-off-by: Weston Andros Adamson <dros@xxxxxxxxxx>
>>>> ---
>>>> fs/nfs/nfs4_fs.h        |    1 +
>>>> fs/nfs/nfs4proc.c       |   54 ++++++++++++++++++++++++++++
>>>> fs/nfs/nfs4xdr.c        |   90 +++++++++++++++++++++++++++++++++++++++++++++++
>>>> include/linux/nfs4.h    |    5 +++
>>>> include/linux/nfs_xdr.h |    6 +++
>>>> 5 files changed, 156 insertions(+), 0 deletions(-)
>>> 
>>> [ ... snipped ... ]
>>> 
>>>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
>>>> index 0987146..80fbbfc6 100644
>>>> --- a/include/linux/nfs4.h
>>>> +++ b/include/linux/nfs4.h
>>>> @@ -69,6 +69,10 @@
>>>> #define NFS4_CDFC4_FORE_OR_BOTH 0x3
>>>> #define NFS4_CDFC4_BACK_OR_BOTH 0x7
>>>> 
>>>> +#define CDFS4_FORE 0x1
>>>> +#define CDFS4_BACK 0x2
>>>> +#define CDFS4_BOTH 0x3
>>> 
>>> A few more nits:
>>> 
>>> I'm looking back at the patch I did last year to add BIND_CONN_TO_SESS for NFSv4.1 migration support... I used NFS4_CDFS4_FORE and so on here, to match the defines in the paragraph above.
>> 
>> Ah, good catch!  I'll change to NFS4_CDFS4_*
>> 
>>> 
>>> In the XDR decoder function, I check the "dir_from_server" field to ensure it is equal to or less than NFS4_CDFS4_BOTH (-EIO if not).
>> 
>> Ok, that can't hurt!  Note I'm already returning EIO in the proc handler if dir != NFS4_CDFS4_BOTH,
> 
> Right, that's an implementation-imposed limit.  proc is the right place for that.  The XDR check is to sanity check servers, and should allow all possible values of that field, and no more.  FWIW.

I'm right there with you :)

-dros

> 
>> but I think it makes sense to add this sanity check.
>> 
>> Thanks!
>> -dros
>> 
>>> 
>>>> +
>>>> #define NFS4_SET_TO_SERVER_TIME	0
>>>> #define NFS4_SET_TO_CLIENT_TIME	1
>>>> 
>>>> @@ -582,6 +586,7 @@ enum {
>>>> 	NFSPROC4_CLNT_SECINFO,
>>>> 
>>>> 	/* nfs41 */
>>>> +	NFSPROC4_CLNT_BIND_CONN_TO_SESSION,
>>>> 	NFSPROC4_CLNT_EXCHANGE_ID,
>>>> 	NFSPROC4_CLNT_CREATE_SESSION,
>>>> 	NFSPROC4_CLNT_DESTROY_SESSION,
>>> 
>>> -- 
>>> Chuck Lever
>>> chuck[dot]lever[at]oracle[dot]com
>>> 
>>> 
>>> 
>>> 
>> 
> 
> -- 
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
> 
> 
> 
> 

Attachment: smime.p7s
Description: S/MIME cryptographic signature


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux