[pnfs] [PATCH 01/13] removed non NFSv41 spec ops definitions added missing ops

[Benny Halevy]

Hi Tigran, thanks for the revised patches!
One general comment - it'd be easier if you send each patch
separately as a plain-text reply to the "cover" message
(that describes the patch set as a whole).  This way the
patches are easier to refer to as well to apply.

Now, a minor comment to patch #1:

> diff --git a/epan/dissectors/packet-nfs.h b/epan/dissectors/packet-nfs.h
> index b569341..f72ae57 100644
> --- a/epan/dissectors/packet-nfs.h
> +++ b/epan/dissectors/packet-nfs.h
> @@ -105,19 +105,25 @@
>  #define NFS4_OP_WRITE						38
>  #define NFS4_OP_RELEASE_LOCKOWNER		39
>  /* Minor version 1 */
> -#define NFS4_OP_EXCHANGE_ID       42
> -#define NFS4_OP_CREATE_SESSION    43
> -#define NFS4_OP_DESTROY_SESSION   44
> -#define NFS4_OP_GETDEVINFO        47
> -#define NFS4_OP_GETDEVLIST		  48
> -#define NFS4_OP_LAYOUTCOMMIT         49
> -#define NFS4_OP_LAYOUTGET	    	 50
> -#define NFS4_OP_LAYOUTRETURN		 51
> -#define NFS4_OP_SEQUENCE             53
> -#define NFS4_OP_NOTIFYDS             60
> -#define NFS4_OP_PNFS_CREATE          61
> -#define NFS4_OP_PNFS_WRITE           62
> -#define NFS4_OP_PNFS_READ            63
> +#define NFS4_OP_BACKCHANNEL_CTL       40
> +#define NFS4_OP_BIND_CONN_TO_SESSION  41
> +#define NFS4_OP_EXCHANGE_ID           42
> +#define NFS4_OP_CREATE_SESSION        43
> +#define NFS4_OP_DESTROY_SESSION       44
> +#define NFS4_OP_FREE_STATEID          45
> +#define NFS4_OP_GET_DIR_DELEGATION    46
> +#define NFS4_OP_GETDEVINFO            47
> +#define NFS4_OP_GETDEVLIST            48
> +#define NFS4_OP_LAYOUTCOMMIT          49
> +#define NFS4_OP_LAYOUTGET             50
> +#define NFS4_OP_LAYOUTRETURN          51
> +#define NFS4_OP_SECINFO_NO_NAME       52
> +#define NFS4_OP_SEQUENCE              53
> +#define NFS4_OP_SET_SSV               54
> +#define NFS4_OP_TEST_STATEID          55
> +#define NFS4_OP_WANT_DELEGATION       56
> +#define NFS4_OP_DESTROY_CLIENTID      57
> +#define NFS4_OP_RECLAIM_COMPLETE      58

Mixing cosmetic clean-ups with actual changes 
in the same patch is a pretty bad idea since it
adds unwanted "noise" that needs to be filtered out
when the patch is being reviewed, as well as when
the patch is examined and revised during a git bisect
(or git blame) process in case a bug is found and one
wants to track down the exact patch that may have
introduced the bug.

Ideally, the part of this patch that only changes
whitespace should be separated into a patch of its own

Benny

Tigran Mkrtchyan wrote:
> Hi Bruce,
> 
> 
> I guess I have addressed most of your complains.
> 
> 
> Regards,
> 	Tigran.
> 
> J. Bruce Fields wrote:
>> On Fri, Jun 06, 2008 at 10:53:16AM +0300, Benny Halevy wrote:
>>> Yup.  And it doesn't really belong in wireshark either :)
>>> (The "If session is ..."  part)
>>> Also, a minor nit: multi-line comments are supposed to have a
>>> space before the '*'s so they all align on the same column...
>> Tigran, will you have time to fix up these various small pieces and
>> resend the fixed patches?
>>
>> If not, then I'll get to it eventually--but that could be a long time.
>>
>> --b.
> 


-- 
Benny Halevy
Software Architect
Tel/Fax: +972-3-647-8340
Mobile: +972-54-802-8340
bhalevy at panasas.com
 
Panasas, Inc.
The Leader in Parallel Storage
www.panasas.com