[PATCH 1/9] mount.nfs: Fix nfs4mount_s prototype.

[PATCH 1/9] mount.nfs: Fix nfs4mount_s prototype.

[Chuck Lever]

The definition of nfs4mount_s in utils/mount/stropts.c doesn't match the
prototype declared in utils/mount/stropts.h.

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

 utils/mount/stropts.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 95392a1..ca79ce0 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -37,6 +37,7 @@
 #include "nls.h"
 #include "nfs_mount.h"
 #include "mount_constants.h"
+#include "stropts.h"
 #include "error.h"
 #include "network.h"
 
@@ -286,13 +287,13 @@ fail:
  * @extra_opts:	pointer to C string containing fs-specific mount options
  *		(possibly also a return argument)
  * @fake:	flag indicating whether to carry out the whole operation
- * @bg:		one if this is a backgrounded mount attempt
+ * @child:	one if this is a backgrounded mount
  *
  * XXX: need to handle bg, fg, and retry options.
  *
  */
 int nfs4mount_s(const char *spec, const char *node, int flags,
-		char **extra_opts, int fake)
+		char **extra_opts, int fake, int child)
 {
 	int retval = EX_FAIL;
 


-------------------------------------------------------------------------
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 1/9] mount.nfs: Fix nfs4mount_s prototype.

[Neil Brown]

> @@ -286,13 +287,13 @@ fail:
>   * @extra_opts:	pointer to C string containing fs-specific mount options
>   *		(possibly also a return argument)
>   * @fake:	flag indicating whether to carry out the whole operation
> - * @bg:		one if this is a backgrounded mount attempt
> + * @child:	one if this is a backgrounded mount

Any reason for changing 'bg' to 'child'?
And not making a matching change in nfsmount_s?

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 1/9] mount.nfs: Fix nfs4mount_s prototype.

[Neil Brown]

On Monday August 27, chuck.lever@oracle.com wrote:
> Hi Neil-
> 
> Neil Brown wrote:
> >> @@ -286,13 +287,13 @@ fail:
> >>   * @extra_opts:	pointer to C string containing fs-specific mount options
> >>   *		(possibly also a return argument)
> >>   * @fake:	flag indicating whether to carry out the whole operation
> >> - * @bg:		one if this is a backgrounded mount attempt
> >> + * @child:	one if this is a backgrounded mount
> > 
> > Any reason for changing 'bg' to 'child'?
> 
> I think it's more clear about how the function is being called.

Yes, I eventually worked this out.
nfs4mount_s is firstly called with 'child' set to 0.  Then if that
fails and that 'bg' option is set, we fork and call it again with
'child' set to 1.   So 'child' is more meaningful than 'bg'.

However that handling of 'bg' seems much more simple in nfsmount_s
than in nfsmount - too simple?
With 'fg', it should try some number of times.
With 'bg', it should try once, then if that fails, fork and retry some
number of times in that background.
Are you doing the 'retry' in the kernel?  How do you tell it to only
try once before you fork for 'bg'?
Does the mount retry really need to be in the kernel?  I think that is
a very different issue that just changing the option parsing to be
string based.

Also, I just noticed the:
                    After a mount operation is backgrounded, all subsequent
       mounts  on  the  same  NFS  server will be backgrounded immediately,
       without first attempting  the  mount.

part of 'bg'.  That is awkward to implement when each individual mount
is done in a separate processes as happens with mount.nfs.   There is
code there which pretends to implement this, but of course it cannot
work and should be stripped out.
I wonder if it is worth trying to record the host name in a file so
that one instance can pass it to the next .... possibly not.  Though
the kernel could easily know that a particular server is not
responding, so the first mount attempt should give up quickly....

Just some thoughts.

NeilBrown

> 
> > And not making a matching change in nfsmount_s?
> 
> Upcoming patches will address this.

I notice that nfsmount and nfs4mount use 'running_bg'.  Not sure which
is better, but it would be nice to be consistent. 

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 1/9] mount.nfs: Fix nfs4mount_s prototype.

[Chuck Lever]

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

Neil Brown wrote:
> Yes, I eventually worked this out.
> nfs4mount_s is firstly called with 'child' set to 0.  Then if that
> fails and that 'bg' option is set, we fork and call it again with
> 'child' set to 1.   So 'child' is more meaningful than 'bg'.
> 
> However that handling of 'bg' seems much more simple in nfsmount_s
> than in nfsmount - too simple?

nfsmount_s doesn't implement bg at all right now.  Those patches are 
still in my stack (and partly in my head).

> With 'fg', it should try some number of times.

It's supposed to retry until the retry=n timeout occurs (2 minutes) but 
experientially I'm not sure that's even implemented in the legacy 
mount.nfs.  I remember a bug we found when I was at NetApp where a fg 
mount tried the request just once and gave up.

> With 'bg', it should try once, then if that fails, fork and retry some
> number of times in that background.

Yes, it will retry based on an exponential back-off scheme, starting at 
one second and going up.  The limit is 30 seconds for the legacy 
mount.nfs, and I'm thinking of going up to 60 for the text-based 
implementation.

> Are you doing the 'retry' in the kernel?

No, we're considering that to be "policy" more or less, so it belongs in 
user space.  I'm going to make the kernel mount client fire one mount 
request that either succeeds or gives up.

> How do you tell it to only
> try once before you fork for 'bg'?

Each retry is driven from mount.nfs.

> Does the mount retry really need to be in the kernel?  I think that is
> a very different issue that just changing the option parsing to be
> string based.

This is one reason why this effort is a challenge.

> Also, I just noticed the:
>                     After a mount operation is backgrounded, all subsequent
>        mounts  on  the  same  NFS  server will be backgrounded immediately,
>        without first attempting  the  mount.
> 
> part of 'bg'.  That is awkward to implement when each individual mount
> is done in a separate processes as happens with mount.nfs.   There is
> code there which pretends to implement this, but of course it cannot
> work and should be stripped out.

That is correct.  Dick S. and I went over this on the list in the past 
week or two.  This is something that doesn't work at all in mount.nfs 
(in the legacy interface or in the text-based interface), but used to 
work in the util-linux mount command.

Peter S. has made the argument that we should more or less abandon all 
such "bg" kludges in favor of the automounter.

> I wonder if it is worth trying to record the host name in a file so
> that one instance can pass it to the next .... possibly not.  Though
> the kernel could easily know that a particular server is not
> responding, so the first mount attempt should give up quickly....

One thing that has been suggested is a mount server connection manager 
on the client.  Could be a kernel process... it would have a single 
connection per mountd, and all mount requests for that server would go 
down that connection.  The connection would time out after 5 minutes of 
inactivity.

In the case of multiple mounts to the same server, the mountd connection 
manager would know about the NFS server being down, and would be able to 
tell any mount requestors to retry later.

>>> And not making a matching change in nfsmount_s?
>> Upcoming patches will address this.
> 
> I notice that nfsmount and nfs4mount use 'running_bg'.  Not sure which
> is better, but it would be nice to be consistent. 

I hear you.  But the interfaces are going to be so different when all 
this is done, a little consistency is probably not going to be an issue. 
  I'll keep it in mind, though.

At some point we need to decide how much priority we're going to give to 
keeping the legacy parts of mount.nfs updated (ie just bug fixes -- 
certainly not new features like the "fscache" mount option).

[-- Attachment #2: chuck.lever.vcf --]
[-- Type: text/x-vcard, Size: 290 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
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
url:http://oss.oracle.com/~cel
version:2.1
end:vcard


[-- Attachment #3: Type: text/plain, Size: 315 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