[PATCH 06/12] text-based mount.nfs:

[PATCH 06/12] text-based mount.nfs:

[Chuck Lever]

Introduce a function for probing the server for what it supports, and then
rewriting the mount options using the probe results.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mount/stropts.c |  113 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 113 insertions(+), 0 deletions(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index c048be0..d000695 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -238,6 +238,119 @@ static int set_mandatory_options(const char *type,
 }
 
 /*
+ * Reconstruct the mount option string based on a portmapper probe
+ * of the server.  Returns one if the server's portmapper returned
+ * something we can use, otherwise zero.
+ *
+ * Unfortunately, for fallback, we need to parse out the mount options
+ * in user space.  Unrecognized options are ignored here.
+ *
+ * Order of processing:
+ * 1.  fill in mnt_server and nfs_server, based on existing options
+ * 2.  strip out transport and NFS version related options
+ * 3.  probe the server
+ * 4.  append fresh transport and NFS version related options
+ *
+ * Returns a new group of mount options if successful; otherwise NULL
+ * is returned if some failure occurred.
+ */
+static struct mount_options *rewrite_mount_options(char *str)
+{
+	struct mount_options *options;
+	char *option, new_option[64];
+	clnt_addr_t mnt_server = { };
+	clnt_addr_t nfs_server = { };
+
+	options = po_split(str);
+	if (!options)
+		return NULL;
+
+	option = po_get(options, "addr");
+	if (option) {
+		nfs_server.saddr.sin_family = AF_INET;
+		if (!inet_aton((const char *)option, &nfs_server.saddr.sin_addr))
+			goto err;
+	} else
+		goto err;
+
+	option = po_get(options, "mountaddr");
+	if (option) {
+		mnt_server.saddr.sin_family = AF_INET;
+		if (!inet_aton((const char *)option, &mnt_server.saddr.sin_addr))
+			goto err;
+	} else
+		memcpy(&mnt_server.saddr, &nfs_server.saddr,
+				sizeof(mnt_server.saddr));
+
+	option = po_get(options, "mountport");
+	if (option)
+		mnt_server.pmap.pm_port = atoi(option);
+	mnt_server.pmap.pm_prog = MOUNTPROG;
+	option = po_get(options, "mountprog");
+	if (option)
+		mnt_server.pmap.pm_prog = atoi(option);
+	option = po_get(options, "mountvers");
+	if (option)
+		mnt_server.pmap.pm_vers = atoi(option);
+
+	option = po_get(options, "port");
+	if (option) {
+		nfs_server.pmap.pm_port = atoi(option);
+		po_remove_all(options, "port");
+	}
+	nfs_server.pmap.pm_prog = NFS_PROGRAM;
+	option = po_get(options, "nfsprog");
+	if (option)
+		nfs_server.pmap.pm_prog = atoi(option);
+
+	po_remove_all(options, "nfsvers");
+	po_remove_all(options, "vers");
+	po_remove_all(options, "tcp");
+	po_remove_all(options, "udp");
+
+	option = po_get(options, "proto");
+	if (option) {
+		if (strcmp(option, "tcp") == 0)
+			po_remove_all(options, "proto");
+		if (strcmp(option, "udp") == 0)
+			po_remove_all(options, "proto");
+	}
+
+	if (!probe_bothports(&mnt_server, &nfs_server)) {
+		rpc_mount_errors("rpcbind", 0, 0);
+		goto err;
+	}
+
+	snprintf(new_option, sizeof(new_option) - 1,
+		 "nfsvers=%lu", nfs_server.pmap.pm_vers);
+	if (po_append(options, new_option) == PO_FAILED)
+		goto err;
+
+	if (nfs_server.pmap.pm_prot == IPPROTO_TCP)
+		snprintf(new_option, sizeof(new_option) - 1,
+			 "proto=tcp");
+	else
+		snprintf(new_option, sizeof(new_option) - 1,
+			 "proto=udp");
+	if (po_append(options, new_option) == PO_FAILED)
+		goto err;
+
+	if (nfs_server.pmap.pm_port != NFS_PORT) {
+		snprintf(new_option, sizeof(new_option) - 1,
+			 "port=%lu", nfs_server.pmap.pm_port);
+		if (po_append(options, new_option) == PO_FAILED)
+			goto err;
+
+	}
+
+	return options;
+
+err:
+	po_destroy(options);
+	return NULL;
+}
+
+/*
  * Attempt an NFSv2/3 mount via a mount(2) system call.
  *
  * Returns 1 if successful.  Otherwise, returns zero.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 06/12] text-based mount.nfs:

[Neil Brown]

First 5 OK and applied.  But I'm a bit uncertain about this one.
I think the rest are OK as well, so once we've sorted this one, the
rest can be applied.

Apart from the empty subject line :-), it doesn't look right.
In particular....

On Monday October 8, chuck.lever@oracle.com wrote:
> Introduce a function for probing the server for what it supports, and then
> rewriting the mount options using the probe results.

>  /*
> + * Reconstruct the mount option string based on a portmapper probe
> + * of the server.  Returns one if the server's portmapper returned
> + * something we can use, otherwise zero.
> + *
> + * Unfortunately, for fallback, we need to parse out the mount options
> + * in user space.  Unrecognized options are ignored here.

(I that paragraph of the comment still relevant?)

> + *
> + * Order of processing:
> + * 1.  fill in mnt_server and nfs_server, based on existing options

This statement doesn't seem to be true.
In particular, the 'version number' and 'proto' in the existing
options are *not* (that I can see) filled in for nfs_server.

So it would seem that
	mount -o vers=3  server:/path /dir

would fall back to v2 if 'server' didn't support v3.  I don't think
that is right (current mount.nfs doesn't do that).

Did I misunderstand the code?


By the by, when testing what the current code does, I discovered that
if

      mount server:/path /dir

falls back to v2 because the default (v3) doesn't work, then
     umount /dir

fails with

     umount.nfs: Server failed to unmount 'server:/path'

which is a worry.
I think there are two problem here:
  1/ we should fall back to v2 umount if v3 fails (well, the MOUNT
     versions  are different, but you know what I mean).
  2/ If the call to the server fails, we should still unmount the
     filesystem (call del_mtab .... not a good choice for the name
     of the function that does the actual unmount...)
Thoughts?

NeilBrown

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 06/12] text-based mount.nfs:

[Chuck Lever]

Yep, this is the one I thought we would noodle on a bit to get it right.

On Oct 8, 2007, at 9:37 PM, Neil Brown wrote:
> First 5 OK and applied.  But I'm a bit uncertain about this one.
> I think the rest are OK as well, so once we've sorted this one, the
> rest can be applied.
>
> Apart from the empty subject line :-), it doesn't look right.

I tried to be careful.  The fallback behavior with text-based mounts  
is an attempt to be bug-for-bug compatible with legacy mount  
behavior.  So the starting place is to see what mount.nfs does in the  
same situation on the 2.6.22 kernel.

> In particular....
>
> On Monday October 8, chuck.lever@oracle.com wrote:
>> Introduce a function for probing the server for what it supports,  
>> and then
>> rewriting the mount options using the probe results.
>
>>  /*
>> + * Reconstruct the mount option string based on a portmapper probe
>> + * of the server.  Returns one if the server's portmapper returned
>> + * something we can use, otherwise zero.
>> + *
>> + * Unfortunately, for fallback, we need to parse out the mount  
>> options
>> + * in user space.  Unrecognized options are ignored here.
>
> (I that paragraph of the comment still relevant?)

I'm not sure what you think is irrelevant... the bit about  
unrecognized options?  If so, that comment was meant to imply that as  
we add support for new mount options in the kernel, we needn't be too  
concerned about adding support for them here as well.

Or maybe you meant the "need to parse out the mount options in user  
space" -- that implies that, for text-based mounts, mount.nfs still  
needs to parse the mount options in order to do the port probe and  
then rewrite them.  Our dream of parsing only in the kernel is dashed  
on the rocks.

I will see about clarifying that when resubmitting the patch.

>> + *
>> + * Order of processing:
>> + * 1.  fill in mnt_server and nfs_server, based on existing options
>
> This statement doesn't seem to be true.
> In particular, the 'version number' and 'proto' in the existing
> options are *not* (that I can see) filled in for nfs_server.

Yes, that's correct.  The idea is that the port probe will fill those  
in for us based on what the server supports.

> So it would seem that
> 	mount -o vers=3  server:/path /dir
>
> would fall back to v2 if 'server' didn't support v3.  I don't think
> that is right (current mount.nfs doesn't do that).

Hrm, I tested that at one point, but this latest version may behave  
incorrectly.  (To be clear, yes, I agree that version fallback should  
occur only if the user did not specify "vers=" or "nfsvers=".   
Likewise for the transport protocol).

Filling in nfs_server based on existing version and protocol options  
will probably prevent this incorrect behavior by nailing the port  
probe to a specific version or protocol, and it's easy enough to add  
that.

> Did I misunderstand the code?

No.  Your review has been helpful at each point.

> By the by, when testing what the current code does, I discovered that
> if
>
>       mount server:/path /dir
>
> falls back to v2 because the default (v3) doesn't work, then
>      umount /dir
>
> fails with
>
>      umount.nfs: Server failed to unmount 'server:/path'
>
> which is a worry.

OK, it's not supposed to do that either, and I never saw that  
behavior during testing.  (Well, actually we did see it, but it was  
only with NFSv4, and is supposed to be fixed by one of the patches in  
the last set I sent you).

> I think there are two problem here:
>   1/ we should fall back to v2 umount if v3 fails (well, the MOUNT
>      versions  are different, but you know what I mean).
>   2/ If the call to the server fails, we should still unmount the
>      filesystem (call del_mtab .... not a good choice for the name
>      of the function that does the actual unmount...)
> Thoughts?

First I would like to understand precisely why umount is failing.  In  
this case, mount.nfs should rewrite the mount options, then post them  
in /etc/mtab so that umount can use the same service endpoint when  
doing the unmount.  The umount should then use the post-fallback  
options, and succeed.  Does "umount -v" provide any helpful information?

Certainly it would be good to rename del_mtab() !

I also agree that it would be an overall improvement if the RPC  
umount request did not determine whether or not umount also tries to  
detach the local file system.  I don't see any good reason to link  
the two.

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



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 06/12] text-based mount.nfs:

[Neil Brown]

On Tuesday October 9, chuck.lever@oracle.com wrote:
> >>  /*
> >> + * Reconstruct the mount option string based on a portmapper probe
> >> + * of the server.  Returns one if the server's portmapper returned
> >> + * something we can use, otherwise zero.
> >> + *
> >> + * Unfortunately, for fallback, we need to parse out the mount  
> >> options
> >> + * in user space.  Unrecognized options are ignored here.
> >
> > (I that paragraph of the comment still relevant?)
> 
> I'm not sure what you think is irrelevant... the bit about  
> unrecognized options?  If so, that comment was meant to imply that as  
> we add support for new mount options in the kernel, we needn't be too  
> concerned about adding support for them here as well.
> 
> Or maybe you meant the "need to parse out the mount options in user  
> space" -- that implies that, for text-based mounts, mount.nfs still  
> needs to parse the mount options in order to do the port probe and  
> then rewrite them.  Our dream of parsing only in the kernel is dashed  
> on the rocks.

The comment talks about parsing out the mount options, which is now
done in a very different part of the code.
And I misread the "Unrecognised options are ignored here" as saying that
they get removed... Maybe the "unfortunately" predisposed me to think
something bad?

> 
> I will see about clarifying that when resubmitting the patch.

Yes, new version much clearer, thanks.

> > By the by, when testing what the current code does, I discovered that
> > if
> >
> >       mount server:/path /dir
> >
> > falls back to v2 because the default (v3) doesn't work, then
> >      umount /dir
> >
> > fails with
> >
> >      umount.nfs: Server failed to unmount 'server:/path'
> >
> > which is a worry.
> 
> OK, it's not supposed to do that either, and I never saw that  
> behavior during testing.  (Well, actually we did see it, but it was  
> only with NFSv4, and is supposed to be fixed by one of the patches in  
> the last set I sent you).
> 
> > I think there are two problem here:
> >   1/ we should fall back to v2 umount if v3 fails (well, the MOUNT
> >      versions  are different, but you know what I mean).
> >   2/ If the call to the server fails, we should still unmount the
> >      filesystem (call del_mtab .... not a good choice for the name
> >      of the function that does the actual unmount...)
> > Thoughts?
> 
> First I would like to understand precisely why umount is failing.  In  
> this case, mount.nfs should rewrite the mount options, then post them  
> in /etc/mtab so that umount can use the same service endpoint when  
> doing the unmount.  The umount should then use the post-fallback  
> options, and succeed.  Does "umount -v" provide any helpful
> information?

(remembering this is with the binary mount interface, not the new
ASCII one) the updated mount options are *not* written to /etc/mtab.
It just has 

 eli:/var/tmp/samba /mnt nfs rw,addr=192.168.1.3 0 0

nfsvers=2 has not been added.
umount -v says:
umount.nfs: trying 192.168.1.3 prog 100005 vers 3 prot UDP port 723
umount.nfs: Server failed to unmount 'eli:/var/tmp/samba'


> 
> Certainly it would be good to rename del_mtab() !
> 
> I also agree that it would be an overall improvement if the RPC  
> umount request did not determine whether or not umount also tries to  
> detach the local file system.  I don't see any good reason to link  
> the two.

I've broken the link, so the unmount now succeeds.  It would be good
to get it to send the right request to the server though, either by
recording the protocol in mtab, or using probe_mntport or similar.

Thanks,
NeilBrown

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 06/12] text-based mount.nfs:

[Chuck Lever]

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

Neil Brown wrote:
>>> By the by, when testing what the current code does, I discovered that
>>> if
>>>
>>>       mount server:/path /dir
>>>
>>> falls back to v2 because the default (v3) doesn't work, then
>>>      umount /dir
>>>
>>> fails with
>>>
>>>      umount.nfs: Server failed to unmount 'server:/path'
>>>
>>> which is a worry.
>> OK, it's not supposed to do that either, and I never saw that  
>> behavior during testing.  (Well, actually we did see it, but it was  
>> only with NFSv4, and is supposed to be fixed by one of the patches in  
>> the last set I sent you).
>>
>>> I think there are two problem here:
>>>   1/ we should fall back to v2 umount if v3 fails (well, the MOUNT
>>>      versions  are different, but you know what I mean).
>>>   2/ If the call to the server fails, we should still unmount the
>>>      filesystem (call del_mtab .... not a good choice for the name
>>>      of the function that does the actual unmount...)
>>> Thoughts?
>> First I would like to understand precisely why umount is failing.  In  
>> this case, mount.nfs should rewrite the mount options, then post them  
>> in /etc/mtab so that umount can use the same service endpoint when  
>> doing the unmount.  The umount should then use the post-fallback  
>> options, and succeed.  Does "umount -v" provide any helpful
>> information?
> 
> (remembering this is with the binary mount interface, not the new
> ASCII one) the updated mount options are *not* written to /etc/mtab.
> It just has 
> 
>  eli:/var/tmp/samba /mnt nfs rw,addr=192.168.1.3 0 0
> 
> nfsvers=2 has not been added.
> umount -v says:
> umount.nfs: trying 192.168.1.3 prog 100005 vers 3 prot UDP port 723
> umount.nfs: Server failed to unmount 'eli:/var/tmp/samba'

OK... the new information is that this is failing with the legacy mount. 
  That makes a little more sense.

>> Certainly it would be good to rename del_mtab() !
>>
>> I also agree that it would be an overall improvement if the RPC  
>> umount request did not determine whether or not umount also tries to  
>> detach the local file system.  I don't see any good reason to link  
>> the two.
> 
> I've broken the link, so the unmount now succeeds.  It would be good
> to get it to send the right request to the server though, either by
> recording the protocol in mtab, or using probe_mntport or similar.

Yeah, that's odd.  It is supposed to be probing the server's mountd in 
nfs_call_umount.  The problem may be that it's only trying a single 
mount RPC (for vers=3), and not doing any kind of version fallback.

[-- Attachment #2: chuck.lever.vcf --]
[-- Type: text/x-vcard, Size: 315 bytes --]

begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture: Linux Projects Group
adr:;;1015 Granger Avenue;Ann Arbor;MI;48104;USA
email;internet:chuck dot lever at nospam oracle dot com
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
version:2.1
end:vcard


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

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

[-- Attachment #4: Type: text/plain, Size: 140 bytes --]

_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 06/12] text-based mount.nfs:

[Neil Brown]

On Wednesday October 10, chuck.lever@oracle.com wrote:
> > 
> > I've broken the link, so the unmount now succeeds.  It would be good
> > to get it to send the right request to the server though, either by
> > recording the protocol in mtab, or using probe_mntport or similar.
> 
> Yeah, that's odd.  It is supposed to be probing the server's mountd in 
> nfs_call_umount.  The problem may be that it's only trying a single 
> mount RPC (for vers=3), and not doing any kind of version fallback.

Thanks for the pointers.  I've got it working, though the patch
(below) is a big ugly at this stage.

Two key fixes:
  1/ Don't give up probing if we get RPC_PROGVERSMISMATCH.
  2/ Don't set pm_vers if it isn't set in mtab.  Leave it at zero so
     the probe routines get a chance to do the right thing.

   An awkwardness here was we were overloading the meaning of 
        pm_vers == 0
   In some contexts it means "Has not been explicitly set, probe".
   In other contexts it means "Mountd is not used (due to NFSv4)".

   I introduced '63356' for the second meaning, but would rather a
   cleaner approach.

   Maybe we don't need the second meaning, as the 2/3 versus 4
   distinction is present in mnt_type so we never call do_nfs_umount
   for a v4 mount.  Is that true?

NeilBrown

  

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 49d3c6b..c37d0f3 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -85,7 +85,7 @@ unsigned long nfsvers_to_mnt(const unsigned long vers)
 {
 	if (vers <= 3)
 		return nfs_to_mnt[vers];
-	return 0;
+	return 65536; /* invalid */
 }
 
 /*
@@ -408,7 +408,8 @@ static int probe_port(clnt_addr_t *server, const unsigned long *versions,
 					goto out_bad;
 			}
 		}
-		if (rpc_createerr.cf_stat != RPC_PROGNOTREGISTERED)
+		if (rpc_createerr.cf_stat != RPC_PROGNOTREGISTERED &&
+		    rpc_createerr.cf_stat != RPC_PROGVERSMISMATCH)
 			goto out_bad;
 
 		if (!prot) {
@@ -589,6 +590,7 @@ int nfs_call_umount(clnt_addr_t *mnt_server, dirpath *argp)
 	case 3:
 	case 2:
 	case 1:
+	case 0:
 		if (!probe_mntport(mnt_server))
 			return 0;
 		clnt = mnt_openclnt(mnt_server, &msock);
@@ -602,7 +604,7 @@ int nfs_call_umount(clnt_addr_t *mnt_server, dirpath *argp)
 		if (res == RPC_SUCCESS)
 			return 1;
 		break;
-	default:
+	default: /* invalid - must be NFSv4 which doesn't use MOUNT */
 		res = RPC_SUCCESS;
 		break;
 	}
diff --git a/utils/mount/nfsumount.c b/utils/mount/nfsumount.c
index 9e9cd16..051592b 100644
--- a/utils/mount/nfsumount.c
+++ b/utils/mount/nfsumount.c
@@ -190,7 +190,7 @@ static int do_nfs_umount(const char *spec, char *opts)
 	}
 
 	pmap->pm_prog = MOUNTPROG;
-	pmap->pm_vers = MOUNTVERS_NFSV3;
+	pmap->pm_vers = 0; /* unknown */
 	if (opts && (p = strstr(opts, "mountprog=")) && isdigit(*(p+10)))
 		pmap->pm_prog = atoi(p+10);
 	if (opts && (p = strstr(opts, "mountport=")) && isdigit(*(p+10)))

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 06/12] text-based mount.nfs:

[Chuck Lever]

On Oct 10, 2007, at 11:29 PM, Neil Brown wrote:
> On Wednesday October 10, chuck.lever@oracle.com wrote:
>>>
>>> I've broken the link, so the unmount now succeeds.  It would be good
>>> to get it to send the right request to the server though, either by
>>> recording the protocol in mtab, or using probe_mntport or similar.
>>
>> Yeah, that's odd.  It is supposed to be probing the server's  
>> mountd in
>> nfs_call_umount.  The problem may be that it's only trying a single
>> mount RPC (for vers=3), and not doing any kind of version fallback.
>
> Thanks for the pointers.  I've got it working, though the patch
> (below) is a big ugly at this stage.
>
> Two key fixes:
>   1/ Don't give up probing if we get RPC_PROGVERSMISMATCH.
>   2/ Don't set pm_vers if it isn't set in mtab.  Leave it at zero so
>      the probe routines get a chance to do the right thing.

I think that's the right thing to do.

>    An awkwardness here was we were overloading the meaning of
>         pm_vers == 0
>    In some contexts it means "Has not been explicitly set, probe".
>    In other contexts it means "Mountd is not used (due to NFSv4)".
>
>    I introduced '63356' for the second meaning, but would rather a
>    cleaner approach.

That shouldn't be necessary.  Kevin and I just added a bit of logic  
to prevent the do_nfs_unmount() call for "nfs4" file systems (see nfs- 
utils commit 97fed306).

That ought to make your patch much simpler, overall.

>    Maybe we don't need the second meaning, as the 2/3 versus 4
>    distinction is present in mnt_type so we never call do_nfs_umount
>    for a v4 mount.  Is that true?

Yep!

Actually you could make it clear that do_nfs_unmount() is not for v4  
by renaming it do_nfs_unmount23() or something like that.

> NeilBrown
>
>
>
> diff --git a/utils/mount/network.c b/utils/mount/network.c
> index 49d3c6b..c37d0f3 100644
> --- a/utils/mount/network.c
> +++ b/utils/mount/network.c
> @@ -85,7 +85,7 @@ unsigned long nfsvers_to_mnt(const unsigned long  
> vers)
>  {
>  	if (vers <= 3)
>  		return nfs_to_mnt[vers];
> -	return 0;
> +	return 65536; /* invalid */
>  }
>
>  /*
> @@ -408,7 +408,8 @@ static int probe_port(clnt_addr_t *server,  
> const unsigned long *versions,
>  					goto out_bad;
>  			}
>  		}
> -		if (rpc_createerr.cf_stat != RPC_PROGNOTREGISTERED)
> +		if (rpc_createerr.cf_stat != RPC_PROGNOTREGISTERED &&
> +		    rpc_createerr.cf_stat != RPC_PROGVERSMISMATCH)
>  			goto out_bad;
>
>  		if (!prot) {
> @@ -589,6 +590,7 @@ int nfs_call_umount(clnt_addr_t *mnt_server,  
> dirpath *argp)
>  	case 3:
>  	case 2:
>  	case 1:
> +	case 0:
>  		if (!probe_mntport(mnt_server))
>  			return 0;
>  		clnt = mnt_openclnt(mnt_server, &msock);
> @@ -602,7 +604,7 @@ int nfs_call_umount(clnt_addr_t *mnt_server,  
> dirpath *argp)
>  		if (res == RPC_SUCCESS)
>  			return 1;
>  		break;
> -	default:
> +	default: /* invalid - must be NFSv4 which doesn't use MOUNT */
>  		res = RPC_SUCCESS;
>  		break;
>  	}
> diff --git a/utils/mount/nfsumount.c b/utils/mount/nfsumount.c
> index 9e9cd16..051592b 100644
> --- a/utils/mount/nfsumount.c
> +++ b/utils/mount/nfsumount.c
> @@ -190,7 +190,7 @@ static int do_nfs_umount(const char *spec, char  
> *opts)
>  	}
>
>  	pmap->pm_prog = MOUNTPROG;
> -	pmap->pm_vers = MOUNTVERS_NFSV3;
> +	pmap->pm_vers = 0; /* unknown */
>  	if (opts && (p = strstr(opts, "mountprog=")) && isdigit(*(p+10)))
>  		pmap->pm_prog = atoi(p+10);
>  	if (opts && (p = strstr(opts, "mountport=")) && isdigit(*(p+10)))

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




-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 06/12] text-based mount.nfs:

[Neil Brown]

On Thursday October 11, chuck.lever@oracle.com wrote:
> On Oct 10, 2007, at 11:29 PM, Neil Brown wrote:
> >
> > Two key fixes:
> >   1/ Don't give up probing if we get RPC_PROGVERSMISMATCH.
> >   2/ Don't set pm_vers if it isn't set in mtab.  Leave it at zero so
> >      the probe routines get a chance to do the right thing.
> 
> I think that's the right thing to do.
> 
> >    An awkwardness here was we were overloading the meaning of
> >         pm_vers == 0
> >    In some contexts it means "Has not been explicitly set, probe".
> >    In other contexts it means "Mountd is not used (due to NFSv4)".
> >
> >    I introduced '63356' for the second meaning, but would rather a
> >    cleaner approach.
> 
> That shouldn't be necessary.  Kevin and I just added a bit of logic  
> to prevent the do_nfs_unmount() call for "nfs4" file systems (see nfs- 
> utils commit 97fed306).
> 
> That ought to make your patch much simpler, overall.
> 
> >    Maybe we don't need the second meaning, as the 2/3 versus 4
> >    distinction is present in mnt_type so we never call do_nfs_umount
> >    for a v4 mount.  Is that true?
> 
> Yep!
> 
> Actually you could make it clear that do_nfs_unmount() is not for v4  
> by renaming it do_nfs_unmount23() or something like that.
> 

Thanks for the feedback.  I've removed the v4 handling from
do_nfs_umount and nfs_call_umount, and it does the right thing for
this case now.

NeilBrown

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs