All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
@ 2009-08-25 17:52 Steve Dickson
  2009-08-25 17:54 ` [PATCH 1/2] " Steve Dickson
       [not found] ` <4A9424DB.2040303-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
  0 siblings, 2 replies; 32+ messages in thread
From: Steve Dickson @ 2009-08-25 17:52 UTC (permalink / raw)
  To: Linux NFSv4 mailing list; +Cc: Linux NFS Mailing list

Here is an update of the patch I posted yesterday... It turns
out the /etc/mtab was not getting updated with the correct 
files system type which in turn causing the remote mountd to
be called on v4 mounts... So the second patch deals with that issue... 

Both of these patches can be found on the "mount_vers4-r2" branch on 
my experiential git tree:
     git://linux-nfs.org/~steved/nfs-utils-exp.git


</rant>
Why we continue to use mtab instead of /proc/mounts on umount is a 
bit mind boggling... plus writing to the '/etc' directory is 
just wrong... IMHO... espically in a read-only root environment.. 
</rant> 


steved.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-25 17:52 [PATCH 0/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02) Steve Dickson
@ 2009-08-25 17:54 ` Steve Dickson
       [not found] ` <4A9424DB.2040303-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
  1 sibling, 0 replies; 32+ messages in thread
From: Steve Dickson @ 2009-08-25 17:54 UTC (permalink / raw)
  To: Linux NFSv4 mailing list; +Cc: Linux NFS Mailing list

commit 69ec4e2c98cd4727eef72c1e16b326171c5ac774
Author: Steve Dickson <steved@redhat.com>
Date:   Tue Aug 25 12:12:14 2009 -0400

    Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set.
    
    Since nfs4 is a different file system than nfs, the file system type
    has to be set when "v4" or "vers=4" or "nfsvers=4" are used. So
    when either of these options are specified, the file system type
    will be set to "nfs4" and those options will be stripped out of the
    option list since the kernel will not know how to parse them
    with v4 mounts
    
    Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/utils/mount/network.c b/utils/mount/network.c
index f6fa5fd..d3185b4 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -90,11 +90,11 @@ static const char *nfs_transport_opttbl[] = {
 static const char *nfs_version_opttbl[] = {
 	"v2",
 	"v3",
+	"v4",
 	"vers",
 	"nfsvers",
 	NULL,
 };
-
 static const unsigned long nfs_to_mnt[] = {
 	0,
 	0,
@@ -1203,7 +1203,7 @@ nfs_nfs_program(struct mount_options *options, unsigned long *program)
  * Returns TRUE if @version contains a valid value for this option,
  * or FALSE if the option was specified with an invalid value.
  */
-static int
+int
 nfs_nfs_version(struct mount_options *options, unsigned long *version)
 {
 	long tmp;
@@ -1215,10 +1215,13 @@ nfs_nfs_version(struct mount_options *options, unsigned long *version)
 	case 1: /* v3 */
 		*version = 3;
 		return 1;
-	case 2:	/* vers */
+	case 2: /* v4 */
+		*version = 4;
+		return 1;
+	case 3:	/* vers */
 		switch (po_get_numeric(options, "vers", &tmp)) {
 		case PO_FOUND:
-			if (tmp >= 2 && tmp <= 3) {
+			if (tmp >= 2 && tmp <= 4) {
 				*version = tmp;
 				return 1;
 			}
@@ -1229,10 +1232,10 @@ nfs_nfs_version(struct mount_options *options, unsigned long *version)
 		case PO_BAD_VALUE:
 			return 0;
 		}
-	case 3: /* nfsvers */
+	case 4: /* nfsvers */
 		switch (po_get_numeric(options, "nfsvers", &tmp)) {
 		case PO_FOUND:
-			if (tmp >= 2 && tmp <= 3) {
+			if (tmp >= 2 && tmp <= 4) {
 				*version = tmp;
 				return 1;
 			}
diff --git a/utils/mount/network.h b/utils/mount/network.h
index db5134c..5169b26 100644
--- a/utils/mount/network.h
+++ b/utils/mount/network.h
@@ -62,6 +62,7 @@ int nfs_options2pmap(struct mount_options *,
 int start_statd(void);
 
 unsigned long nfsvers_to_mnt(const unsigned long);
+int nfs_nfs_version(struct mount_options *, unsigned long *);
 
 int nfs_call_umount(clnt_addr_t *, dirpath *);
 int nfs_advise_umount(const struct sockaddr *, const socklen_t,
diff --git a/utils/mount/nfsmount.conf b/utils/mount/nfsmount.conf
index f9fcfcb..1848359 100644
--- a/utils/mount/nfsmount.conf
+++ b/utils/mount/nfsmount.conf
@@ -28,7 +28,7 @@
 # This statically named section defines global mount 
 # options that can be applied on all NFS mount.
 #
-# Protocol Version [2,3]
+# Protocol Version [2,3,4]
 # Nfsvers=3
 # Network Transport [Udp,Tcp,Rdma]
 # Proto=Tcp
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index a12ace7..153723d 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -656,6 +656,48 @@ static int nfsmount_bg(struct nfsmount_info *mi)
 		return nfsmount_child(mi);
 }
 
+static const char *nfs_fstype_opttbl[] = {
+	"v4",
+	"vers",
+	"nfsvers",
+	NULL,
+};
+
+/*
+ * Returns either "nfs" or "nfs4" as the file system type
+ * depending on which (if any) of the nfs version options
+ * are specified. 
+ */
+char *nfs_fs_type(struct nfsmount_info *mi)
+{
+	struct mount_options *options = mi->options;
+	char *fs_type = (char *)mi->type;
+	unsigned long version;
+
+	if (nfs_nfs_version(options, &version) == 0)
+		return fs_type;
+
+	if (version != 4)
+		return fs_type;
+
+	fs_type = "nfs4";
+	switch(po_rightmost(options, nfs_fstype_opttbl)) {
+	case 0: /* v4 */
+		po_remove_all(options, "v4");
+		break;
+	case 1: /* vers=4 */
+		po_remove_all(options, "vers");
+		break;
+	case 2: /* nfsvers=4 */
+		po_remove_all(options, "nfsvers");
+		break;
+	default: /* Is this even possible ?? */
+		fs_type = (char *)mi->type; 
+	}
+
+	return fs_type;
+}
+
 /*
  * Process mount options and try a mount system call.
  *
@@ -669,6 +711,9 @@ static const char *nfs_background_opttbl[] = {
 
 static int nfsmount_start(struct nfsmount_info *mi)
 {
+
+	mi->type = nfs_fs_type(mi);
+
 	if (!nfs_validate_options(mi))
 		return EX_FAIL;

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
       [not found] ` <4A9424DB.2040303-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
@ 2009-08-25 17:55   ` Steve Dickson
  2009-08-25 18:59     ` Chuck Lever
  0 siblings, 1 reply; 32+ messages in thread
From: Steve Dickson @ 2009-08-25 17:55 UTC (permalink / raw)
  To: Linux NFSv4 mailing list; +Cc: Linux NFS Mailing list

commit 1471d23d692efc7388794a8a3c3b9e548d1c5be8
Author: Steve Dickson <steved@redhat.com>
Date:   Tue Aug 25 12:15:18 2009 -0400

    Make sure umount use correct fs type.
    
    umounts use the fs type in /etc/mtab to determine
    which file system is being unmounted. The mtab
    entry is create during the mount. To ensure the
    correct entry is create when the fs type changes
    due to the mount options, the address of the fs_type
    variable has to be passed so it can be updated.
    
    Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/utils/mount/mount.c b/utils/mount/mount.c
index 355df79..507fbb5 100644
--- a/utils/mount/mount.c
+++ b/utils/mount/mount.c
@@ -417,13 +417,14 @@ static int chk_mountpoint(char *mount_point)
 }
 
 static int try_mount(char *spec, char *mount_point, int flags,
-			char *fs_type, char **extra_opts, char *mount_opts,
+			char *type, char **extra_opts, char *mount_opts,
 			int fake, int bg)
 {
 	int ret;
+	char *fs_type = type;
 
 	if (string)
-		ret = nfsmount_string(spec, mount_point, fs_type, flags,
+		ret = nfsmount_string(spec, mount_point, &fs_type, flags,
 					extra_opts, fake, bg);
 	else {
 		if (strcmp(fs_type, "nfs4") == 0)
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 153723d..1031c08 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -734,13 +734,13 @@ static int nfsmount_start(struct nfsmount_info *mi)
  * @fake: flag indicating whether to carry out the whole operation
  * @child: one if this is a mount daemon (bg)
  */
-int nfsmount_string(const char *spec, const char *node, const char *type,
+int nfsmount_string(const char *spec, const char *node, char **type,
 		    int flags, char **extra_opts, int fake, int child)
 {
 	struct nfsmount_info mi = {
 		.spec		= spec,
 		.node		= node,
-		.type		= type,
+		.type		= *type,
 		.extra_opts	= extra_opts,
 		.flags		= flags,
 		.fake		= fake,
@@ -751,6 +751,9 @@ int nfsmount_string(const char *spec, const char *node, const char *type,
 	mi.options = po_split(*extra_opts);
 	if (mi.options) {
 		retval = nfsmount_start(&mi);
+		/* Note any fs type changes */
+		if (!retval)
+			*type =  (char *)mi.type;
 		po_destroy(mi.options);
 	} else
 		nfs_error(_("%s: internal option parsing error"), progname);
diff --git a/utils/mount/stropts.h b/utils/mount/stropts.h
index b4fd888..dad9997 100644
--- a/utils/mount/stropts.h
+++ b/utils/mount/stropts.h
@@ -24,7 +24,7 @@
 #ifndef _NFS_UTILS_MOUNT_STROPTS_H
 #define _NFS_UTILS_MOUNT_STROPTS_H
 
-int nfsmount_string(const char *, const char *, const char *, int,
+int nfsmount_string(const char *, const char *, char **, int,
 			char **, int, int);
 
 #endif	/* _NFS_UTILS_MOUNT_STROPTS_H */


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-25 17:55   ` [PATCH 2/2] " Steve Dickson
@ 2009-08-25 18:59     ` Chuck Lever
  2009-08-25 19:18       ` Steve Dickson
  0 siblings, 1 reply; 32+ messages in thread
From: Chuck Lever @ 2009-08-25 18:59 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On Aug 25, 2009, at 1:55 PM, Steve Dickson wrote:
> commit 1471d23d692efc7388794a8a3c3b9e548d1c5be8
> Author: Steve Dickson <steved@redhat.com>
> Date:   Tue Aug 25 12:15:18 2009 -0400
>
>    Make sure umount use correct fs type.
>
>    umounts use the fs type in /etc/mtab to determine
>    which file system is being unmounted. The mtab
>    entry is create during the mount. To ensure the
>    correct entry is create when the fs type changes
>    due to the mount options, the address of the fs_type
>    variable has to be passed so it can be updated.

In general, my policy is to record the user requested mount options  
in /etc/mtab, and let umount.nfs handle renegotiating as needed.  For  
version/transport this means that the server's configuration can  
change between the mount and the umount, and the umount will still work.

Perhaps this is not a consideration for NFSv4, but leaving the mount  
options as specified by the user would save the need to update the fs  
type, and would be a consistent policy for v2, v3, and v4.  I think it  
would be cleaner to teach umount.nfs to do the right thing with "-t  
nfs -o v4" rather than rewriting the options in /etc/mtab.

>    Signed-off-by: Steve Dickson <steved@redhat.com>
>
> diff --git a/utils/mount/mount.c b/utils/mount/mount.c
> index 355df79..507fbb5 100644
> --- a/utils/mount/mount.c
> +++ b/utils/mount/mount.c
> @@ -417,13 +417,14 @@ static int chk_mountpoint(char *mount_point)
> }
>
> static int try_mount(char *spec, char *mount_point, int flags,
> -			char *fs_type, char **extra_opts, char *mount_opts,
> +			char *type, char **extra_opts, char *mount_opts,
> 			int fake, int bg)
> {
> 	int ret;
> +	char *fs_type = type;
>
> 	if (string)
> -		ret = nfsmount_string(spec, mount_point, fs_type, flags,
> +		ret = nfsmount_string(spec, mount_point, &fs_type, flags,
> 					extra_opts, fake, bg);
> 	else {
> 		if (strcmp(fs_type, "nfs4") == 0)
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 153723d..1031c08 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -734,13 +734,13 @@ static int nfsmount_start(struct nfsmount_info  
> *mi)
>  * @fake: flag indicating whether to carry out the whole operation
>  * @child: one if this is a mount daemon (bg)
>  */
> -int nfsmount_string(const char *spec, const char *node, const char  
> *type,
> +int nfsmount_string(const char *spec, const char *node, char **type,
> 		    int flags, char **extra_opts, int fake, int child)
> {
> 	struct nfsmount_info mi = {
> 		.spec		= spec,
> 		.node		= node,
> -		.type		= type,
> +		.type		= *type,
> 		.extra_opts	= extra_opts,
> 		.flags		= flags,
> 		.fake		= fake,
> @@ -751,6 +751,9 @@ int nfsmount_string(const char *spec, const char  
> *node, const char *type,
> 	mi.options = po_split(*extra_opts);
> 	if (mi.options) {
> 		retval = nfsmount_start(&mi);
> +		/* Note any fs type changes */
> +		if (!retval)
> +			*type =  (char *)mi.type;
> 		po_destroy(mi.options);
> 	} else
> 		nfs_error(_("%s: internal option parsing error"), progname);
> diff --git a/utils/mount/stropts.h b/utils/mount/stropts.h
> index b4fd888..dad9997 100644
> --- a/utils/mount/stropts.h
> +++ b/utils/mount/stropts.h
> @@ -24,7 +24,7 @@
> #ifndef _NFS_UTILS_MOUNT_STROPTS_H
> #define _NFS_UTILS_MOUNT_STROPTS_H
>
> -int nfsmount_string(const char *, const char *, const char *, int,
> +int nfsmount_string(const char *, const char *, char **, int,
> 			char **, int, int);
>
> #endif	/* _NFS_UTILS_MOUNT_STROPTS_H */
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs"  
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-25 18:59     ` Chuck Lever
@ 2009-08-25 19:18       ` Steve Dickson
  2009-08-25 19:32         ` Chuck Lever
  0 siblings, 1 reply; 32+ messages in thread
From: Steve Dickson @ 2009-08-25 19:18 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list



On 08/25/2009 02:59 PM, Chuck Lever wrote:
> On Aug 25, 2009, at 1:55 PM, Steve Dickson wrote:
>> commit 1471d23d692efc7388794a8a3c3b9e548d1c5be8
>> Author: Steve Dickson <steved@redhat.com>
>> Date:   Tue Aug 25 12:15:18 2009 -0400
>>
>>    Make sure umount use correct fs type.
>>
>>    umounts use the fs type in /etc/mtab to determine
>>    which file system is being unmounted. The mtab
>>    entry is create during the mount. To ensure the
>>    correct entry is create when the fs type changes
>>    due to the mount options, the address of the fs_type
>>    variable has to be passed so it can be updated.
> 
> In general, my policy is to record the user requested mount options in
> /etc/mtab, and let umount.nfs handle renegotiating as needed.  For
> version/transport this means that the server's configuration can change
> between the mount and the umount, and the umount will still work.
> 
> Perhaps this is not a consideration for NFSv4, but leaving the mount
> options as specified by the user would save the need to update the fs
> type, and would be a consistent policy for v2, v3, and v4.  I think it
> would be cleaner to teach umount.nfs to do the right thing with "-t nfs
> -o v4" rather than rewriting the options in /etc/mtab.
Since nfs4 is truly a separate/different file system from nfs in the
kernel, I think we should continue making that distinction in system 
files like mtab and /proc/mounts.... 

Also note there is no '-o ' flag to umount so 'umount -t nfs -o v4' is
not valid... but 'umount -t nfs' is and works on both nfs4 and nfs
file systems. 
 
steved.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-25 19:18       ` Steve Dickson
@ 2009-08-25 19:32         ` Chuck Lever
  2009-08-25 20:15           ` Steve Dickson
  0 siblings, 1 reply; 32+ messages in thread
From: Chuck Lever @ 2009-08-25 19:32 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On Aug 25, 2009, at 3:18 PM, Steve Dickson wrote:
> On 08/25/2009 02:59 PM, Chuck Lever wrote:
>> On Aug 25, 2009, at 1:55 PM, Steve Dickson wrote:
>>> commit 1471d23d692efc7388794a8a3c3b9e548d1c5be8
>>> Author: Steve Dickson <steved@redhat.com>
>>> Date:   Tue Aug 25 12:15:18 2009 -0400
>>>
>>>   Make sure umount use correct fs type.
>>>
>>>   umounts use the fs type in /etc/mtab to determine
>>>   which file system is being unmounted. The mtab
>>>   entry is create during the mount. To ensure the
>>>   correct entry is create when the fs type changes
>>>   due to the mount options, the address of the fs_type
>>>   variable has to be passed so it can be updated.
>>
>> In general, my policy is to record the user requested mount options  
>> in
>> /etc/mtab, and let umount.nfs handle renegotiating as needed.  For
>> version/transport this means that the server's configuration can  
>> change
>> between the mount and the umount, and the umount will still work.
>>
>> Perhaps this is not a consideration for NFSv4, but leaving the mount
>> options as specified by the user would save the need to update the fs
>> type, and would be a consistent policy for v2, v3, and v4.  I think  
>> it
>> would be cleaner to teach umount.nfs to do the right thing with "-t  
>> nfs
>> -o v4" rather than rewriting the options in /etc/mtab.
> Since nfs4 is truly a separate/different file system from nfs in the
> kernel, I think we should continue making that distinction in system
> files like mtab and /proc/mounts....

We are teaching mount.nfs not to care about nfs/nfs4 (at least  
externally).  Why should umount.nfs?

> Also note there is no '-o ' flag to umount so 'umount -t nfs -o v4' is
> not valid... but 'umount -t nfs' is and works on both nfs4 and nfs
> file systems.

Sorry I wasn't clear.  I meant that umount.nfs should be able to read  
a line in /etc/mtab that has "nfs" and "v4" and do the right thing...  
then you wouldn't have to change the fs_type in /etc/mtab at all.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-25 19:32         ` Chuck Lever
@ 2009-08-25 20:15           ` Steve Dickson
  2009-08-25 20:37             ` Chuck Lever
  2009-08-25 20:49             ` Trond Myklebust
  0 siblings, 2 replies; 32+ messages in thread
From: Steve Dickson @ 2009-08-25 20:15 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list



On 08/25/2009 03:32 PM, Chuck Lever wrote:
> On Aug 25, 2009, at 3:18 PM, Steve Dickson wrote:
>> On 08/25/2009 02:59 PM, Chuck Lever wrote:
>>> On Aug 25, 2009, at 1:55 PM, Steve Dickson wrote:
>>>> commit 1471d23d692efc7388794a8a3c3b9e548d1c5be8
>>>> Author: Steve Dickson <steved@redhat.com>
>>>> Date:   Tue Aug 25 12:15:18 2009 -0400
>>>>
>>>>   Make sure umount use correct fs type.
>>>>
>>>>   umounts use the fs type in /etc/mtab to determine
>>>>   which file system is being unmounted. The mtab
>>>>   entry is create during the mount. To ensure the
>>>>   correct entry is create when the fs type changes
>>>>   due to the mount options, the address of the fs_type
>>>>   variable has to be passed so it can be updated.
>>>
>>> In general, my policy is to record the user requested mount options in
>>> /etc/mtab, and let umount.nfs handle renegotiating as needed.  For
>>> version/transport this means that the server's configuration can change
>>> between the mount and the umount, and the umount will still work.
>>>
>>> Perhaps this is not a consideration for NFSv4, but leaving the mount
>>> options as specified by the user would save the need to update the fs
>>> type, and would be a consistent policy for v2, v3, and v4.  I think it
>>> would be cleaner to teach umount.nfs to do the right thing with "-t nfs
>>> -o v4" rather than rewriting the options in /etc/mtab.
>> Since nfs4 is truly a separate/different file system from nfs in the
>> kernel, I think we should continue making that distinction in system
>> files like mtab and /proc/mounts....
> 
> We are teaching mount.nfs not to care about nfs/nfs4 (at least
> externally).  Why should umount.nfs?
That's not quite accurate... IMHO.. I see it as we are teach mount.nfs to
accept new command line arguments that will cause a nfs4 file system
to be mounted... and that is done by caring which fs type mount is
dealing with... 

> 
>> Also note there is no '-o ' flag to umount so 'umount -t nfs -o v4' is
>> not valid... but 'umount -t nfs' is and works on both nfs4 and nfs
>> file systems.
> 
> Sorry I wasn't clear.  I meant that umount.nfs should be able to read a
> line in /etc/mtab that has "nfs" and "v4" and do the right thing... then
> you wouldn't have to change the fs_type in /etc/mtab at all.
Ok.. I gotta you now... and I did take a few minutes to look into what
something like this would take... I quickly came to the realization
that adding all complexity to make a system file, that nobody see or 
care about, more aesthetic really not worth it and not necessary, IMHO....

Point being,  umount is so simple when it comes to umounting a nfs4 file 
system... It basically does nothing! Which is a beautiful thing! So to added 
all the code (on both the mount and umount side) to translate
'-t nfs -o v4' into nfs4 (which  would have to happen since 
del_mtab() takes a fs type) is just not worth it... Especially when
the other option is adding no code to the umount side...  


steved.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-25 20:15           ` Steve Dickson
@ 2009-08-25 20:37             ` Chuck Lever
  2009-08-26 12:10               ` Steve Dickson
  2009-08-25 20:49             ` Trond Myklebust
  1 sibling, 1 reply; 32+ messages in thread
From: Chuck Lever @ 2009-08-25 20:37 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On Aug 25, 2009, at 4:15 PM, Steve Dickson wrote:
> On 08/25/2009 03:32 PM, Chuck Lever wrote:
>> On Aug 25, 2009, at 3:18 PM, Steve Dickson wrote:
>>> On 08/25/2009 02:59 PM, Chuck Lever wrote:
>>>> On Aug 25, 2009, at 1:55 PM, Steve Dickson wrote:
>>>>> commit 1471d23d692efc7388794a8a3c3b9e548d1c5be8
>>>>> Author: Steve Dickson <steved@redhat.com>
>>>>> Date:   Tue Aug 25 12:15:18 2009 -0400
>>>>>
>>>>>  Make sure umount use correct fs type.
>>>>>
>>>>>  umounts use the fs type in /etc/mtab to determine
>>>>>  which file system is being unmounted. The mtab
>>>>>  entry is create during the mount. To ensure the
>>>>>  correct entry is create when the fs type changes
>>>>>  due to the mount options, the address of the fs_type
>>>>>  variable has to be passed so it can be updated.
>>>>
>>>> In general, my policy is to record the user requested mount  
>>>> options in
>>>> /etc/mtab, and let umount.nfs handle renegotiating as needed.  For
>>>> version/transport this means that the server's configuration can  
>>>> change
>>>> between the mount and the umount, and the umount will still work.
>>>>
>>>> Perhaps this is not a consideration for NFSv4, but leaving the  
>>>> mount
>>>> options as specified by the user would save the need to update  
>>>> the fs
>>>> type, and would be a consistent policy for v2, v3, and v4.  I  
>>>> think it
>>>> would be cleaner to teach umount.nfs to do the right thing with "- 
>>>> t nfs
>>>> -o v4" rather than rewriting the options in /etc/mtab.
>>> Since nfs4 is truly a separate/different file system from nfs in the
>>> kernel, I think we should continue making that distinction in system
>>> files like mtab and /proc/mounts....
>>
>> We are teaching mount.nfs not to care about nfs/nfs4 (at least
>> externally).  Why should umount.nfs?
> That's not quite accurate... IMHO.. I see it as we are teach  
> mount.nfs to
> accept new command line arguments that will cause a nfs4 file system
> to be mounted... and that is done by caring which fs type mount is
> dealing with...

Right.  I think umount.nfs should get the same treatment.  But it gets  
it's "command line arguments" from /etc/mtab.

>>> Also note there is no '-o ' flag to umount so 'umount -t nfs -o  
>>> v4' is
>>> not valid... but 'umount -t nfs' is and works on both nfs4 and nfs
>>> file systems.
>>
>> Sorry I wasn't clear.  I meant that umount.nfs should be able to  
>> read a
>> line in /etc/mtab that has "nfs" and "v4" and do the right thing...  
>> then
>> you wouldn't have to change the fs_type in /etc/mtab at all.
> Ok.. I gotta you now... and I did take a few minutes to look into what
> something like this would take... I quickly came to the realization
> that adding all complexity to make a system file, that nobody see or
> care about, more aesthetic really not worth it and not necessary,  
> IMHO....

It's more of a maintainability issue.  Make umount.nfs behave the same  
way for v2, v3, and v4, instead of doing one thing for v2/v3 and  
another for v4.

> Point being,  umount is so simple when it comes to umounting a nfs4  
> file
> system... It basically does nothing! Which is a beautiful thing! So  
> to added
> all the code (on both the mount and umount side) to translate
> '-t nfs -o v4' into nfs4 (which  would have to happen since
> del_mtab() takes a fs type) is just not worth it... Especially when
> the other option is adding no code to the umount side...

I doubt it would be a lot of complexity, actually.  We already have  
parser calls in umount.nfs to handle v2/v3 version/transport  
negotiation, so I don't think it would be much of a stretch at all to  
look for "v4" before deciding whether to do a v2/v3 umount or a v4  
umount.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-25 20:15           ` Steve Dickson
  2009-08-25 20:37             ` Chuck Lever
@ 2009-08-25 20:49             ` Trond Myklebust
  2009-08-26 12:28               ` Steve Dickson
  2009-08-26 14:20               ` Chuck Lever
  1 sibling, 2 replies; 32+ messages in thread
From: Trond Myklebust @ 2009-08-25 20:49 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On Tue, 2009-08-25 at 16:15 -0400, Steve Dickson wrote:
> 
> On 08/25/2009 03:32 PM, Chuck Lever wrote:
> > On Aug 25, 2009, at 3:18 PM, Steve Dickson wrote:
> >> On 08/25/2009 02:59 PM, Chuck Lever wrote:
> >>> On Aug 25, 2009, at 1:55 PM, Steve Dickson wrote:
> >>>> commit 1471d23d692efc7388794a8a3c3b9e548d1c5be8
> >>>> Author: Steve Dickson <steved@redhat.com>
> >>>> Date:   Tue Aug 25 12:15:18 2009 -0400
> >>>>
> >>>>   Make sure umount use correct fs type.
> >>>>
> >>>>   umounts use the fs type in /etc/mtab to determine
> >>>>   which file system is being unmounted. The mtab
> >>>>   entry is create during the mount. To ensure the
> >>>>   correct entry is create when the fs type changes
> >>>>   due to the mount options, the address of the fs_type
> >>>>   variable has to be passed so it can be updated.
> >>>
> >>> In general, my policy is to record the user requested mount options in
> >>> /etc/mtab, and let umount.nfs handle renegotiating as needed.  For
> >>> version/transport this means that the server's configuration can change
> >>> between the mount and the umount, and the umount will still work.
> >>>
> >>> Perhaps this is not a consideration for NFSv4, but leaving the mount
> >>> options as specified by the user would save the need to update the fs
> >>> type, and would be a consistent policy for v2, v3, and v4.  I think it
> >>> would be cleaner to teach umount.nfs to do the right thing with "-t nfs
> >>> -o v4" rather than rewriting the options in /etc/mtab.
> >> Since nfs4 is truly a separate/different file system from nfs in the
> >> kernel, I think we should continue making that distinction in system
> >> files like mtab and /proc/mounts....
> > 
> > We are teaching mount.nfs not to care about nfs/nfs4 (at least
> > externally).  Why should umount.nfs?
> That's not quite accurate... IMHO.. I see it as we are teach mount.nfs to
> accept new command line arguments that will cause a nfs4 file system
> to be mounted... and that is done by caring which fs type mount is
> dealing with... 

So, why couldn't we just do this in the kernel? It should be easily
doable to set nfs -overs=4 to mount an NFSv4 filesystem. We only have to
do this for text mounts...

Cheers
   Trond

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-25 20:37             ` Chuck Lever
@ 2009-08-26 12:10               ` Steve Dickson
  0 siblings, 0 replies; 32+ messages in thread
From: Steve Dickson @ 2009-08-26 12:10 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On 08/25/2009 04:37 PM, Chuck Lever wrote:
>>>> Also note there is no '-o ' flag to umount so 'umount -t nfs -o v4' is
>>>> not valid... but 'umount -t nfs' is and works on both nfs4 and nfs
>>>> file systems.
>>>
>>> Sorry I wasn't clear.  I meant that umount.nfs should be able to read a
>>> line in /etc/mtab that has "nfs" and "v4" and do the right thing... then
>>> you wouldn't have to change the fs_type in /etc/mtab at all.
>> Ok.. I gotta you now... and I did take a few minutes to look into what
>> something like this would take... I quickly came to the realization
>> that adding all complexity to make a system file, that nobody see or
>> care about, more aesthetic really not worth it and not necessary,
>> IMHO....
> 
> It's more of a maintainability issue.  Make umount.nfs behave the same
> way for v2, v3, and v4, instead of doing one thing for v2/v3 and another
> for v4.
Then why even have a mount.nfs4 command? Lets simple get ride of that 
command all together and ignore the fact nfs and nfs4 are to separate 
filesystems? Personally I think this would be wrong... 

It was deemed, rightly so, that nfs4 would be a separate file system.
So there there will be things that will have to be done to maintain
both of them... All this patch set does is create a shorthand way of 
mounting an nfs4 file system... nothing more and nothing less... 
   
> 
>> Point being,  umount is so simple when it comes to umounting a nfs4 file
>> system... It basically does nothing! Which is a beautiful thing! So to
>> added
>> all the code (on both the mount and umount side) to translate
>> '-t nfs -o v4' into nfs4 (which  would have to happen since
>> del_mtab() takes a fs type) is just not worth it... Especially when
>> the other option is adding no code to the umount side...
> 
> I doubt it would be a lot of complexity, actually.  We already have
> parser calls in umount.nfs to handle v2/v3 version/transport
> negotiation, so I don't think it would be much of a stretch at all to
> look for "v4" before deciding whether to do a v2/v3 umount or a v4 umount.

Let's make a deal! ;-) If a bug report is opened about the exact user-given command 
arguments to the mount command are not portrayed correctly in /etc/mtab, 
I will fix that bug and then buy you dinner! :-) 

steved.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-25 20:49             ` Trond Myklebust
@ 2009-08-26 12:28               ` Steve Dickson
  2009-08-26 14:20               ` Chuck Lever
  1 sibling, 0 replies; 32+ messages in thread
From: Steve Dickson @ 2009-08-26 12:28 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On 08/25/2009 04:49 PM, Trond Myklebust wrote:
> On Tue, 2009-08-25 at 16:15 -0400, Steve Dickson wrote:
>>
>> On 08/25/2009 03:32 PM, Chuck Lever wrote:
>>> On Aug 25, 2009, at 3:18 PM, Steve Dickson wrote:
>>>> On 08/25/2009 02:59 PM, Chuck Lever wrote:
>>>>> On Aug 25, 2009, at 1:55 PM, Steve Dickson wrote:
>>>>>> commit 1471d23d692efc7388794a8a3c3b9e548d1c5be8
>>>>>> Author: Steve Dickson <steved@redhat.com>
>>>>>> Date:   Tue Aug 25 12:15:18 2009 -0400
>>>>>>
>>>>>>   Make sure umount use correct fs type.
>>>>>>
>>>>>>   umounts use the fs type in /etc/mtab to determine
>>>>>>   which file system is being unmounted. The mtab
>>>>>>   entry is create during the mount. To ensure the
>>>>>>   correct entry is create when the fs type changes
>>>>>>   due to the mount options, the address of the fs_type
>>>>>>   variable has to be passed so it can be updated.
>>>>>
>>>>> In general, my policy is to record the user requested mount options in
>>>>> /etc/mtab, and let umount.nfs handle renegotiating as needed.  For
>>>>> version/transport this means that the server's configuration can change
>>>>> between the mount and the umount, and the umount will still work.
>>>>>
>>>>> Perhaps this is not a consideration for NFSv4, but leaving the mount
>>>>> options as specified by the user would save the need to update the fs
>>>>> type, and would be a consistent policy for v2, v3, and v4.  I think it
>>>>> would be cleaner to teach umount.nfs to do the right thing with "-t nfs
>>>>> -o v4" rather than rewriting the options in /etc/mtab.
>>>> Since nfs4 is truly a separate/different file system from nfs in the
>>>> kernel, I think we should continue making that distinction in system
>>>> files like mtab and /proc/mounts....
>>>
>>> We are teaching mount.nfs not to care about nfs/nfs4 (at least
>>> externally).  Why should umount.nfs?
>> That's not quite accurate... IMHO.. I see it as we are teach mount.nfs to
>> accept new command line arguments that will cause a nfs4 file system
>> to be mounted... and that is done by caring which fs type mount is
>> dealing with... 
> 
> So, why couldn't we just do this in the kernel? It should be easily
> doable to set nfs -overs=4 to mount an NFSv4 filesystem. We only have to
> do this for text mounts...
Basically time to market.... A kernel patch will not make into a mainstream
kernel until 2.6.32 which is in the distant future... verses having this
patch committed in the very near future (like today! ;-) ). 

Plus its historical reasons... Things like this have always been done in 
in user level code since its much easier to change user level code that it
is kernel.

With that said, I will be more than willing to come up with kernel patch
which in the end would make these patch obsolete... but again that
would be in the distant future... 

steved.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-25 20:49             ` Trond Myklebust
  2009-08-26 12:28               ` Steve Dickson
@ 2009-08-26 14:20               ` Chuck Lever
  2009-08-26 15:07                 ` Steve Dickson
  1 sibling, 1 reply; 32+ messages in thread
From: Chuck Lever @ 2009-08-26 14:20 UTC (permalink / raw)
  To: Trond Myklebust, Steve Dickson
  Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On Aug 25, 2009, at 4:49 PM, Trond Myklebust wrote:
> On Tue, 2009-08-25 at 16:15 -0400, Steve Dickson wrote:
>>
>> On 08/25/2009 03:32 PM, Chuck Lever wrote:
>>> On Aug 25, 2009, at 3:18 PM, Steve Dickson wrote:
>>>> On 08/25/2009 02:59 PM, Chuck Lever wrote:
>>>>> On Aug 25, 2009, at 1:55 PM, Steve Dickson wrote:
>>>>>> commit 1471d23d692efc7388794a8a3c3b9e548d1c5be8
>>>>>> Author: Steve Dickson <steved@redhat.com>
>>>>>> Date:   Tue Aug 25 12:15:18 2009 -0400
>>>>>>
>>>>>>  Make sure umount use correct fs type.
>>>>>>
>>>>>>  umounts use the fs type in /etc/mtab to determine
>>>>>>  which file system is being unmounted. The mtab
>>>>>>  entry is create during the mount. To ensure the
>>>>>>  correct entry is create when the fs type changes
>>>>>>  due to the mount options, the address of the fs_type
>>>>>>  variable has to be passed so it can be updated.
>>>>>
>>>>> In general, my policy is to record the user requested mount  
>>>>> options in
>>>>> /etc/mtab, and let umount.nfs handle renegotiating as needed.  For
>>>>> version/transport this means that the server's configuration can  
>>>>> change
>>>>> between the mount and the umount, and the umount will still work.
>>>>>
>>>>> Perhaps this is not a consideration for NFSv4, but leaving the  
>>>>> mount
>>>>> options as specified by the user would save the need to update  
>>>>> the fs
>>>>> type, and would be a consistent policy for v2, v3, and v4.  I  
>>>>> think it
>>>>> would be cleaner to teach umount.nfs to do the right thing with  
>>>>> "-t nfs
>>>>> -o v4" rather than rewriting the options in /etc/mtab.
>>>> Since nfs4 is truly a separate/different file system from nfs in  
>>>> the
>>>> kernel, I think we should continue making that distinction in  
>>>> system
>>>> files like mtab and /proc/mounts....
>>>
>>> We are teaching mount.nfs not to care about nfs/nfs4 (at least
>>> externally).  Why should umount.nfs?
>> That's not quite accurate... IMHO.. I see it as we are teach  
>> mount.nfs to
>> accept new command line arguments that will cause a nfs4 file system
>> to be mounted... and that is done by caring which fs type mount is
>> dealing with...
>
> So, why couldn't we just do this in the kernel? It should be easily
> doable to set nfs -overs=4 to mount an NFSv4 filesystem. We only  
> have to
> do this for text mounts...

I think that would be a much better approach.  If nfs4 goes away  
someday, for example, it will be completely transparent to the mount  
command if we've already pushed "-t nfs, vers=4" conversion into the  
kernel.

We are pushing all of the details of NFS mounting into the kernel  
anyway, over time.  If we change the mount command now, and plan to  
change the kernel in the future, that will make the mount command  
balloon in complexity over time (if it's this kernel version, do this;  
other kernel versions do that; yet others do something else).  Yes, we  
can make the mount command do that, but do we really want to make this  
a policy for all new features that we can do it in mount first?  One  
of the reasons for text-based mounts was to do all this in the kernel  
so we don't have feature dependencies on user space.

We have already fixed version/transport negotiation in the mount  
command with the promise that the kernel will handle it later; I think  
that will be an issue down the road.  In the version/transport case,  
though, that feature had already been widely deployed, so an immediate  
fix was nearly mandatory.  For vers=4, we still have an opportunity to  
think ahead.

Another advantage is that this would provide cleaner NFSROOT v4 support.

The problem with having the kernel handle the version upgrade, though,  
is that the NFS super code paths would need to convert the mount to an  
nfs4 file system type when vers=4 is detected.  I looked at that a  
little last week, and it looked potentially pretty wonky.  Maybe you  
have an idea how to make this easy?

Note also that if the kernel does the vers=4 processing instead of the  
mount command, we would have to change the umount command as I  
described before anyway.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-26 14:20               ` Chuck Lever
@ 2009-08-26 15:07                 ` Steve Dickson
  2009-08-26 16:35                   ` Chuck Lever
  0 siblings, 1 reply; 32+ messages in thread
From: Steve Dickson @ 2009-08-26 15:07 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On 08/26/2009 10:20 AM, Chuck Lever wrote:
> On Aug 25, 2009, at 4:49 PM, Trond Myklebust wrote:
>> On Tue, 2009-08-25 at 16:15 -0400, Steve Dickson wrote:
>>>
>>> On 08/25/2009 03:32 PM, Chuck Lever wrote:
>>>> On Aug 25, 2009, at 3:18 PM, Steve Dickson wrote:
>>>>> On 08/25/2009 02:59 PM, Chuck Lever wrote:
>>>>>> On Aug 25, 2009, at 1:55 PM, Steve Dickson wrote:
>>>>>>> commit 1471d23d692efc7388794a8a3c3b9e548d1c5be8
>>>>>>> Author: Steve Dickson <steved@redhat.com>
>>>>>>> Date:   Tue Aug 25 12:15:18 2009 -0400
>>>>>>>
>>>>>>>  Make sure umount use correct fs type.
>>>>>>>
>>>>>>>  umounts use the fs type in /etc/mtab to determine
>>>>>>>  which file system is being unmounted. The mtab
>>>>>>>  entry is create during the mount. To ensure the
>>>>>>>  correct entry is create when the fs type changes
>>>>>>>  due to the mount options, the address of the fs_type
>>>>>>>  variable has to be passed so it can be updated.
>>>>>>
>>>>>> In general, my policy is to record the user requested mount
>>>>>> options in
>>>>>> /etc/mtab, and let umount.nfs handle renegotiating as needed.  For
>>>>>> version/transport this means that the server's configuration can
>>>>>> change
>>>>>> between the mount and the umount, and the umount will still work.
>>>>>>
>>>>>> Perhaps this is not a consideration for NFSv4, but leaving the mount
>>>>>> options as specified by the user would save the need to update the fs
>>>>>> type, and would be a consistent policy for v2, v3, and v4.  I
>>>>>> think it
>>>>>> would be cleaner to teach umount.nfs to do the right thing with
>>>>>> "-t nfs
>>>>>> -o v4" rather than rewriting the options in /etc/mtab.
>>>>> Since nfs4 is truly a separate/different file system from nfs in the
>>>>> kernel, I think we should continue making that distinction in system
>>>>> files like mtab and /proc/mounts....
>>>>
>>>> We are teaching mount.nfs not to care about nfs/nfs4 (at least
>>>> externally).  Why should umount.nfs?
>>> That's not quite accurate... IMHO.. I see it as we are teach
>>> mount.nfs to
>>> accept new command line arguments that will cause a nfs4 file system
>>> to be mounted... and that is done by caring which fs type mount is
>>> dealing with...
>>
>> So, why couldn't we just do this in the kernel? It should be easily
>> doable to set nfs -overs=4 to mount an NFSv4 filesystem. We only have to
>> do this for text mounts...
> 
> I think that would be a much better approach.  If nfs4 goes away
> someday, for example, it will be completely transparent to the mount
> command if we've already pushed "-t nfs, vers=4" conversion into the
> kernel.
Well when/if that day comes, we can easily pull the patches from the mount
command.

> 
> We are pushing all of the details of NFS mounting into the kernel
> anyway, over time.  
Which I've never been a fan of... Again it's much easier change user
level code (and more people can do it) than kernel code... especially
with things of this nature...
 
> If we change the mount command now, and plan to
> change the kernel in the future, that will make the mount command
> balloon in complexity over time (if it's this kernel version, do this;
> other kernel versions do that; yet others do something else).  Yes, we
> can make the mount command do that, but do we really want to make this a
> policy for all new features that we can do it in mount first?  One of
> the reasons for text-based mounts was to do all this in the kernel so we
> don't have feature dependencies on user space.
I disagree with the ifdef kernel... The approach I'm propose will *always*
work as long as the nfs4 file system exists. If someday down the road 
the kernel learns how to deal with -o v4 *and* its decided that the nfs and 
nfs4 file systems will be combined into one file system (maybe due to 
protocol/version negotiation) then yes, the mount command will have to 
change.. but IMHO I don't see that day coming any time soon... 

> 
> We have already fixed version/transport negotiation in the mount command
> with the promise that the kernel will handle it later; I think that will
> be an issue down the road.  In the version/transport case, though, that
> feature had already been widely deployed, so an immediate fix was nearly
> mandatory.  For vers=4, we still have an opportunity to think ahead
> 
> Another advantage is that this would provide cleaner NFSROOT v4 support.
Maybe or may not... Only time will tell..

> 
> The problem with having the kernel handle the version upgrade, though,
> is that the NFS super code paths would need to convert the mount to an
> nfs4 file system type when vers=4 is detected.  I looked at that a
> little last week, and it looked potentially pretty wonky.  Maybe you
> have an idea how to make this easy?
I also took a look and yes its is much much easier and straight forward
to do the vers=4 handling in the mount command... at least at this
point..

> 
> Note also that if the kernel does the vers=4 processing instead of the
> mount command, we would have to change the umount command as I described
> before anyway.
Granted... yeah another reason to to do the processing in the mount command 8-)

steved.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-26 15:07                 ` Steve Dickson
@ 2009-08-26 16:35                   ` Chuck Lever
  2009-08-26 17:08                     ` Steve Dickson
  0 siblings, 1 reply; 32+ messages in thread
From: Chuck Lever @ 2009-08-26 16:35 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list


On Aug 26, 2009, at 11:07 AM, Steve Dickson wrote:

> On 08/26/2009 10:20 AM, Chuck Lever wrote:
>> On Aug 25, 2009, at 4:49 PM, Trond Myklebust wrote:
>>> On Tue, 2009-08-25 at 16:15 -0400, Steve Dickson wrote:
>>>>
>>>> On 08/25/2009 03:32 PM, Chuck Lever wrote:
>>>>> On Aug 25, 2009, at 3:18 PM, Steve Dickson wrote:
>>>>>> On 08/25/2009 02:59 PM, Chuck Lever wrote:
>>>>>>> On Aug 25, 2009, at 1:55 PM, Steve Dickson wrote:
>>>>>>>> commit 1471d23d692efc7388794a8a3c3b9e548d1c5be8
>>>>>>>> Author: Steve Dickson <steved@redhat.com>
>>>>>>>> Date:   Tue Aug 25 12:15:18 2009 -0400
>>>>>>>>
>>>>>>>> Make sure umount use correct fs type.
>>>>>>>>
>>>>>>>> umounts use the fs type in /etc/mtab to determine
>>>>>>>> which file system is being unmounted. The mtab
>>>>>>>> entry is create during the mount. To ensure the
>>>>>>>> correct entry is create when the fs type changes
>>>>>>>> due to the mount options, the address of the fs_type
>>>>>>>> variable has to be passed so it can be updated.
>>>>>>>
>>>>>>> In general, my policy is to record the user requested mount
>>>>>>> options in
>>>>>>> /etc/mtab, and let umount.nfs handle renegotiating as needed.   
>>>>>>> For
>>>>>>> version/transport this means that the server's configuration can
>>>>>>> change
>>>>>>> between the mount and the umount, and the umount will still  
>>>>>>> work.
>>>>>>>
>>>>>>> Perhaps this is not a consideration for NFSv4, but leaving the  
>>>>>>> mount
>>>>>>> options as specified by the user would save the need to update  
>>>>>>> the fs
>>>>>>> type, and would be a consistent policy for v2, v3, and v4.  I
>>>>>>> think it
>>>>>>> would be cleaner to teach umount.nfs to do the right thing with
>>>>>>> "-t nfs
>>>>>>> -o v4" rather than rewriting the options in /etc/mtab.
>>>>>> Since nfs4 is truly a separate/different file system from nfs  
>>>>>> in the
>>>>>> kernel, I think we should continue making that distinction in  
>>>>>> system
>>>>>> files like mtab and /proc/mounts....
>>>>>
>>>>> We are teaching mount.nfs not to care about nfs/nfs4 (at least
>>>>> externally).  Why should umount.nfs?
>>>> That's not quite accurate... IMHO.. I see it as we are teach
>>>> mount.nfs to
>>>> accept new command line arguments that will cause a nfs4 file  
>>>> system
>>>> to be mounted... and that is done by caring which fs type mount is
>>>> dealing with...
>>>
>>> So, why couldn't we just do this in the kernel? It should be easily
>>> doable to set nfs -overs=4 to mount an NFSv4 filesystem. We only  
>>> have to
>>> do this for text mounts...
>>
>> I think that would be a much better approach.  If nfs4 goes away
>> someday, for example, it will be completely transparent to the mount
>> command if we've already pushed "-t nfs, vers=4" conversion into the
>> kernel.
> Well when/if that day comes, we can easily pull the patches from the  
> mount
> command.

You know it's never that easy.  The mount command has to keep legacy  
logic for older kernels.  I'm just saying that the less the mount  
command has to worry about what kernel version is running, the cleaner  
the mount command will be.

>> We are pushing all of the details of NFS mounting into the kernel
>> anyway, over time.
> Which I've never been a fan of... Again it's much easier change user
> level code (and more people can do it) than kernel code... especially
> with things of this nature...

People can continue to change the mount command all they want.  In  
fact the user space text-based option parsing code is pretty darn  
flexible as it is now.

I don't think we're denying that your proposal is expedient.  The  
question I think is where we want to be in the long run, and if your  
proposed method to handle -t nfs -o vers=4 will make it more  
complicated to get there.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-26 16:35                   ` Chuck Lever
@ 2009-08-26 17:08                     ` Steve Dickson
  2009-08-26 17:22                       ` Chuck Lever
  0 siblings, 1 reply; 32+ messages in thread
From: Steve Dickson @ 2009-08-26 17:08 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On 08/26/2009 12:35 PM, Chuck Lever wrote: 
> On Aug 26, 2009, at 11:07 AM, Steve Dickson wrote:
>>> I think that would be a much better approach.  If nfs4 goes away
>>> someday, for example, it will be completely transparent to the mount
>>> command if we've already pushed "-t nfs, vers=4" conversion into the
>>> kernel.
>> Well when/if that day comes, we can easily pull the patches from the
>> mount
>> command.
> 
> You know it's never that easy.  The mount command has to keep legacy
> logic for older kernels.  I'm just saying that the less the mount
> command has to worry about what kernel version is running, the cleaner
> the mount command will be.
Well with this patch, since we are only concentrating on text mounts,
we are already breaking with the tradition of keeping legacy logic... 
And again as long as the nfs4 file system exists this approach will work... 
 
> 
>>> We are pushing all of the details of NFS mounting into the kernel
>>> anyway, over time.
>> Which I've never been a fan of... Again it's much easier change user
>> level code (and more people can do it) than kernel code... especially
>> with things of this nature...
> 
> People can continue to change the mount command all they want.  In fact
> the user space text-based option parsing code is pretty darn flexible as
> it is now.
Yes... the user space parsing code is very well written... 

> 
> I don't think we're denying that your proposal is expedient.  The
> question I think is where we want to be in the long run, 
NFS v4 as the default protocol version followed by NFS V4.1 becoming the
default protocol version. 

> and if your proposed method to handle -t nfs -o vers=4 will make 
> it more complicated to get there.
No. I'm proposing a simple shorthand patch that will make mounting nfs4
file systems easier in hope of moving the technology forward by making 
it more accessible... What I believe you are proposing is architecture
change to hide the fact nfs and nfs4 are separate file systems... 

But in the end, if we do the simple shorthand patch (making the technology 
available today) or the major architecture change (making the technology
available the distant future) with both approaches 'mount -o v4' 
will do the exact same thing... 

I for moving the technology today... 

steved.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-26 17:08                     ` Steve Dickson
@ 2009-08-26 17:22                       ` Chuck Lever
  2009-08-26 17:51                         ` Steve Dickson
  0 siblings, 1 reply; 32+ messages in thread
From: Chuck Lever @ 2009-08-26 17:22 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list


On Aug 26, 2009, at 1:08 PM, Steve Dickson wrote:

> On 08/26/2009 12:35 PM, Chuck Lever wrote:
>> On Aug 26, 2009, at 11:07 AM, Steve Dickson wrote:
>>>> I think that would be a much better approach.  If nfs4 goes away
>>>> someday, for example, it will be completely transparent to the  
>>>> mount
>>>> command if we've already pushed "-t nfs, vers=4" conversion into  
>>>> the
>>>> kernel.
>>> Well when/if that day comes, we can easily pull the patches from the
>>> mount
>>> command.
>>
>> You know it's never that easy.  The mount command has to keep legacy
>> logic for older kernels.  I'm just saying that the less the mount
>> command has to worry about what kernel version is running, the  
>> cleaner
>> the mount command will be.
> Well with this patch, since we are only concentrating on text mounts,
> we are already breaking with the tradition of keeping legacy logic...
> And again as long as the nfs4 file system exists this approach will  
> work...
>
>>
>>>> We are pushing all of the details of NFS mounting into the kernel
>>>> anyway, over time.
>>> Which I've never been a fan of... Again it's much easier change user
>>> level code (and more people can do it) than kernel code...  
>>> especially
>>> with things of this nature...
>>
>> People can continue to change the mount command all they want.  In  
>> fact
>> the user space text-based option parsing code is pretty darn  
>> flexible as
>> it is now.
> Yes... the user space parsing code is very well written...
>
>>
>> I don't think we're denying that your proposal is expedient.  The
>> question I think is where we want to be in the long run,
> NFS v4 as the default protocol version followed by NFS V4.1 becoming  
> the
> default protocol version.
>
>> and if your proposed method to handle -t nfs -o vers=4 will make
>> it more complicated to get there.
> No. I'm proposing a simple shorthand patch that will make mounting  
> nfs4
> file systems easier in hope of moving the technology forward by making
> it more accessible... What I believe you are proposing is architecture
> change to hide the fact nfs and nfs4 are separate file systems...

Nope, we're proposing doing the simple method in the kernel instead of  
in the mount command.

> But in the end, if we do the simple shorthand patch (making the  
> technology
> available today) or the major architecture change (making the  
> technology
> available the distant future) with both approaches 'mount -o v4'
> will do the exact same thing...
>
> I for moving the technology today...
>
> steved.
>
>
>

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-26 17:22                       ` Chuck Lever
@ 2009-08-26 17:51                         ` Steve Dickson
  2009-08-26 19:50                           ` Chuck Lever
  0 siblings, 1 reply; 32+ messages in thread
From: Steve Dickson @ 2009-08-26 17:51 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On 08/26/2009 01:22 PM, Chuck Lever wrote:
>>> and if your proposed method to handle -t nfs -o vers=4 will make
>>> it more complicated to get there.
>> No. I'm proposing a simple shorthand patch that will make mounting nfs4
>> file systems easier in hope of moving the technology forward by making
>> it more accessible... What I believe you are proposing is architecture
>> change to hide the fact nfs and nfs4 are separate file systems...
> 
> Nope, we're proposing doing the simple method in the kernel instead of
> in the mount command.
> 
My apologize then... I was misinterpreting what you guys were suggesting..
(email sometimes causes that... :-\ )

I don't think the -o v4 translation will be as easy as a 
"simple method in the kernel" and it surely will not be as simple 
and unintrusive as the patch I'm proposing.... Here is why... 

>From an Linux  architecture standpoint the mount command *always*
know what file system its mounting. There not been a precedence set
where mount, mounts one file system which turns into mounting a
different file system. Meaning there is no kernel support for
nfs_get_sb() to all of sudden decide to roll back the system call
and jump into nfs4_get_sb() (or vice a versa depending on which is the 
default).

Of course we could set that precedence and quit frank we would have
to. I'm not totally against that, although other in the kernel community
might be...  

But there is no way that re-architecturing of kernel will as simple
and straightforward as following the Linux standard architecture of
having mount, mount the correct file system.

steved.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-26 17:51                         ` Steve Dickson
@ 2009-08-26 19:50                           ` Chuck Lever
  2009-08-26 19:59                             ` Trond Myklebust
  0 siblings, 1 reply; 32+ messages in thread
From: Chuck Lever @ 2009-08-26 19:50 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On Aug 26, 2009, at 1:51 PM, Steve Dickson wrote:
> On 08/26/2009 01:22 PM, Chuck Lever wrote:
>>>> and if your proposed method to handle -t nfs -o vers=4 will make
>>>> it more complicated to get there.
>>> No. I'm proposing a simple shorthand patch that will make mounting  
>>> nfs4
>>> file systems easier in hope of moving the technology forward by  
>>> making
>>> it more accessible... What I believe you are proposing is  
>>> architecture
>>> change to hide the fact nfs and nfs4 are separate file systems...
>>
>> Nope, we're proposing doing the simple method in the kernel instead  
>> of
>> in the mount command.
>>
> My apologize then... I was misinterpreting what you guys were  
> suggesting..
> (email sometimes causes that... :-\ )
>
> I don't think the -o v4 translation will be as easy as a
> "simple method in the kernel" and it surely will not be as simple
> and unintrusive as the patch I'm proposing.... Here is why...
>
> From an Linux  architecture standpoint the mount command *always*
> know what file system its mounting. There not been a precedence set
> where mount, mounts one file system which turns into mounting a
> different file system. Meaning there is no kernel support for
> nfs_get_sb() to all of sudden decide to roll back the system call
> and jump into nfs4_get_sb() (or vice a versa depending on which is the
> default).

Yeah, switching file system types in the mount(2) system call is the  
fly in the ointment.  I'm just wondering if Trond had some thoughts  
about making that more feasible.

> Of course we could set that precedence and quit frank we would have
> to. I'm not totally against that, although other in the kernel  
> community
> might be...
>
> But there is no way that re-architecturing of kernel will as simple
> and straightforward as following the Linux standard architecture of
> having mount, mount the correct file system.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-26 19:50                           ` Chuck Lever
@ 2009-08-26 19:59                             ` Trond Myklebust
  2009-08-27 14:14                               ` Steve Dickson
  0 siblings, 1 reply; 32+ messages in thread
From: Trond Myklebust @ 2009-08-26 19:59 UTC (permalink / raw)
  To: Chuck Lever
  Cc: Steve Dickson, Linux NFSv4 mailing list, Linux NFS Mailing list

On Wed, 2009-08-26 at 15:50 -0400, Chuck Lever wrote:
> Yeah, switching file system types in the mount(2) system call is the  
> fly in the ointment.  I'm just wondering if Trond had some thoughts  
> about making that more feasible.

Just look at what we're already doing for NFSv4. Inside nfs4_get_sb, we
basically do a kernel mount in order to get the real super block. We
then simply have to attach it to the vfsmount that the sys_mount() call
passed down to us.

This really isn't anything new or difficult...



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-26 19:59                             ` Trond Myklebust
@ 2009-08-27 14:14                               ` Steve Dickson
  2009-08-27 17:32                                 ` Chuck Lever
  2009-08-27 17:48                                 ` Trond Myklebust
  0 siblings, 2 replies; 32+ messages in thread
From: Steve Dickson @ 2009-08-27 14:14 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On 08/26/2009 03:59 PM, Trond Myklebust wrote:
> On Wed, 2009-08-26 at 15:50 -0400, Chuck Lever wrote:
>> Yeah, switching file system types in the mount(2) system call is the  
>> fly in the ointment.  I'm just wondering if Trond had some thoughts  
>> about making that more feasible.
> 
> Just look at what we're already doing for NFSv4. Inside nfs4_get_sb, we
> basically do a kernel mount in order to get the real super block. We
> then simply have to attach it to the vfsmount that the sys_mount() call
> passed down to us.
Well its not nfs4_get_sb() that would have to change its nfs_get_sb()
that would have to do an nfs4 mount after it discovered the -o vers=4.
It would get very messy very quickly for absolutely no reason since 
the propose mount patch is straightforward, it works and better yet
its done! ;-)  

> 
> This really isn't anything new or difficult...
Granted, mounting from the kernel is not new, but giving sys_mount()
on file system type which ends up mounting a complete different 
file system is new... Plus architecturally that just seems wrong...
A abit incestual... would you say! ;-)

I say we go with the proposed patch since its simple, architecturally
sound, will not cause problems down the road as long as nfs4 remains
a standalone file system and its done! Plus I have a patch waiting 
in the wings that actually does make v4 the first mount that is
tried... making v4 the default version... 

steved.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-27 14:14                               ` Steve Dickson
@ 2009-08-27 17:32                                 ` Chuck Lever
  2009-08-28  2:55                                   ` Steve Dickson
  2009-08-28 16:12                                   ` Christoph Hellwig
  2009-08-27 17:48                                 ` Trond Myklebust
  1 sibling, 2 replies; 32+ messages in thread
From: Chuck Lever @ 2009-08-27 17:32 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On Aug 27, 2009, at 10:14 AM, Steve Dickson wrote:
> On 08/26/2009 03:59 PM, Trond Myklebust wrote:
>> On Wed, 2009-08-26 at 15:50 -0400, Chuck Lever wrote:
>>> Yeah, switching file system types in the mount(2) system call is the
>>> fly in the ointment.  I'm just wondering if Trond had some thoughts
>>> about making that more feasible.
>>
>> Just look at what we're already doing for NFSv4. Inside  
>> nfs4_get_sb, we
>> basically do a kernel mount in order to get the real super block. We
>> then simply have to attach it to the vfsmount that the sys_mount()  
>> call
>> passed down to us.
> Well its not nfs4_get_sb() that would have to change its nfs_get_sb()
> that would have to do an nfs4 mount after it discovered the -o vers=4.
> It would get very messy very quickly for absolutely no reason since
> the propose mount patch is straightforward, it works and better yet
> its done! ;-)

Right now we are only speculating that doing this in the kernel will  
not be straightforward.  You say it will be unbearably ugly, and Trond  
says it should be simple.  He said - he said.  Why not try it and find  
out?

I hear your point that you want this done for RHEL 6.  I want IPv6  
done for RHEL 6, but that's looking less and less likely.  If this  
were a RHEL-only proposal, I'd be all over it.  But I'm concerned that  
going with the mount command solution will make our lives more  
challenging after RHEL 6 is come and gone.  It seems to me that  
upstream is less concerned with expediency and more concerned with  
good long term solutions.

I'm going to ask around about this.  If it really does look offensive  
to people, or technically infeasible, or will take a ridiculously long  
time to implement correctly, I'm OK with the mount command solution.   
I think we can afford to investigate a little more if we can find a  
solution that gets us farther down the road.  All I'm asking for is a  
little time to study the problem.

>> This really isn't anything new or difficult...
> Granted, mounting from the kernel is not new, but giving sys_mount()
> on file system type which ends up mounting a complete different
> file system is new... Plus architecturally that just seems wrong...
> A abit incestual... would you say! ;-)
>
> I say we go with the proposed patch since its simple, architecturally
> sound,

The whole point of text-based mounts is that we are supposed to be  
building up the NFS mount stuff in the kernel, closer to where all the  
client features are actually implemented, instead of in user space.   
It doesn't make sense to add logic in the mount command while our long  
term goal is to move it to the kernel, especially if we can find a  
viable alternative kernel implementation.

> will not cause problems down the road as long as nfs4 remains
> a standalone file system and its done!

We know for _sure_ that at some point nfs4 will likely no longer be  
visible to user space (or gone completely)...  so that last point is  
rather moot.  Doing it in the mount command _will_ increase mount  
command complexity down the road.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-27 14:14                               ` Steve Dickson
  2009-08-27 17:32                                 ` Chuck Lever
@ 2009-08-27 17:48                                 ` Trond Myklebust
  2009-08-27 17:58                                   ` Chuck Lever
  1 sibling, 1 reply; 32+ messages in thread
From: Trond Myklebust @ 2009-08-27 17:48 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On Thu, 2009-08-27 at 10:14 -0400, Steve Dickson wrote:
> I say we go with the proposed patch since its simple, architecturally
> sound, will not cause problems down the road as long as nfs4 remains
> a standalone file system and its done! Plus I have a patch waiting 
> in the wings that actually does make v4 the first mount that is
> tried... making v4 the default version... 

I do worry that if, at some point, we do get rid of the nfs4 filesystem
alias then we may find ourselves in trouble if we have a large base of
mount programs out there that translate '-t nfs -overs=4' into '-t
nfs4'.

I think that if you want to go down this path, you should somehow ensure
that the resulting program is capable of passing '-t nfs -overs=4' down
to the kernel (perhaps a configuration file option?).

Cheers
  Trond

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-27 17:48                                 ` Trond Myklebust
@ 2009-08-27 17:58                                   ` Chuck Lever
  2009-08-27 19:28                                     ` Steve Dickson
  0 siblings, 1 reply; 32+ messages in thread
From: Chuck Lever @ 2009-08-27 17:58 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list


On Aug 27, 2009, at 1:48 PM, Trond Myklebust wrote:

> On Thu, 2009-08-27 at 10:14 -0400, Steve Dickson wrote:
>> I say we go with the proposed patch since its simple, architecturally
>> sound, will not cause problems down the road as long as nfs4 remains
>> a standalone file system and its done! Plus I have a patch waiting
>> in the wings that actually does make v4 the first mount that is
>> tried... making v4 the default version...
>
> I do worry that if, at some point, we do get rid of the nfs4  
> filesystem
> alias then we may find ourselves in trouble if we have a large base of
> mount programs out there that translate '-t nfs -overs=4' into '-t
> nfs4'.
>
> I think that if you want to go down this path, you should somehow  
> ensure
> that the resulting program is capable of passing '-t nfs -overs=4'  
> down
> to the kernel (perhaps a configuration file option?).

My impression was that this mount command behavior would be controlled  
with a kernel version switch in mount.nfs, much the same way that we  
already handle switching between text-based and legacy mounting.  That  
won't help with old nfs-utils on new kernels, though.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-27 17:58                                   ` Chuck Lever
@ 2009-08-27 19:28                                     ` Steve Dickson
  0 siblings, 0 replies; 32+ messages in thread
From: Steve Dickson @ 2009-08-27 19:28 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list



On 08/27/2009 01:58 PM, Chuck Lever wrote:
> 
> On Aug 27, 2009, at 1:48 PM, Trond Myklebust wrote:
> 
>> On Thu, 2009-08-27 at 10:14 -0400, Steve Dickson wrote:
>>> I say we go with the proposed patch since its simple, architecturally
>>> sound, will not cause problems down the road as long as nfs4 remains
>>> a standalone file system and its done! Plus I have a patch waiting
>>> in the wings that actually does make v4 the first mount that is
>>> tried... making v4 the default version...
>>
>> I do worry that if, at some point, we do get rid of the nfs4 filesystem
>> alias then we may find ourselves in trouble if we have a large base of
>> mount programs out there that translate '-t nfs -overs=4' into '-t
>> nfs4'.
Well at that point, if that every and happens and if the mount command is the
only thing that has to change it would a minor miracle... IMHO... 

>>
>> I think that if you want to go down this path, you should somehow ensure
>> that the resulting program is capable of passing '-t nfs -overs=4' down
>> to the kernel (perhaps a configuration file option?).
> 
> My impression was that this mount command behavior would be controlled
> with a kernel version switch in mount.nfs, much the same way that we
> already handle switching between text-based and legacy mounting.  That
> won't help with old nfs-utils on new kernels, though.

Right... and I really don't want to  adding yet another configuration 
option for something that should be so simple... 

steved.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-27 17:32                                 ` Chuck Lever
@ 2009-08-28  2:55                                   ` Steve Dickson
  2009-08-28 16:12                                   ` Christoph Hellwig
  1 sibling, 0 replies; 32+ messages in thread
From: Steve Dickson @ 2009-08-28  2:55 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On 08/27/2009 01:32 PM, Chuck Lever wrote:
>>> Just look at what we're already doing for NFSv4. Inside nfs4_get_sb, we
>>> basically do a kernel mount in order to get the real super block. We
>>> then simply have to attach it to the vfsmount that the sys_mount() call
>>> passed down to us.
>> Well its not nfs4_get_sb() that would have to change its nfs_get_sb()
>> that would have to do an nfs4 mount after it discovered the -o vers=4.
>> It would get very messy very quickly for absolutely no reason since
>> the propose mount patch is straightforward, it works and better yet
>> its done! ;-)
> 
> Right now we are only speculating that doing this in the kernel will not
> be straightforward.  You say it will be unbearably ugly, and Trond says
> it should be simple.  He said - he said.  Why not try it and find out?
IMHO... adding a simple short cut to the mounting code is inherently simpler 
and less risky than changing kernel code...

> 
> I hear your point that you want this done for RHEL 6.  
Wrong! What I'm trying to do is move the Linux NFS implementation 
forward for ALL distros and more importantly for the community as a whole... 


> But I'm concerned that going with the mount command solution will make our 
> lives more challenging after RHEL 6 is come and gone.
Please.. Lets keep this in perspective.. My little short cut patch will
have absolutely no effect on our technology 2 to 3 years when RHEL 6
be comes active... I can speak from experience...   

>  It seems to me that upstream is less concerned with expediency and 
> more concerned with good long term solutions.
Expediency?? NFS v4 was first introduced in Fedora 2 which was released
in the middle of 2004... that's over five years ago... So I have to respectfully
disagree with the term expediency... 

> 
> I'm going to ask around about this.  If it really does look offensive to
> people, or technically infeasible, or will take a ridiculously long time
> to implement correctly, I'm OK with the mount command solution.  I think
> we can afford to investigate a little more if we can find a solution
> that gets us farther down the road.  All I'm asking for is a little time
> to study the problem.
To study what? All the patch does is make easier to mount a file system
that has been in the mainstream kernel since 2.6.5... Remember, people
do not have to use the "-o v4' option... the '-t nfs4' option will 
continue to work... 
 
> 
>>> This really isn't anything new or difficult...
>> Granted, mounting from the kernel is not new, but giving sys_mount()
>> on file system type which ends up mounting a complete different
>> file system is new... Plus architecturally that just seems wrong...
>> A abit incestual... would you say! ;-)
>>
>> I say we go with the proposed patch since its simple, architecturally
>> sound,
> 
> The whole point of text-based mounts is that we are supposed to be
> building up the NFS mount stuff in the kernel, closer to where all the
> client features are actually implemented, instead of in user space.  It
> doesn't make sense to add logic in the mount command while our long term
> goal is to move it to the kernel, especially if we can find a viable
> alternative kernel implementation.
Which is a bit ironic... It appears to me that the rest of the 
Linux kernel community is actually moving things out of the kernel which
actually makes sense IMHO... Just because we adopted the Solaris way of 
moving the parsing of mount options into the kernel does not mean its
the right thing to do... And in the end... goals do change... 

> 
>> will not cause problems down the road as long as nfs4 remains
>> a standalone file system and its done!
> 
> We know for _sure_ that at some point nfs4 will likely no longer be
> visible to user space (or gone completely)...  so that last point is
> rather moot.  Doing it in the mount command _will_ increase mount
> command complexity down the road.
I can't disagree with this more... All this patch does make it easier to
mount an *existing* file system... Now if that file system goes away,
so be it.. changes will have to make... and the least of our problems
will removing this simple short cut to mount that file system.

At the end of the day..  'mount -o v4' will *always* work, whether 
the nfs4 file system is or is not visible from user space... 

If or when the kernel no longer support a nfs4 file system, we will *have* to
change the mount version since mount.nfs4 will no longer be valid and
we will have to put in some safeguards to be backwards compatible...
We have done it in the past and will have to do it again... The proposed
patch does not change that fact whatsoever... 

So I see absolutely no reason not to make it easier for our community 
to use the technology we have worked so hard on, for so long, now.
And that's exactly what this patch does... 

steved.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-27 17:32                                 ` Chuck Lever
  2009-08-28  2:55                                   ` Steve Dickson
@ 2009-08-28 16:12                                   ` Christoph Hellwig
  2009-08-28 16:35                                     ` Steve Dickson
  1 sibling, 1 reply; 32+ messages in thread
From: Christoph Hellwig @ 2009-08-28 16:12 UTC (permalink / raw)
  To: Chuck Lever
  Cc: Steve Dickson, Linux NFS Mailing list, Linux NFSv4 mailing list

On Thu, Aug 27, 2009 at 01:32:09PM -0400, Chuck Lever wrote:
>>>> about making that more feasible.
>>>
>>> Just look at what we're already doing for NFSv4. Inside nfs4_get_sb, 
>>> we
>>> basically do a kernel mount in order to get the real super block. We
>>> then simply have to attach it to the vfsmount that the sys_mount()  
>>> call
>>> passed down to us.
>> Well its not nfs4_get_sb() that would have to change its nfs_get_sb()
>> that would have to do an nfs4 mount after it discovered the -o vers=4.
>> It would get very messy very quickly for absolutely no reason since
>> the propose mount patch is straightforward, it works and better yet
>> its done! ;-)
>
> Right now we are only speculating that doing this in the kernel will not 
> be straightforward.  You say it will be unbearably ugly, and Trond says 
> it should be simple.  He said - he said.  Why not try it and find out?

It's actually trivial.  The file_system_type does not have much meaning
at all, it contains very little information:

 - the implementation module - this is obviously the same for nfs and
   nfs4
 - the name - we actually want nfs for v4 here, that's the point.
 - the get_sb method - we want to call the v4 method after initial
   option parsing.  This is totally trivial if done as a quick hack,
   but some refactoring to share code might be a better idea
 - the kill_sb method - we just need a flag to chose which one.
   Again a bit of refactoring there might not be a bad idea here either:
   e.g. why does nfs_kill_super do a bdi_unregister but nfs4_kill_super
   not.
 - the flags in fs_flags - fortunately the same for nfs and nfs4.
 - a list entry for the list of register filesystems.  That list is not
   used much:

     - for printing the list of filesystems in /proc/filesystems
     - for finding the filesystem by name using get_fs_type, mostly
       for mounting
     - for the whacko sysfs system call needing an index into this
       list.

So doing this really is easy.  And if done properly it might actually
clean the code up, too.

>
> I hear your point that you want this done for RHEL 6.  I want IPv6 done 
> for RHEL 6, but that's looking less and less likely.  If this were a 
> RHEL-only proposal, I'd be all over it.  But I'm concerned that going 
> with the mount command solution will make our lives more challenging 
> after RHEL 6 is come and gone.  It seems to me that upstream is less 
> concerned with expediency and more concerned with good long term 
> solutions.
>
> I'm going to ask around about this.  If it really does look offensive to 
> people, or technically infeasible, or will take a ridiculously long time 
> to implement correctly, I'm OK with the mount command solution.  I think 
> we can afford to investigate a little more if we can find a solution that 
> gets us farther down the road.  All I'm asking for is a little time to 
> study the problem.
>
>>> This really isn't anything new or difficult...
>> Granted, mounting from the kernel is not new, but giving sys_mount()
>> on file system type which ends up mounting a complete different
>> file system is new... Plus architecturally that just seems wrong...
>> A abit incestual... would you say! ;-)
>>
>> I say we go with the proposed patch since its simple, architecturally
>> sound,
>
> The whole point of text-based mounts is that we are supposed to be  
> building up the NFS mount stuff in the kernel, closer to where all the  
> client features are actually implemented, instead of in user space.  It 
> doesn't make sense to add logic in the mount command while our long term 
> goal is to move it to the kernel, especially if we can find a viable 
> alternative kernel implementation.
>
>> will not cause problems down the road as long as nfs4 remains
>> a standalone file system and its done!
>
> We know for _sure_ that at some point nfs4 will likely no longer be  
> visible to user space (or gone completely)...  so that last point is  
> rather moot.  Doing it in the mount command _will_ increase mount  
> command complexity down the road.
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>
> _______________________________________________
> NFSv4 mailing list
> NFSv4@linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/nfsv4
---end quoted text---

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-28 16:12                                   ` Christoph Hellwig
@ 2009-08-28 16:35                                     ` Steve Dickson
       [not found]                                       ` <4A980751.7070206-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 32+ messages in thread
From: Steve Dickson @ 2009-08-28 16:35 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list



On 08/28/2009 12:12 PM, Christoph Hellwig wrote:
> On Thu, Aug 27, 2009 at 01:32:09PM -0400, Chuck Lever wrote:
>>>>> about making that more feasible.
>>>>
>>>> Just look at what we're already doing for NFSv4. Inside nfs4_get_sb, 
>>>> we
>>>> basically do a kernel mount in order to get the real super block. We
>>>> then simply have to attach it to the vfsmount that the sys_mount()  
>>>> call
>>>> passed down to us.
>>> Well its not nfs4_get_sb() that would have to change its nfs_get_sb()
>>> that would have to do an nfs4 mount after it discovered the -o vers=4.
>>> It would get very messy very quickly for absolutely no reason since
>>> the propose mount patch is straightforward, it works and better yet
>>> its done! ;-)
>>
>> Right now we are only speculating that doing this in the kernel will not 
>> be straightforward.  You say it will be unbearably ugly, and Trond says 
>> it should be simple.  He said - he said.  Why not try it and find out?
> 
> It's actually trivial.  The file_system_type does not have much meaning
> at all, it contains very little information:
Question... How are will it be to back out of this? Meaning the
kernel tries a v4 mount only to see the server does not support v4
or for some other recoverable error and then has to restart back
down the mount NFS again... This is very fairly trivial in user space...  

steved.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
       [not found]                                       ` <4A980751.7070206-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
@ 2009-08-28 16:41                                         ` Christoph Hellwig
  2009-08-28 16:44                                           ` Chuck Lever
  2009-08-28 16:53                                           ` Steve Dickson
  0 siblings, 2 replies; 32+ messages in thread
From: Christoph Hellwig @ 2009-08-28 16:41 UTC (permalink / raw)
  To: Steve Dickson
  Cc: Christoph Hellwig, Chuck Lever, Linux NFS Mailing list,
	Linux NFSv4 mailing list

On Fri, Aug 28, 2009 at 12:35:29PM -0400, Steve Dickson wrote:
> Question... How are will it be to back out of this? Meaning the
> kernel tries a v4 mount only to see the server does not support v4
> or for some other recoverable error and then has to restart back
> down the mount NFS again... This is very fairly trivial in user space...  

Same as we do now?  As soon as it doesn't work return an error, unwind
and return to userspace?


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-28 16:41                                         ` Christoph Hellwig
@ 2009-08-28 16:44                                           ` Chuck Lever
  2009-08-28 16:53                                           ` Steve Dickson
  1 sibling, 0 replies; 32+ messages in thread
From: Chuck Lever @ 2009-08-28 16:44 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On Aug 28, 2009, at 12:41 PM, Christoph Hellwig wrote:
> On Fri, Aug 28, 2009 at 12:35:29PM -0400, Steve Dickson wrote:
>> Question... How are will it be to back out of this? Meaning the
>> kernel tries a v4 mount only to see the server does not support v4
>> or for some other recoverable error and then has to restart back
>> down the mount NFS again... This is very fairly trivial in user  
>> space...
>
> Same as we do now?  As soon as it doesn't work return an error, unwind
> and return to userspace?

If we handle NFSv4 in nfs_get_sb(), then we can handle version/ 
transport negotiation entirely in the kernel, which is another long  
term goal.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-28 16:41                                         ` Christoph Hellwig
  2009-08-28 16:44                                           ` Chuck Lever
@ 2009-08-28 16:53                                           ` Steve Dickson
  2009-08-28 16:59                                             ` Christoph Hellwig
  1 sibling, 1 reply; 32+ messages in thread
From: Steve Dickson @ 2009-08-28 16:53 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list



On 08/28/2009 12:41 PM, Christoph Hellwig wrote:
> On Fri, Aug 28, 2009 at 12:35:29PM -0400, Steve Dickson wrote:
>> Question... How are will it be to back out of this? Meaning the
>> kernel tries a v4 mount only to see the server does not support v4
>> or for some other recoverable error and then has to restart back
>> down the mount NFS again... This is very fairly trivial in user space...  
> 
> Same as we do now?  
Yes... 

> As soon as it doesn't work return an error, unwind
> and return to userspace?
Then why not simply stay in user space?

steved.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-28 16:53                                           ` Steve Dickson
@ 2009-08-28 16:59                                             ` Christoph Hellwig
  2009-08-28 17:19                                               ` Steve Dickson
  0 siblings, 1 reply; 32+ messages in thread
From: Christoph Hellwig @ 2009-08-28 16:59 UTC (permalink / raw)
  To: Steve Dickson
  Cc: Christoph Hellwig, Linux NFS Mailing list, Linux NFSv4 mailing list

On Fri, Aug 28, 2009 at 12:53:01PM -0400, Steve Dickson wrote:
> > As soon as it doesn't work return an error, unwind
> > and return to userspace?
> Then why not simply stay in user space?

I just told you the merging the two file_system_type implementations
(actually more than two, for some reason nfs alone has more than all
other network filesystems combined) is easy and beneficial for the
quality of the kernel code.  If there's something in the behaviour
of the userspace mount helpers not actually making it useful it's
up to you to decide if you want to do it or not.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)
  2009-08-28 16:59                                             ` Christoph Hellwig
@ 2009-08-28 17:19                                               ` Steve Dickson
  0 siblings, 0 replies; 32+ messages in thread
From: Steve Dickson @ 2009-08-28 17:19 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list



On 08/28/2009 12:59 PM, Christoph Hellwig wrote:
> On Fri, Aug 28, 2009 at 12:53:01PM -0400, Steve Dickson wrote:
>>> As soon as it doesn't work return an error, unwind
>>> and return to userspace?
>> Then why not simply stay in user space?
> 
> I just told you the merging the two file_system_type implementations
> (actually more than two, for some reason nfs alone has more than all
> other network filesystems combined) is easy and beneficial for the
> quality of the kernel code.  If there's something in the behaviour
> of the userspace mount helpers not actually making it useful it's
> up to you to decide if you want to do it or not.
> 
Point taken... merging file systems is not hard... but that 
was not the point of this patch... the point of this patch 
was to make it easier (or more straightforward) to mount 
the existing nfs4 file system... nothing more and nothing less...

Personally I see no reason to merge file systems especially 
just to make it easier to mount one of them..
 
steved.

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2009-08-28 17:19 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-25 17:52 [PATCH 0/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02) Steve Dickson
2009-08-25 17:54 ` [PATCH 1/2] " Steve Dickson
     [not found] ` <4A9424DB.2040303-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
2009-08-25 17:55   ` [PATCH 2/2] " Steve Dickson
2009-08-25 18:59     ` Chuck Lever
2009-08-25 19:18       ` Steve Dickson
2009-08-25 19:32         ` Chuck Lever
2009-08-25 20:15           ` Steve Dickson
2009-08-25 20:37             ` Chuck Lever
2009-08-26 12:10               ` Steve Dickson
2009-08-25 20:49             ` Trond Myklebust
2009-08-26 12:28               ` Steve Dickson
2009-08-26 14:20               ` Chuck Lever
2009-08-26 15:07                 ` Steve Dickson
2009-08-26 16:35                   ` Chuck Lever
2009-08-26 17:08                     ` Steve Dickson
2009-08-26 17:22                       ` Chuck Lever
2009-08-26 17:51                         ` Steve Dickson
2009-08-26 19:50                           ` Chuck Lever
2009-08-26 19:59                             ` Trond Myklebust
2009-08-27 14:14                               ` Steve Dickson
2009-08-27 17:32                                 ` Chuck Lever
2009-08-28  2:55                                   ` Steve Dickson
2009-08-28 16:12                                   ` Christoph Hellwig
2009-08-28 16:35                                     ` Steve Dickson
     [not found]                                       ` <4A980751.7070206-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
2009-08-28 16:41                                         ` Christoph Hellwig
2009-08-28 16:44                                           ` Chuck Lever
2009-08-28 16:53                                           ` Steve Dickson
2009-08-28 16:59                                             ` Christoph Hellwig
2009-08-28 17:19                                               ` Steve Dickson
2009-08-27 17:48                                 ` Trond Myklebust
2009-08-27 17:58                                   ` Chuck Lever
2009-08-27 19:28                                     ` Steve Dickson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.