2009-08-25 17:55:33

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

commit 1471d23d692efc7388794a8a3c3b9e548d1c5be8
Author: Steve Dickson <[email protected]>
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 <[email protected]>

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 */



2009-08-26 12:10:02

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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.

2009-08-26 12:28:26

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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 <[email protected]>
>>>>>> 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.

2009-08-26 14:20:44

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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 <[email protected]>
>>>>>> 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

2009-08-26 15:07:43

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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 <[email protected]>
>>>>>>> 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.

2009-08-26 16:35:17

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)


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 <[email protected]>
>>>>>>>> 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

2009-08-26 17:08:02

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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.

2009-08-26 17:22:30

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)


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

2009-08-26 17:51:08

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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.

2009-08-26 19:50:21

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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

2009-08-26 19:59:34

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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...



2009-08-27 14:14:42

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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.

2009-08-27 17:32:09

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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

2009-08-27 17:48:44

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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

2009-08-27 17:58:57

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)


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

2009-08-27 19:28:25

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)



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.

2009-08-28 02:55:16

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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.

2009-08-28 16:12:57

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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
> [email protected]
> http://linux-nfs.org/cgi-bin/mailman/listinfo/nfsv4
---end quoted text---

2009-08-28 16:35:29

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)



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.

2009-08-25 18:59:12

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

On Aug 25, 2009, at 1:55 PM, Steve Dickson wrote:
> commit 1471d23d692efc7388794a8a3c3b9e548d1c5be8
> Author: Steve Dickson <[email protected]>
> 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 <[email protected]>
>
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

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

2009-08-25 19:18:44

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)



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 <[email protected]>
>> 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.

2009-08-25 19:32:36

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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 <[email protected]>
>>> 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

2009-08-25 20:15:01

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)



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 <[email protected]>
>>>> 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.

2009-08-25 20:37:52

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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 <[email protected]>
>>>>> 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

2009-08-25 20:49:05

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 2/2] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set (vers-02)

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 <[email protected]>
> >>>> 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