All of lore.kernel.org
 help / color / mirror / Atom feed
* Allow vers=4 and nfsvers=4 mount options
@ 2009-05-05 21:46 Kevin Constantine
       [not found] ` <1241560004-16787-1-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Kevin Constantine @ 2009-05-05 21:46 UTC (permalink / raw)
  To: linux-nfs

I've run into a problem recently while trying to set up an environment for testing
nfsv4.  In our environment, we have a mixture of Linux, and OSX, and all of our mounts
are made via automount maps stored in LDAP which are shared among the platforms.  The
problem is made even more complicated by the fact that some of our file servers
support v4, while others do not.  This means that v4 mounts need to be specified on
a mount by mount basis, and only on certain test machines.

With nfsv4, Linux appears to use fstype=nfs4 to distinguish between v2,3 and v4, and
attempts to specify vers=4, or nfsvers=4 error.  OSX doesn't understand fstype
and mounts error when this option is encountered.  OSX instead relies on
vers=2,3,4 or nfsvers=2,3,4 to determine the version.  It appears that Solaris will
likely behave the same way as OSX based on the manpage, though I haven't tested it.

I've written the following patches to allow vers=4, and nfsvers=4 to be passed to
the mount command.  There's a patch for the text-based mount, and a second for the
older style mount options.  In both cases, I attempt to detect the vers= option
as early as possible in order to set fstype=nfs4, and then let the code continue on
as though the user had specified fstype=nfs4 on the command line.

Using these patches, I can now specify something like:
pamtest4  -rw,intr,hard,timeo=600$RSZ,vers=$NFSVERS     supportsv4:/vol/someexport
pamtest3  -rw,intr,hard,timeo=600$RSZ                   supportsv3only:/vol/v3export
in the automount map.  The variable $NFSVERS is set to 4 on the test clients, and 3
on production Linux machines, and OSX.

[PATCH 1/2] Allow nfs/vers=4 option in old style mount commands
[PATCH 2/2] Allow nfs/vers=4 option in text-based mount commands

-kevin

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

* [PATCH 1/2] Allow nfs/vers=4 option in old style mount commands
       [not found] ` <1241560004-16787-1-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
@ 2009-05-05 21:46   ` Kevin Constantine
       [not found]     ` <1241560004-16787-2-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Kevin Constantine @ 2009-05-05 21:46 UTC (permalink / raw)
  To: linux-nfs; +Cc: Kevin Constantine

This allows a user to specify nfsvers=4, or vers=4 on the mount
commandline and have mount.nfs4 called as though fstype=nfs4 were
specified.  This patch handles the nfs4mount, and nfsmount cases in
try_mount().

Added the function find_rightmost() to return the rightmost option in the
extra_opts string.  If vers=4 is overridden later in the string by
vers=3, we need to honor that.

Signed-off-by: Kevin Constantine <kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
---
 utils/mount/mount.c |   41 ++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/utils/mount/mount.c b/utils/mount/mount.c
index 06e2804..4cebc0f 100644
--- a/utils/mount/mount.c
+++ b/utils/mount/mount.c
@@ -444,11 +444,38 @@ static int try_mount(char *spec, char *mount_point, int flags,
 	return ret;
 }
 
+static void find_rightmost(char *extra_opts, char *search_opt, char **rightmost_opt)
+{
+	char *opts = xstrdup(extra_opts);
+	char *right_opt = NULL, *token = NULL;
+	int i = 0;
+
+	for (;;) {
+		if (i == 0) {
+			token = strtok(opts, ",");
+			i++;
+		}
+		else 
+			token = strtok(NULL, ",");
+
+		if (token == NULL) 
+			break;
+		if (! strncmp(token, search_opt, strlen(search_opt))) 
+			right_opt = xstrdup(token);
+	}
+	if (right_opt != NULL) {
+		*rightmost_opt = xmalloc(strlen(right_opt));
+		strcpy(*rightmost_opt, right_opt);
+	}
+	free(opts);
+	free(right_opt);
+}
+
 int main(int argc, char *argv[])
 {
 	int c, flags = 0, mnt_err = 1, fake = 0;
 	char *spec, *mount_point, *fs_type = "nfs";
-	char *extra_opts = NULL, *mount_opts = NULL;
+	char *extra_opts = NULL, *mount_opts = NULL, *vers_opt=NULL;
 	uid_t uid = getuid();
 
 	progname = basename(argv[0]);
@@ -562,6 +589,18 @@ int main(int argc, char *argv[])
 
 	parse_opts(mount_opts, &flags, &extra_opts);
 
+	find_rightmost(extra_opts, "vers", &vers_opt);
+	if (vers_opt != NULL) {
+		if ((strcmp(vers_opt, "vers=4")) == 0) 
+			fs_type = "nfs4";
+	}
+
+	find_rightmost(extra_opts, "nfsvers", &vers_opt);
+	if (vers_opt != NULL) {
+		if ((strcmp(vers_opt, "nfsvers=4")) == 0) 
+			fs_type = "nfs4";
+	}
+
 	if (uid != 0) {
 		if (!(flags & (MS_USERS|MS_USER))) {
 			nfs_error(_("%s: permission denied"), progname);
-- 
1.6.2.1


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

* [PATCH 2/2] Allow nfs/vers=4 option in text-based mount commands
       [not found]     ` <1241560004-16787-2-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
@ 2009-05-05 21:46       ` Kevin Constantine
       [not found]         ` <1241560004-16787-3-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
  2009-05-05 22:08       ` [PATCH 1/2] Allow nfs/vers=4 option in old style " Chuck Lever
  1 sibling, 1 reply; 7+ messages in thread
From: Kevin Constantine @ 2009-05-05 21:46 UTC (permalink / raw)
  To: linux-nfs; +Cc: Kevin Constantine

This allows a user to specify nfsvers=4, or vers=4 on the mount
commandline and have mount.nfs4 called as though fstype=nfs4 were
specified.  This patch handles the nfsmount_string case in
mount.c's try_mount().

We get the value of the "vers=" or "nfsvers=" from the nfsmount_info
structure, and if the value equals 4, we set the fstype to nfs4, and
remove the nfsvers/vers options from the structure since it shouldn't
be there in the first place, and we don't want to pass it along down
the stack.

po_get_numeric returns the rightmost instance, so we honor the last
value of nfsvers/vers in the event that it is overridden later in the
options string.

Signed-off-by: Kevin Constantine <kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
---
 utils/mount/stropts.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index c369136..72b0d13 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -754,6 +754,15 @@ static const char *nfs_background_opttbl[] = {
 
 static int nfsmount_start(struct nfsmount_info *mi)
 {
+	long tmp;
+	po_get_numeric(mi->options, "vers", &tmp);
+	po_get_numeric(mi->options, "nfsvers", &tmp);
+	if (tmp == 4) {
+		mi->type = "nfs4";
+		po_remove_all(mi->options, "vers");
+		po_remove_all(mi->options, "nfsvers");
+	}
+
 	if (!nfs_validate_options(mi))
 		return EX_FAIL;
 
-- 
1.6.2.1


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

* Re: [PATCH 2/2] Allow nfs/vers=4 option in text-based mount commands
       [not found]         ` <1241560004-16787-3-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
@ 2009-05-05 22:02           ` Chuck Lever
  2009-05-07 20:03             ` Kevin Constantine
  0 siblings, 1 reply; 7+ messages in thread
From: Chuck Lever @ 2009-05-05 22:02 UTC (permalink / raw)
  To: Kevin Constantine; +Cc: Linux NFS Mailing List, Kevin Constantine


On May 5, 2009, at 5:46 PM, Kevin Constantine wrote:

> This allows a user to specify nfsvers=4, or vers=4 on the mount
> commandline and have mount.nfs4 called as though fstype=nfs4 were
> specified.  This patch handles the nfsmount_string case in
> mount.c's try_mount().
>
> We get the value of the "vers=" or "nfsvers=" from the nfsmount_info
> structure, and if the value equals 4, we set the fstype to nfs4, and
> remove the nfsvers/vers options from the structure since it shouldn't
> be there in the first place, and we don't want to pass it along down
> the stack.
>
> po_get_numeric returns the rightmost instance, so we honor the last
> value of nfsvers/vers in the event that it is overridden later in the
> options string.
>
> Signed-off-by: Kevin Constantine <kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org 
> >
> ---
> utils/mount/stropts.c |    9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index c369136..72b0d13 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -754,6 +754,15 @@ static const char *nfs_background_opttbl[] = {
>
> static int nfsmount_start(struct nfsmount_info *mi)
> {
> +	long tmp;
> +	po_get_numeric(mi->options, "vers", &tmp);
> +	po_get_numeric(mi->options, "nfsvers", &tmp);

If someone specifies both a vers= and a nfsvers= on the command line,  
this won't handle it.  You need to implement a rightmost search, as  
Steve's patch (posted previously on this list) did.

> +	if (tmp == 4) {
> +		mi->type = "nfs4";
> +		po_remove_all(mi->options, "vers");
> +		po_remove_all(mi->options, "nfsvers");
> +	}
> +
> 	if (!nfs_validate_options(mi))
> 		return EX_FAIL;
>
> -- 
> 1.6.2.1
>
> --
> 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] 7+ messages in thread

* Re: [PATCH 1/2] Allow nfs/vers=4 option in old style mount commands
       [not found]     ` <1241560004-16787-2-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
  2009-05-05 21:46       ` [PATCH 2/2] Allow nfs/vers=4 option in text-based " Kevin Constantine
@ 2009-05-05 22:08       ` Chuck Lever
  1 sibling, 0 replies; 7+ messages in thread
From: Chuck Lever @ 2009-05-05 22:08 UTC (permalink / raw)
  To: Kevin Constantine; +Cc: linux-nfs, Kevin Constantine

On May 5, 2009, at 5:46 PM, Kevin Constantine wrote:

> This allows a user to specify nfsvers=4, or vers=4 on the mount
> commandline and have mount.nfs4 called as though fstype=nfs4 were
> specified.  This patch handles the nfs4mount, and nfsmount cases in
> try_mount().
>
> Added the function find_rightmost() to return the rightmost option  
> in the
> extra_opts string.  If vers=4 is overridden later in the string by
> vers=3, we need to honor that.
>
> Signed-off-by: Kevin Constantine <kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org 
> >
> ---
> utils/mount/mount.c |   41 ++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 40 insertions(+), 1 deletions(-)
>
> diff --git a/utils/mount/mount.c b/utils/mount/mount.c
> index 06e2804..4cebc0f 100644
> --- a/utils/mount/mount.c
> +++ b/utils/mount/mount.c
> @@ -444,11 +444,38 @@ static int try_mount(char *spec, char  
> *mount_point, int flags,
> 	return ret;
> }
>
> +static void find_rightmost(char *extra_opts, char *search_opt, char  
> **rightmost_opt)
> +{
> +	char *opts = xstrdup(extra_opts);
> +	char *right_opt = NULL, *token = NULL;
> +	int i = 0;
> +
> +	for (;;) {
> +		if (i == 0) {
> +			token = strtok(opts, ",");
> +			i++;
> +		}
> +		else
> +			token = strtok(NULL, ",");
> +
> +		if (token == NULL)
> +			break;
> +		if (! strncmp(token, search_opt, strlen(search_opt)))
> +			right_opt = xstrdup(token);
> +	}
> +	if (right_opt != NULL) {
> +		*rightmost_opt = xmalloc(strlen(right_opt));
> +		strcpy(*rightmost_opt, right_opt);
> +	}
> +	free(opts);
> +	free(right_opt);
> +}
> +
> int main(int argc, char *argv[])
> {
> 	int c, flags = 0, mnt_err = 1, fake = 0;
> 	char *spec, *mount_point, *fs_type = "nfs";
> -	char *extra_opts = NULL, *mount_opts = NULL;
> +	char *extra_opts = NULL, *mount_opts = NULL, *vers_opt=NULL;
> 	uid_t uid = getuid();
>
> 	progname = basename(argv[0]);
> @@ -562,6 +589,18 @@ int main(int argc, char *argv[])
>
> 	parse_opts(mount_opts, &flags, &extra_opts);
>
> +	find_rightmost(extra_opts, "vers", &vers_opt);

Calling this here means it will affect both text-based and non-text- 
based mounts.  A better place for it would be in try_mount(), as  
Steve's previous posted patch did.

Do you really need this support on legacy kernels?  Parsing this  
properly for the binary case will be a challenge.

> +	if (vers_opt != NULL) {
> +		if ((strcmp(vers_opt, "vers=4")) == 0)
> +			fs_type = "nfs4";
> +	}
> +
> +	find_rightmost(extra_opts, "nfsvers", &vers_opt);
> +	if (vers_opt != NULL) {
> +		if ((strcmp(vers_opt, "nfsvers=4")) == 0)
> +			fs_type = "nfs4";
> +	}
> +
>
> 	if (uid != 0) {
> 		if (!(flags & (MS_USERS|MS_USER))) {
> 			nfs_error(_("%s: permission denied"), progname);

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

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

* Re: [PATCH 2/2] Allow nfs/vers=4 option in text-based mount commands
  2009-05-05 22:02           ` Chuck Lever
@ 2009-05-07 20:03             ` Kevin Constantine
       [not found]               ` <4A033E7C.9060605-P5ys19MLBK/QT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Kevin Constantine @ 2009-05-07 20:03 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing List, Steve Dickson

Chuck Lever wrote:
> 
> On May 5, 2009, at 5:46 PM, Kevin Constantine wrote:
> 
>> This allows a user to specify nfsvers=4, or vers=4 on the mount
>> commandline and have mount.nfs4 called as though fstype=nfs4 were
>> specified.  This patch handles the nfsmount_string case in
>> mount.c's try_mount().
>>
>> We get the value of the "vers=" or "nfsvers=" from the nfsmount_info
>> structure, and if the value equals 4, we set the fstype to nfs4, and
>> remove the nfsvers/vers options from the structure since it shouldn't
>> be there in the first place, and we don't want to pass it along down
>> the stack.
>>
>> po_get_numeric returns the rightmost instance, so we honor the last
>> value of nfsvers/vers in the event that it is overridden later in the
>> options string.
>>
>> Signed-off-by: Kevin Constantine <kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
>> ---
>> utils/mount/stropts.c |    9 +++++++++
>> 1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>> index c369136..72b0d13 100644
>> --- a/utils/mount/stropts.c
>> +++ b/utils/mount/stropts.c
>> @@ -754,6 +754,15 @@ static const char *nfs_background_opttbl[] = {
>>
>> static int nfsmount_start(struct nfsmount_info *mi)
>> {
>> +    long tmp;
>> +    po_get_numeric(mi->options, "vers", &tmp);
>> +    po_get_numeric(mi->options, "nfsvers", &tmp);
> 
> If someone specifies both a vers= and a nfsvers= on the command line, 
> this won't handle it.  You need to implement a rightmost search, as 
> Steve's patch (posted previously on this list) did.
> 

I can't seem to find Steve's patches relating to this issue.  I'm 
assuming that since there are already patches related to this, it's not 
worth pursuing these particular patches.

Let me know

-kevin

>> +    if (tmp == 4) {
>> +        mi->type = "nfs4";
>> +        po_remove_all(mi->options, "vers");
>> +        po_remove_all(mi->options, "nfsvers");
>> +    }
>> +
>>     if (!nfs_validate_options(mi))
>>         return EX_FAIL;
>>
>> -- 
>> 1.6.2.1
>>
>> -- 
>> 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
> 

-- 
------------------------------------------------------------
Kevin Constantine

Systems Engineer		t: 818.460.8221
Walt Disney Animation Studios	e: kevin.constantine-P5ys19MLBK/QT0dZR+AlfA@public.gmane.org

Any sufficiently advanced technology is indistinguishable from magic.
     - Arthur C. Clarke

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

* Re: [PATCH 2/2] Allow nfs/vers=4 option in text-based mount commands
       [not found]               ` <4A033E7C.9060605-P5ys19MLBK/QT0dZR+AlfA@public.gmane.org>
@ 2009-05-07 20:47                 ` Chuck Lever
  0 siblings, 0 replies; 7+ messages in thread
From: Chuck Lever @ 2009-05-07 20:47 UTC (permalink / raw)
  To: Kevin Constantine; +Cc: Linux NFS Mailing List, Steve Dickson

[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]


On May 7, 2009, at 4:03 PM, Kevin Constantine wrote:

> Chuck Lever wrote:
>> On May 5, 2009, at 5:46 PM, Kevin Constantine wrote:
>>> This allows a user to specify nfsvers=4, or vers=4 on the mount
>>> commandline and have mount.nfs4 called as though fstype=nfs4 were
>>> specified.  This patch handles the nfsmount_string case in
>>> mount.c's try_mount().
>>>
>>> We get the value of the "vers=" or "nfsvers=" from the nfsmount_info
>>> structure, and if the value equals 4, we set the fstype to nfs4, and
>>> remove the nfsvers/vers options from the structure since it  
>>> shouldn't
>>> be there in the first place, and we don't want to pass it along down
>>> the stack.
>>>
>>> po_get_numeric returns the rightmost instance, so we honor the last
>>> value of nfsvers/vers in the event that it is overridden later in  
>>> the
>>> options string.
>>>
>>> Signed-off-by: Kevin Constantine <kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org 
>>> >
>>> ---
>>> utils/mount/stropts.c |    9 +++++++++
>>> 1 files changed, 9 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>>> index c369136..72b0d13 100644
>>> --- a/utils/mount/stropts.c
>>> +++ b/utils/mount/stropts.c
>>> @@ -754,6 +754,15 @@ static const char *nfs_background_opttbl[] = {
>>>
>>> static int nfsmount_start(struct nfsmount_info *mi)
>>> {
>>> +    long tmp;
>>> +    po_get_numeric(mi->options, "vers", &tmp);
>>> +    po_get_numeric(mi->options, "nfsvers", &tmp);
>> If someone specifies both a vers= and a nfsvers= on the command  
>> line, this won't handle it.  You need to implement a rightmost  
>> search, as Steve's patch (posted previously on this list) did.
>
> I can't seem to find Steve's patches relating to this issue.  I'm  
> assuming that since there are already patches related to this, it's  
> not worth pursuing these particular patches.

It was on the NFSv4 mailing list.  Sorry about that.


[-- Attachment #2: Re: [PATCH] [RFC] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set..eml --]
[-- Type: message/rfc822, Size: 10492 bytes --]

From: Chuck Lever <chuck.lever@oracle.com>
To: Steve Dickson <SteveD@redhat.com>
Cc: Linux NFSv4 mailing list <nfsv4@linux-nfs.org>
Subject: Re: [PATCH] [RFC] Enable v4 mounts when either "nfsvers=4" or "vers=4" option are set.
Date: Tue, 24 Mar 2009 13:02:09 -0400
Message-ID: <8B1CA12E-5B35-4DA1-B48C-0916D27CD9AB@oracle.com>

Hi Steve-

Yep, this is the idea.  Comments below.

On Mar 23, 2009, at 12:16 PM, Steve Dickson wrote:
> Chuck Lever wrote:
>> On Mar 20, 2009, at Mar 20, 2009, 1:44 PM, Steve Dickson wrote:
>>> Chuck Lever wrote:
>>>> Hi Steve-
>>>>
>>>> You could limit this to just text-based mounts, then the existing  
>>>> option
>>>> parser would handle most of this for you.
>>>>
>>> I took a look at that approach... Basically have parse_opts() and/or
>>> parse_opt() do the work figuring out what nfsvers or vers is set to.
>>> But the problem with that, I would have to pass down the address of
>>> fs_type pointer so I could overwritten... Adding that extra  
>>> parameter
>>> I didn't think was the best approach... bordering on the hacky  
>>> side...
>>>
>>> So took the approach of lets set file system type once and only  
>>> once...
>>> Unfortunately that approach does call for me to strip out the  
>>> "nfsvers"
>>> or "vers" string from that options string...
>>
>> The text-based path uses a single data structure, nfs_mountinfo,  
>> which
>> carries all the mount data through the whole code path.  In
>> nfsmount_start() you can add a little logic that resets the fs_type  
>> (in
>> the .type field) before proceeding with the rest of the mount.
>>
>> You would also need to expose nfs_nfs_version() in utils/mount/ 
>> network.c
>> (and add support for version 4) so you can handle the case where  
>> someone
>> specifies "vers=" more than once on the command line (or specifies
>> something obnoxious like "v2,nfsvers=4,vers=3", and you can also  
>> detect
>> "v4" here as well.
>>
>> There is code at the top of nfs_construct_new_options() which  
>> provides
>> an example of how to easily strip out the existing vers options.
>>
>> So nfsmount_start() could call nfs_nfs_version to see what version to
>> use if the passed-in fs_type is "nfs", and if it's version 4, just
>> remove all the vers options, and say 'mi.type = "nfs4";'.
>>
>> All that assumes you don't care about providing nfsvers=4 support for
>> legacy non-text-based mounts.
> Here is the patch implementing  your suggestion...
>
> The patches are about the same size... As you mentioned this patch
> does not provide legacy support, which may or may not matter...
>
> Both patches have to remove the version option from the list
> pumped down to the kernel, since the option will cause the
> kernel routines to error out... Its much cleaner to
> remove moved the option 'mount_options' list than stripping
> the option from the string.
>
> So I guess it comes down to do we really care about legacy support??

I don't have a strong opinion, but my preference would be to leave  
enhancements like this to text-based mounts only.

> steved.
>
> Using only the string option parsing routine, allow the file system
> type to be changed to 'nfs4' when the NFS version options are  
> specified.
>
>
> Signed-off-by: Steve Dickson <steved@redhat.com>
> -----------
>
> diff --git a/utils/mount/network.c b/utils/mount/network.c
> index bcd0c0f..612c9c2 100644
> --- a/utils/mount/network.c
> +++ b/utils/mount/network.c
> @@ -94,6 +94,12 @@ static const char *nfs_version_opttbl[] = {
> 	"nfsvers",
> 	NULL,
> };
> +static const char *nfs_fstype_opttbl[] = {
> +	"v4",
> +	"vers",
> +	"nfsvers",
> +	NULL,
> +};
>
> static const unsigned long nfs_to_mnt[] = {
> 	0,
> @@ -1242,6 +1248,47 @@ static rpcvers_t nfs_nfs_version(struct  
> mount_options *options)
> }
>
> /*
> + * 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 mount_options *options)
> +{
> +	char *keyword;
> +	long tmp;
> +	static char *fs_type = "nfs";
> +
> +	switch (po_rightmost(options, nfs_fstype_opttbl)) {

As far as I recall, we recently fixed po_rightmost() to return a C- 
style array offset.  Shouldn't the cases be 0, 1, and 2?

> +	case 2: /* v4 */
> +		keyword = "v4";
> +		break;
> +	case 3:	/* vers */
> +		if (po_get_numeric(options, "vers", &tmp) == PO_FOUND)
> +			if (tmp != 4)
> +				return fs_type;
> +		keyword = "vers";
> +		break;
> +	case 4: /* nfsvers */
> +		if (po_get_numeric(options, "nfsvers", &tmp) == PO_FOUND)
> +			if (tmp != 4)
> +				return fs_type;
> +		keyword = "nfsvers";
> +		break;
> +	default:
> +		return fs_type;
> +	}
> +
> +	/*
> +	 * Need to remove the v4 version option from the
> +	 * list since the kernel will not how to parse it.
> +	 */
> +	po_remove_all(options, keyword);

You always need to remove all of "nfsvers," "vers", "v2", "v3", and  
"v4" if this is a nfs4 mount.  The reason for this is to make sure the  
user hasn't specified something bogus like "nfsvers=2,vers=3,v4".  In  
that case the correct behavior is to do a "nfs4" mount and strip out  
all verison options.  For something like "v4,nfsvers=3,vers=2" the  
correct behavior is to do a "nfs" mount and remove any v4-related  
version options (which the kernel won't like).  The rule is "rightmost  
wins".

So you need two exits from this function:

  * for "nfs" mounts, return "nfs" and make sure any of "v4"  
"nfsvers=4" or "vers=4" are removed, and

  * for "nfs4" mounts, return "nfs4" and strip all version options  
from the mount string.

> +	fs_type = "nfs4";
> +
> +	return fs_type;

I'm pretty sure you can get the same effect (returning a statically  
allocated character string) with:

	return "nfs4";

> +}
> +
> +/*
>  * Returns the NFS transport protocol specified by the given mount  
> options
>  *
>  * Returns the IPPROTO_ value specified by the given mount options, or
> diff --git a/utils/mount/network.h b/utils/mount/network.h
> index 0dd90f8..203e728 100644
> --- a/utils/mount/network.h
> +++ b/utils/mount/network.h
> @@ -64,6 +64,7 @@ void nfs_options2pmap(struct mount_options *,
> int start_statd(void);
>
> unsigned long nfsvers_to_mnt(const unsigned long);
> +char *nfs_fs_type(struct mount_options *);

If you're not going to use nfs_nfs_version() at all, then this  
function would be better off living in utils/mount/stropts.c.   
network.c is just for the network-related functions (like creating a  
socket, handling DNS lookups, or doing getport calls), and they are  
generally shared between legacy and text-based support.  This is  
clearly going to be a text-based-only kind of thing.

> int nfs_call_umount(clnt_addr_t *, dirpath *);
> int nfs_advise_umount(const struct sockaddr *, const socklen_t,
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index c369136..7a4f533 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -754,6 +754,9 @@ static const char *nfs_background_opttbl[] = {
>
> static int nfsmount_start(struct nfsmount_info *mi)
> {
> +
> +	mi->type = nfs_fs_type(mi->options);
> +
> 	if (!nfs_validate_options(mi))
> 		return EX_FAIL;

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

[-- Attachment #3: Type: text/plain, Size: 51 bytes --]




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

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

end of thread, other threads:[~2009-05-07 20:48 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-05 21:46 Allow vers=4 and nfsvers=4 mount options Kevin Constantine
     [not found] ` <1241560004-16787-1-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
2009-05-05 21:46   ` [PATCH 1/2] Allow nfs/vers=4 option in old style mount commands Kevin Constantine
     [not found]     ` <1241560004-16787-2-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
2009-05-05 21:46       ` [PATCH 2/2] Allow nfs/vers=4 option in text-based " Kevin Constantine
     [not found]         ` <1241560004-16787-3-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
2009-05-05 22:02           ` Chuck Lever
2009-05-07 20:03             ` Kevin Constantine
     [not found]               ` <4A033E7C.9060605-P5ys19MLBK/QT0dZR+AlfA@public.gmane.org>
2009-05-07 20:47                 ` Chuck Lever
2009-05-05 22:08       ` [PATCH 1/2] Allow nfs/vers=4 option in old style " Chuck Lever

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.