Re: [PATCH 3/3] nfs-utils: Add nonegde mount option

[Bob Bell <b_linuxnfs-Y/+76LoPTq9wBoktGHYdvgC/G2K4zDHf@public.gmane.org>]

On Tue, Jan 15, 2008 at 11:42:54AM -0500, Chuck Lever wrote:
>On Jan 15, 2008, at 11:31 AM, Bob Bell wrote:
>>diff --git a/utils/mount/nfs_mount.h b/utils/mount/nfs_mount.h
>>index 7df8fb2..2deca87 100644
>>--- a/utils/mount/nfs_mount.h
>>+++ b/utils/mount/nfs_mount.h
>>@@ -50,21 +50,22 @@ struct nfs_mount_data {
>>/* bits in the flags field */
>>-#define NFS_MOUNT_SOFT		0x0001	/* 1 */
>>-#define NFS_MOUNT_INTR		0x0002	/* 1 */
>>-#define NFS_MOUNT_SECURE	0x0004	/* 1 */
>>-#define NFS_MOUNT_POSIX		0x0008	/* 1 */
>>-#define NFS_MOUNT_NOCTO		0x0010	/* 1 */
>>-#define NFS_MOUNT_NOAC		0x0020	/* 1 */
>>-#define NFS_MOUNT_TCP		0x0040	/* 2 */
>>-#define NFS_MOUNT_VER3		0x0080	/* 3 */
>>-#define NFS_MOUNT_KERBEROS	0x0100	/* 3 */
>>-#define NFS_MOUNT_NONLM		0x0200	/* 3 */
>>-#define NFS_MOUNT_BROKEN_SUID	0x0400	/* 4 */
>>-#define NFS_MOUNT_NOACL     0x0800  /* 4 */
>>-#define NFS_MOUNT_SECFLAVOUR	0x2000	/* 5 */
>>-#define NFS_MOUNT_NORDIRPLUS	0x4000	/* 5 */
>>-#define NFS_MOUNT_UNSHARED	0x8000	/* 5 */
>>+#define NFS_MOUNT_SOFT		0x00001	/* 1 */
>>+#define NFS_MOUNT_INTR		0x00002	/* 1 */
>>+#define NFS_MOUNT_SECURE	0x00004	/* 1 */
>>+#define NFS_MOUNT_POSIX		0x00008	/* 1 */
>>+#define NFS_MOUNT_NOCTO		0x00010	/* 1 */
>>+#define NFS_MOUNT_NOAC		0x00020	/* 1 */
>>+#define NFS_MOUNT_TCP		0x00040	/* 2 */
>>+#define NFS_MOUNT_VER3		0x00080	/* 3 */
>>+#define NFS_MOUNT_KERBEROS	0x00100	/* 3 */
>>+#define NFS_MOUNT_NONLM		0x00200	/* 3 */
>>+#define NFS_MOUNT_BROKEN_SUID	0x00400	/* 4 */
>>+#define NFS_MOUNT_NOACL		0x00800	/* 4 */
>>+#define NFS_MOUNT_SECFLAVOUR	0x02000	/* 5 */
>>+#define NFS_MOUNT_NORDIRPLUS	0x04000	/* 5 */
>>+#define NFS_MOUNT_UNSHARED	0x08000	/* 5 */
>>+#define NFS_MOUNT_NONEGDE	0x10000	/* 5 */
>
>Why is the patch replacing all the flag definitions?  Did you check  
>for inadvertent white space damage?

Actually, I fixed one bit of erroneous whitespace I found (spaces 
instead of tab).  The reason the definitions all change is because 
a leading 0.  The existing flags were 0x0001 through 0x8000.  The new 
flag is 0x10000.  I thought would be nice to make the flags all 5 digits 
so that they visually lined up.  I don't have to do that; it does make 
the patch uglier, but IMHO I think the resulting file is nicer.

>As I understand it, we aren't adding new options to the legacy part  of 
>the nfs-utils mount command any longer.  Instead, add support for  the 
>option in the kernel's NFS mount option parser in fs/nfs/super.c.

Oh, perhaps I didn't fully investigate the context.  I was just 
following the pattern I saw.  I can see if it still works without that 
one snippet (I believe I already updated fs/nfs/super.c appropriately).  
I presume updating the man page is still appropriate though, correct?  
What about defining the flag?  If it's not going to be used, should it 
still be defined as a placeholder?  I'll default to "yes" unless I hear 
otherwise.

-- 
Bob Bell