[daniel@vision.ime.usp.br: Bug#304245: patch - NFS mounts occasionally fail]

[daniel@vision.ime.usp.br: Bug#304245: patch - NFS mounts occasionally fail]

[Steinar H. Gunderson]

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

Hi,

Thanks for the patch; sending upstream for evaluation.

/* Steinar */
-- 
Homepage: http://www.sesse.net/

[-- Attachment #2: Type: message/rfc822, Size: 5749 bytes --]

From: Daniel Andre Vaquero <daniel@vision.ime.usp.br>
To: 304245@bugs.debian.org
Subject: Bug#304245: patch - NFS mounts occasionally fail
Date: Sat, 16 Apr 2005 23:38:26 -0300
Message-ID: <20050417023826.GA4527@vision.ime.usp.br>

Hi,

After some days of testing with upstream version 4.1.4, I noticed that it
is also broken (although mount failures occur less frequently). My scenario
consists of a single server, with no replicated mounts. Debugging the code,
I found that the problem happens in the function get_best_mount()
of mount_nfs.c. If the server don't respond to rpc_ping() within 0.1 seconds,
winner remains NULL and the function is not called again with the longer
timeout (10 sec), leading to a mount failure because no winner is elected.
Another thing I noticed is that the behavior when there are replicated hosts
is not the described in README.replicated-server. If all pings fail, winner
should be set to the first host in the list, but the current code will return
winner = NULL.

I have made a patch that should solve these problems, and included it below
(maybe this fixes the problem of #297359 too).
It must be applied over upstream version 4.1.4. I'll be testing it for the 
single server scenario in the next few days.

$ diff -upr autofs-4.1.4.orig/modules/mount_nfs.c autofs-4.1.4/modules/mount_nfs.c

--- autofs-4.1.4.orig/modules/mount_nfs.c 2005-04-05 09:42:42.000000000 -0300
+++ autofs-4.1.4/modules/mount_nfs.c   2005-04-16 22:59:05.000000000 -0300
@@ -251,33 +251,28 @@ int get_best_mount(char *what, const cha
 
   debug(MODPREFIX "winner = %s local = %d", winner, local);
 
-  /*
-   * We didn't find a weighted winner or local
-   */
-  if (!local && winner_weight == INT_MAX) {
-     /* We had more than one contender and none responded in time */
-     if (winner_time != 0 && winner_time > 500) {
-        /* We've already tried a longer timeout */
-        if (!longtimeout) {
-           /* Reset string and try again */
-           strcpy(what, original);
-
-           debug(MODPREFIX 
-                 "all hosts timed out for '%s', "
-                 "retrying with longer timeout",
-                 original);
-
-           return get_best_mount(what, original, 1, 1);
-        }
+  /* No winner found */
+  if (!local && !winner) {
+     
+     /* Retry with a longer timeout */
+     if (!longtimeout) {
+        /* Reset string and try again */
+        strcpy(what, original);
+      
+        debug(MODPREFIX 
+              "all hosts timed out for '%s', "
+              "retrying with longer timeout",
+              original);
+        
+        return get_best_mount(what, original, 1, 1);
+     }
+     else {
+        /* No hosts responded, winner = first host */
+        winner = what;
      }
+     
   }
-
-  /* No winner found so bail */
-  if (!winner) {
-     *what = '\0';
-     return 0;
-  }
-
+  
   /*
    * We now have our winner, copy it to the front of the string,
    * followed by the next :string<delim>



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

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

Re: [daniel@vision.ime.usp.br: Bug#304245: patch - NFS mounts occasionally fail]

[Daniel Andre Vaquero]

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

Hi,

My editor broke the indentation style in the patch; I'm resending it as
an attached file.

Thanks,

Daniel

On Sun, Apr 17, 2005 at 05:42:40PM +0200, Steinar H. Gunderson wrote:
> Hi,
> 
> Thanks for the patch; sending upstream for evaluation.
> 
> /* Steinar */

[-- Attachment #2: nfs_mount_fail.diff --]
[-- Type: text/plain, Size: 1490 bytes --]

--- autofs-4.1.4.orig/modules/mount_nfs.c	2005-04-05 09:42:42.000000000 -0300
+++ autofs-4.1.4/modules/mount_nfs.c	2005-04-16 22:59:05.000000000 -0300
@@ -251,33 +251,28 @@ int get_best_mount(char *what, const cha
 
 	debug(MODPREFIX "winner = %s local = %d", winner, local);
 
-	/*
-	 * We didn't find a weighted winner or local
-	 */
-	if (!local && winner_weight == INT_MAX) {
-		/* We had more than one contender and none responded in time */
-		if (winner_time != 0 && winner_time > 500) {
-			/* We've already tried a longer timeout */
-			if (!longtimeout) {
-				/* Reset string and try again */
-				strcpy(what, original);
-
-				debug(MODPREFIX 
-				      "all hosts timed out for '%s', "
-				      "retrying with longer timeout",
-				      original);
-
-				return get_best_mount(what, original, 1, 1);
-			}
+	/* No winner found */
+	if (!local && !winner) {
+		
+		/* Retry with a longer timeout */
+		if (!longtimeout) {
+			/* Reset string and try again */
+			strcpy(what, original);
+	    
+			debug(MODPREFIX 
+			      "all hosts timed out for '%s', "
+			      "retrying with longer timeout",
+			      original);
+			
+			return get_best_mount(what, original, 1, 1);
+		}
+		else {
+			/* No hosts responded, winner = first host */
+			winner = what;
 		}
+		
 	}
-
-	/* No winner found so bail */
-	if (!winner) {
-		*what = '\0';
-		return 0;
-	}
-
+	
 	/*
 	 * We now have our winner, copy it to the front of the string,
 	 * followed by the next :string<delim>

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

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

Re: Re: [daniel@vision.ime.usp.br: Bug#304245: patch - NFS mounts occasionally fail]

[raven]

On Sun, 17 Apr 2005, Daniel Andre Vaquero wrote:

> Hi,
>
> My editor broke the indentation style in the patch; I'm resending it as
> an attached file.

Will check out the patch.

Thanks for the feedback.
Ian

Re: [daniel@vision.ime.usp.br: Bug#304245: patch - NFS mounts occasionally fail]

[Chris Feist]

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

I'm currently using the attached patch which should fix this problem. We've 
tested it and it works pretty well (It also fixes a problem with local mounts 
not using --bind).

Thanks,
Chris

Steinar H. Gunderson wrote:
> Hi,
> 
> Thanks for the patch; sending upstream for evaluation.
> 
> /* Steinar */
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> autofs mailing list
> autofs@linux.kernel.org
> http://linux.kernel.org/mailman/listinfo/autofs


[-- Attachment #2: autofs-4.1.4-non-replicated-ping.patch --]
[-- Type: text/x-patch, Size: 5442 bytes --]

--- autofs-4.1.4/modules/mount_nfs.c.orig	2005-04-13 20:31:24.639009544 -0400
+++ autofs-4.1.4/modules/mount_nfs.c	2005-04-13 20:31:40.727563712 -0400
@@ -31,6 +31,7 @@
 #include <netinet/in.h>
 #include <linux/nfs.h>
 #include <linux/nfs2.h>
+#include <ctype.h>
 
 #define MODULE_MOUNT
 #include "automount.h"
@@ -105,28 +106,116 @@ int is_local_addr(const char *host, cons
 	
 	return 1;
 }
+
+/*
+ * If the map doesn't contain a ',' and doesn't contain more than
+ * one ':' then @what is not a multimount entry.
+ */
+static int is_multimount_entry(char *what)
+{
+	return !strchr(what, ',') && (strchr(what, ':') == strrchr(what, ':'));
+}
+
+/*
+ *  Check to see if the 'host:path' or 'host' is on the local machine
+ *  Returns < 0 if there is a host lookup problem, otherwise returns 0
+ *  if it's not a local mount, and returns > 0 if it is a local mount.
+ */
+int is_local_mount(const char *hostpath)
+{
+	struct hostent *he;
+	char **haddr;
+	char *delim;
+	char *hostname;
+	int hostnamelen;
+	int local = 0;
+
+	debug(MODPREFIX "is_local_mount: %s", hostpath);
+	delim = strpbrk(hostpath,":");
+
+	if (delim) 
+		hostnamelen = delim - hostpath; 
+	else 
+		hostnamelen = strlen(hostpath);
+
+	hostname = malloc(hostnamelen+1);
+	strncpy(hostname,hostpath,hostnamelen);
+	hostname[hostnamelen] = '\0';
+	he = gethostbyname(hostname);
+	if (!he) {
+		error(MODPREFIX "host %s: lookup failure", hostname);
+		return -1;
+	}
+
+	for (haddr = he->h_addr_list; *haddr; haddr++) {
+		local = is_local_addr(hostname, *haddr, he->h_length);
+		if (local < 0) 
+			return local;
+ 		if (local) {
+			debug(MODPREFIX "host %s: is localhost",
+					hostname);
+			return local;
+		}
+	}
+	return 0;
+}
+
 /*
  * Given a mount string, return (in the same string) the
- * best mount to use based on weight/locality/rpctime
+ * best mount to use based on locality/weight/rpctime.
+ *
+ * If longtimeout is set to 0 then we only do 100 ms pings to hosts.  In
+ * the event that this fails, we call ourself recursively with the
+ * longtimeout option set to 1.  In this case we ping for up to 10s and
+ * skip logic for detecting if a localhost has been passed. (if a local
+ * host had been passed, we would have returned that mount as the best
+ * mount.  The skipping of local maps in this case is an optimization).
+ *
  * - return -1 and what = '\0' on error,
  *           1 and what = local mount path if local bind,
  *     else  0 and what = remote mount path
  */
-int get_best_mount(char *what, const char *original, int longtimeout, int skiplocal)
+int get_best_mount(char *what, const char *original, int longtimeout)
 {
 	char *p = what;
 	char *winner = NULL;
 	int winner_weight = INT_MAX, local = 0;
 	double winner_time = 0;
-	char *delim;
+	char *delim, *pstrip;
 	int sec = (longtimeout) ? 10 : 0;
 	int micros = (longtimeout) ? 0 : 100000;
+	int skiplocal = longtimeout; /* clearly local is not available */
 
 	if (!p) {
 		*what = '\0';
 		return -1;
 	}
 
+	/*
+	 *  If only one mountpoint has been passed in, we don't need to
+	 *  do anything except strip whitespace from the end of the string.
+	 */
+	if (!is_multimount_entry(p)) {
+		for (pstrip = p+strlen(p) - 1; pstrip >= p; pstrip--) 
+			if (isspace(*pstrip))
+				*pstrip = '\0';
+
+		/* Check if the host is the localhost */
+		if (is_local_mount(p)) {
+			debug(MODPREFIX "host %s: is localhost", p);
+
+			/* Strip off hostname and ':' */
+			delim = strchr(p,':');
+			while (delim && *delim != '\0') {
+				delim++;
+				*what = *delim;
+				what++;
+			}
+			return 1;
+		}
+		return 0;
+	}
+
 	while (p && *p) {
 		char *next;
 		unsigned int ping_stat = 0;
@@ -171,37 +260,17 @@ int get_best_mount(char *what, const cha
 		/* p points to a server, "next is our next parse point */
 		if (!skiplocal) {
 			/* Check if it's localhost */
-			struct hostent *he;
-			char **haddr;
-
-			he = gethostbyname(p);
-			if (!he) {
-				error(MODPREFIX "host %s: lookup failure", p);
-				p = next;
-				continue;
-			}
-
-			/* Check each host in round robin list */
-			for (haddr = he->h_addr_list; *haddr; haddr++) {
-				local = is_local_addr(p, *haddr, he->h_length);
-
-				if (local < 0)
-					continue;
-
-				if (local) {
-					winner = p;
-					break;
-				}
-			}
-			
+			local = is_local_mount(p);
 			if (local < 0) {
 				local = 0;
 				p = next;
 				continue;
 			}
 
-			if (local)
+			if (local) {
+				winner = p;
 				break;
+			}
 		}
 
 		/* ping each (or the) entry to see if it's alive. */
@@ -256,7 +325,7 @@ int get_best_mount(char *what, const cha
 	 */
 	if (!local && winner_weight == INT_MAX) {
 		/* We had more than one contender and none responded in time */
-		if (winner_time != 0 && winner_time > 500) {
+		if (winner_time == 0 || winner_time > 500) {
 			/* We've already tried a longer timeout */
 			if (!longtimeout) {
 				/* Reset string and try again */
@@ -267,7 +336,7 @@ int get_best_mount(char *what, const cha
 				      "retrying with longer timeout",
 				      original);
 
-				return get_best_mount(what, original, 1, 1);
+				return get_best_mount(what, original, 1);
 			}
 		}
 	}
@@ -395,7 +464,7 @@ int mount_mount(const char *root, const 
 		/* No colon, take this as a bind (local) entry */
 		local = 1;
 	} else if (!nosymlink) {
-		local = get_best_mount(whatstr, what, 0, 0);
+		local = get_best_mount(whatstr, what, 0);
 		if (!*whatstr) {
 			warn(MODPREFIX "no host elected");
 			return 1;

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

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

Re: Re: [daniel@vision.ime.usp.br: Bug#304245: patch - NFS mounts occasionally fail]

[Chris Feist]

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

I've attached the redhat patch which should fix this problem as well.  (And 
we've tested it and it seems to work pretty well).

Thanks,
Chris

raven@themaw.net wrote:
> On Sun, 17 Apr 2005, Daniel Andre Vaquero wrote:
> 
>> Hi,
>>
>> My editor broke the indentation style in the patch; I'm resending it as
>> an attached file.
> 
> 
> Will check out the patch.
> 
> Thanks for the feedback.
> Ian
> 
> _______________________________________________
> autofs mailing list
> autofs@linux.kernel.org
> http://linux.kernel.org/mailman/listinfo/autofs


[-- Attachment #2: autofs-4.1.4-non-replicated-ping.patch --]
[-- Type: text/x-patch, Size: 5442 bytes --]

--- autofs-4.1.4/modules/mount_nfs.c.orig	2005-04-13 20:31:24.639009544 -0400
+++ autofs-4.1.4/modules/mount_nfs.c	2005-04-13 20:31:40.727563712 -0400
@@ -31,6 +31,7 @@
 #include <netinet/in.h>
 #include <linux/nfs.h>
 #include <linux/nfs2.h>
+#include <ctype.h>
 
 #define MODULE_MOUNT
 #include "automount.h"
@@ -105,28 +106,116 @@ int is_local_addr(const char *host, cons
 	
 	return 1;
 }
+
+/*
+ * If the map doesn't contain a ',' and doesn't contain more than
+ * one ':' then @what is not a multimount entry.
+ */
+static int is_multimount_entry(char *what)
+{
+	return !strchr(what, ',') && (strchr(what, ':') == strrchr(what, ':'));
+}
+
+/*
+ *  Check to see if the 'host:path' or 'host' is on the local machine
+ *  Returns < 0 if there is a host lookup problem, otherwise returns 0
+ *  if it's not a local mount, and returns > 0 if it is a local mount.
+ */
+int is_local_mount(const char *hostpath)
+{
+	struct hostent *he;
+	char **haddr;
+	char *delim;
+	char *hostname;
+	int hostnamelen;
+	int local = 0;
+
+	debug(MODPREFIX "is_local_mount: %s", hostpath);
+	delim = strpbrk(hostpath,":");
+
+	if (delim) 
+		hostnamelen = delim - hostpath; 
+	else 
+		hostnamelen = strlen(hostpath);
+
+	hostname = malloc(hostnamelen+1);
+	strncpy(hostname,hostpath,hostnamelen);
+	hostname[hostnamelen] = '\0';
+	he = gethostbyname(hostname);
+	if (!he) {
+		error(MODPREFIX "host %s: lookup failure", hostname);
+		return -1;
+	}
+
+	for (haddr = he->h_addr_list; *haddr; haddr++) {
+		local = is_local_addr(hostname, *haddr, he->h_length);
+		if (local < 0) 
+			return local;
+ 		if (local) {
+			debug(MODPREFIX "host %s: is localhost",
+					hostname);
+			return local;
+		}
+	}
+	return 0;
+}
+
 /*
  * Given a mount string, return (in the same string) the
- * best mount to use based on weight/locality/rpctime
+ * best mount to use based on locality/weight/rpctime.
+ *
+ * If longtimeout is set to 0 then we only do 100 ms pings to hosts.  In
+ * the event that this fails, we call ourself recursively with the
+ * longtimeout option set to 1.  In this case we ping for up to 10s and
+ * skip logic for detecting if a localhost has been passed. (if a local
+ * host had been passed, we would have returned that mount as the best
+ * mount.  The skipping of local maps in this case is an optimization).
+ *
  * - return -1 and what = '\0' on error,
  *           1 and what = local mount path if local bind,
  *     else  0 and what = remote mount path
  */
-int get_best_mount(char *what, const char *original, int longtimeout, int skiplocal)
+int get_best_mount(char *what, const char *original, int longtimeout)
 {
 	char *p = what;
 	char *winner = NULL;
 	int winner_weight = INT_MAX, local = 0;
 	double winner_time = 0;
-	char *delim;
+	char *delim, *pstrip;
 	int sec = (longtimeout) ? 10 : 0;
 	int micros = (longtimeout) ? 0 : 100000;
+	int skiplocal = longtimeout; /* clearly local is not available */
 
 	if (!p) {
 		*what = '\0';
 		return -1;
 	}
 
+	/*
+	 *  If only one mountpoint has been passed in, we don't need to
+	 *  do anything except strip whitespace from the end of the string.
+	 */
+	if (!is_multimount_entry(p)) {
+		for (pstrip = p+strlen(p) - 1; pstrip >= p; pstrip--) 
+			if (isspace(*pstrip))
+				*pstrip = '\0';
+
+		/* Check if the host is the localhost */
+		if (is_local_mount(p)) {
+			debug(MODPREFIX "host %s: is localhost", p);
+
+			/* Strip off hostname and ':' */
+			delim = strchr(p,':');
+			while (delim && *delim != '\0') {
+				delim++;
+				*what = *delim;
+				what++;
+			}
+			return 1;
+		}
+		return 0;
+	}
+
 	while (p && *p) {
 		char *next;
 		unsigned int ping_stat = 0;
@@ -171,37 +260,17 @@ int get_best_mount(char *what, const cha
 		/* p points to a server, "next is our next parse point */
 		if (!skiplocal) {
 			/* Check if it's localhost */
-			struct hostent *he;
-			char **haddr;
-
-			he = gethostbyname(p);
-			if (!he) {
-				error(MODPREFIX "host %s: lookup failure", p);
-				p = next;
-				continue;
-			}
-
-			/* Check each host in round robin list */
-			for (haddr = he->h_addr_list; *haddr; haddr++) {
-				local = is_local_addr(p, *haddr, he->h_length);
-
-				if (local < 0)
-					continue;
-
-				if (local) {
-					winner = p;
-					break;
-				}
-			}
-			
+			local = is_local_mount(p);
 			if (local < 0) {
 				local = 0;
 				p = next;
 				continue;
 			}
 
-			if (local)
+			if (local) {
+				winner = p;
 				break;
+			}
 		}
 
 		/* ping each (or the) entry to see if it's alive. */
@@ -256,7 +325,7 @@ int get_best_mount(char *what, const cha
 	 */
 	if (!local && winner_weight == INT_MAX) {
 		/* We had more than one contender and none responded in time */
-		if (winner_time != 0 && winner_time > 500) {
+		if (winner_time == 0 || winner_time > 500) {
 			/* We've already tried a longer timeout */
 			if (!longtimeout) {
 				/* Reset string and try again */
@@ -267,7 +336,7 @@ int get_best_mount(char *what, const cha
 				      "retrying with longer timeout",
 				      original);
 
-				return get_best_mount(what, original, 1, 1);
+				return get_best_mount(what, original, 1);
 			}
 		}
 	}
@@ -395,7 +464,7 @@ int mount_mount(const char *root, const 
 		/* No colon, take this as a bind (local) entry */
 		local = 1;
 	} else if (!nosymlink) {
-		local = get_best_mount(whatstr, what, 0, 0);
+		local = get_best_mount(whatstr, what, 0);
 		if (!*whatstr) {
 			warn(MODPREFIX "no host elected");
 			return 1;

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

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

Re: [daniel@vision.ime.usp.br: Bug#304245: patch - NFS mounts occasionally fail]

[raven]

On Sun, 17 Apr 2005, Steinar H. Gunderson wrote:

> Hi,
>
> Thanks for the patch; sending upstream for evaluation.

Has this patch been tested with multi-mount map entries?

Ian

Re: Bug#304245: patch - NFS mounts occasionally fail

[Daniel Andre Vaquero]

Hi,

On Wed, Apr 20, 2005 at 01:00:30AM +0800, raven@themaw.net wrote:
> 
> Has this patch been tested with multi-mount map entries?

I don't have experience with multi-mount map entries, but I have done a
small test with entries like this (borrowed from
http://linux.kernel.org/pipermail/autofs/2005-February/001909.html):

auto.master contains:

/testing    /etc/auto.testing

/etc/auto.testing contains:

foo     -ro,hard,intr \
        /dir/bar        server:/export/a \
        /dir/baz        server:/export/b

It seems to work (both directories were mounted) if I cd to
/testing/foo. After the expiration time they were also unmounted.

Thanks,

Daniel

Re: Bug#304245: patch - NFS mounts occasionally fail

[Ian Kent]

On Tue, 19 Apr 2005, Daniel Andre Vaquero wrote:

> Hi,
> 
> On Wed, Apr 20, 2005 at 01:00:30AM +0800, raven@themaw.net wrote:
> > 
> > Has this patch been tested with multi-mount map entries?
> 
> I don't have experience with multi-mount map entries, but I have done a
> small test with entries like this (borrowed from
> http://linux.kernel.org/pipermail/autofs/2005-February/001909.html):
> 
> auto.master contains:
> 
> /testing    /etc/auto.testing
> 
> /etc/auto.testing contains:
> 
> foo     -ro,hard,intr \
>         /dir/bar        server:/export/a \
>         /dir/baz        server:/export/b
> 
> It seems to work (both directories were mounted) if I cd to
> /testing/foo. After the expiration time they were also unmounted.
> 

Fine, but what I'm after is what happpens when the ping fails for all 
servers. Changing the host names to bogus names should be enough.

Ian

Re: Bug#304245: patch - NFS mounts occasionally fail

[Daniel Andre Vaquero]

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

On Wed, Apr 20, 2005 at 09:02:22AM +0800, Ian Kent wrote:
> 
> Fine, but what I'm after is what happpens when the ping fails for all 
> servers. Changing the host names to bogus names should be enough.

Changed /etc/auto.testing to

foo     -ro,hard,intr \
        /dir/bar        bogus:/usr/local/bin \
        /dir/baz        morebogus:/usr/local/sbin

cd /testing/foo resulted in the part of the log that follows attached.

Daniel

[-- Attachment #2: autofs.log --]
[-- Type: text/plain, Size: 4470 bytes --]

Apr 19 22:16:52 myhost automount[4357]: handle_packet: type = 0 
Apr 19 22:16:52 myhost automount[4357]: handle_packet_missing: token 299, name foo 
Apr 19 22:16:52 myhost automount[4357]: attempting to mount entry /testing/foo
Apr 19 22:16:52 myhost automount[9850]: lookup(file): foo -> -ro,hard,intr         /dir/bar        bogus:/usr/local/bin         /dir/baz        morebogus:/usr/local/sbin
Apr 19 22:16:52 myhost automount[9850]: parse(sun): expanded entry: -ro,hard,intr         /dir/bar        bogus:/usr/local/bin         /dir/baz        morebogus:/usr/local/sbin
Apr 19 22:16:52 myhost automount[9850]: parse(sun): dequote("ro,hard,intr") -> ro,hard,intr
Apr 19 22:16:52 myhost automount[9850]: parse(sun): gathered options: ro,hard,intr
Apr 19 22:16:52 myhost automount[9850]: parse(sun): dequote("/dir/bar") -> /dir/bar
Apr 19 22:16:52 myhost automount[9850]: parse(sun): dequote("bogus:/usr/local/bin") -> bogus:/usr/local/bin
Apr 19 22:16:52 myhost automount[9850]: parse(sun): dequote("/dir/baz") -> /dir/baz
Apr 19 22:16:52 myhost automount[9850]: parse(sun): dequote("morebogus:/usr/local/sbin") -> morebogus:/usr/local/sbin
Apr 19 22:16:52 myhost automount[9850]: parse(sun): multimount: morebogus:/usr/local/sbin on /dir/baz with options ro,hard,intr
Apr 19 22:16:52 myhost automount[9850]: parse(sun): mounting root /testing/foo, mountpoint dir/baz, what morebogus:/usr/local/sbin, fstype nfs, options ro,hard,intr 
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): root=/testing/foo name=dir/baz what=morebogus:/usr/local/sbin, fstype=nfs, options=ro,hard,intr
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): nfs options="ro,hard,intr", nosymlink=0, ro=1
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): host morebogus: lookup failure
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): winner = (null) local = 0
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): all hosts timed out for 'morebogus:/usr/local/sbin', retrying with longer timeout
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): winner = (null) local = 0
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): from morebogus:/usr/local/sbin elected morebogus:/usr/local/sbin
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): calling mkdir_path /testing/foo/dir/baz
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): calling mount -t nfs -s  -o ro,hard,intr morebogus:/usr/local/sbin /testing/foo/dir/baz
Apr 19 22:16:52 myhost automount[9850]: >> mount: can't get address for morebogus
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): nfs: mount failure morebogus:/usr/local/sbin on /testing/foo/dir/baz
Apr 19 22:16:52 myhost automount[9850]: parse_mount: ignoring failure of non-strict mount
Apr 19 22:16:52 myhost automount[9850]: parse(sun): multimount: bogus:/usr/local/bin on /dir/bar with options ro,hard,intr
Apr 19 22:16:52 myhost automount[9850]: parse(sun): mounting root /testing/foo, mountpoint dir/bar, what bogus:/usr/local/bin, fstype nfs, options ro,hard,intr 
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): root=/testing/foo name=dir/bar what=bogus:/usr/local/bin, fstype=nfs, options=ro,hard,intr
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): nfs options="ro,hard,intr", nosymlink=0, ro=1
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): host bogus: lookup failure
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): winner = (null) local = 0
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): all hosts timed out for 'bogus:/usr/local/bin', retrying with longer timeout
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): winner = (null) local = 0
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): from bogus:/usr/local/bin elected bogus:/usr/local/bin
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): calling mkdir_path /testing/foo/dir/bar
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): calling mount -t nfs -s  -o ro,hard,intr bogus:/usr/local/bin /testing/foo/dir/bar
Apr 19 22:16:52 myhost automount[9850]: >> mount: can't get address for bogus
Apr 19 22:16:52 myhost automount[9850]: mount(nfs): nfs: mount failure bogus:/usr/local/bin on /testing/foo/dir/bar
Apr 19 22:16:52 myhost automount[9850]: parse_mount: ignoring failure of non-strict mount
Apr 19 22:16:52 myhost automount[4357]: handle_child: got pid 9850, sig 0 (0), stat 0 
Apr 19 22:16:52 myhost automount[4357]: sig_child: found pending iop pid 9850: signalled 0 (sig 0), exit status 0
Apr 19 22:16:52 myhost automount[4357]: send_ready: token=299 

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

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

Re: Bug#304245: patch - NFS mounts occasionally fail

[raven]

On Tue, 19 Apr 2005, Daniel Andre Vaquero wrote:

> On Wed, Apr 20, 2005 at 09:02:22AM +0800, Ian Kent wrote:
>>
>> Fine, but what I'm after is what happpens when the ping fails for all
>> servers. Changing the host names to bogus names should be enough.
>
> Changed /etc/auto.testing to
>
> foo     -ro,hard,intr \
>        /dir/bar        bogus:/usr/local/bin \
>        /dir/baz        morebogus:/usr/local/sbin
>
> cd /testing/foo resulted in the part of the log that follows attached.
>

Cool.

That seems to work properly.
Thanks for doing that for me.

Ian

Re: Re: [daniel@vision.ime.usp.br: Bug#304245: patch - NFS mounts occasionally fail]

[raven]

On Mon, 18 Apr 2005, Chris Feist wrote:

> I've attached the redhat patch which should fix this problem as well.  (And 
> we've tested it and it seems to work pretty well).
>
> Thanks,
> Chris
>
> raven@themaw.net wrote:
>>  On Sun, 17 Apr 2005, Daniel Andre Vaquero wrote:
>> 
>> >  Hi,
>> > 
>> >  My editor broke the indentation style in the patch; I'm resending it as
>> >  an attached file.
>> 
>> 
>>  Will check out the patch.
>> 
>>  Thanks for the feedback.

Hi guys,

I've reviewed the two patches submitted regarding this problem.

Thanks for your efforts Daniel but the patch that Chris has submitted 
covers the issue that you reported and addresses a couple of other 
things as well.

Thanks Chris, I'll merge the patch and copy it up to kernel.org.

There was however one thing that Daniel pointed out that I added to your 
patch (hunk below). That is the selection of the first multi-mount entry 
if all else fails. I know that this was changed due to excessive mount 
delays but as your patch avoids the ping altogether for single entries and 
the doco says it behaves this way I expect it will be OK. If not I'll 
revert it.

Ian

--- autofs-4.1.4/modules/mount_nfs.c.select-first	2005-04-24 16:51:04.000000000 +0800
+++ autofs-4.1.4/modules/mount_nfs.c	2005-04-24 16:52:01.000000000 +0800
@@ -341,11 +341,9 @@ int get_best_mount(char *what, const cha
  		}
  	}

-	/* No winner found so bail */
-	if (!winner) {
-		*what = '\0';
-		return 0;
-	}
+	/* No winner found so return first */
+	if (!winner)
+		winner = what;

  	/*
  	 * We now have our winner, copy it to the front of the string,

Re: Re: [daniel@vision.ime.usp.br: Bug#304245: patch - NFS mounts occasionally fail]

[raven]

On Sun, 24 Apr 2005 raven@themaw.net wrote:

> On Mon, 18 Apr 2005, Chris Feist wrote:
>
>>  I've attached the redhat patch which should fix this problem as well. 
>>  (And we've tested it and it seems to work pretty well).
>> 
>>  Thanks,
>>  Chris
>> 
>>  raven@themaw.net wrote:
>> >   On Sun, 17 Apr 2005, Daniel Andre Vaquero wrote:
>> > 
>> > >   Hi,
>> > > 
>> > >   My editor broke the indentation style in the patch; I'm resending it 
>> > >   as
>> > >   an attached file.
>> > 
>> > 
>> >   Will check out the patch.
>> > 
>> >   Thanks for the feedback.
>
> Hi guys,
>
> I've reviewed the two patches submitted regarding this problem.
>
> Thanks for your efforts Daniel but the patch that Chris has submitted covers 
> the issue that you reported and addresses a couple of other things as well.
>
> Thanks Chris, I'll merge the patch and copy it up to kernel.org.
>
> There was however one thing that Daniel pointed out that I added to your 
> patch (hunk below). That is the selection of the first multi-mount entry if 
> all else fails. I know that this was changed due to excessive mount delays 
> but as your patch avoids the ping altogether for single entries and the doco 
> says it behaves this way I expect it will be OK. If not I'll revert it.
>

Ooops. Spotted something else.

The function is_multimount_entry should use a || as multi-mount entries 
can be like:

host1,host2:/path

or

host1:/path host2:/path

or a combination.

Ian

Re: Re: [daniel@vision.ime.usp.br: Bug#304245: patch - NFS mounts occasionally fail]

[raven]

On Sun, 24 Apr 2005 raven@themaw.net wrote:

> On Sun, 24 Apr 2005 raven@themaw.net wrote:
>
>>  On Mon, 18 Apr 2005, Chris Feist wrote:
>> 
>> >   I've attached the redhat patch which should fix this problem as well. 
>> >   (And we've tested it and it seems to work pretty well).
>> > 
>> >   Thanks,
>> >   Chris
>> > 
>> >   raven@themaw.net wrote:
>> > >    On Sun, 17 Apr 2005, Daniel Andre Vaquero wrote:
>> > > 
>> > > >    Hi,
>> > > > 
>> > > >    My editor broke the indentation style in the patch; I'm resending 
>> > > >    it as
>> > > >    an attached file.
>> > > 
>> > > 
>> > >    Will check out the patch.
>> > > 
>> > >    Thanks for the feedback.
>> 
>>  Hi guys,
>> 
>>  I've reviewed the two patches submitted regarding this problem.
>> 
>>  Thanks for your efforts Daniel but the patch that Chris has submitted 
>>  covers the issue that you reported and addresses a couple of other things 
>>  as well.
>> 
>>  Thanks Chris, I'll merge the patch and copy it up to kernel.org.
>> 
>>  There was however one thing that Daniel pointed out that I added to your 
>>  patch (hunk below). That is the selection of the first multi-mount entry 
>>  if all else fails. I know that this was changed due to excessive mount 
>>  delays but as your patch avoids the ping altogether for single entries and 
>>  the doco says it behaves this way I expect it will be OK. If not I'll 
>>  revert it.
>> 
>
> Ooops. Spotted something else.
>
> The function is_multimount_entry should use a || as multi-mount entries can 
> be like:
>
> host1,host2:/path
>
> or
>
> host1:/path host2:/path
>
> or a combination.
>
> Ian
>
>

And yet another !!

Another possibility is

key	/[dir]	host:/dir

I also added a test for *what == '/' to the conditional.

Ian

Re: Bug#304245: patch - NFS mounts occasionally fail

[Daniel Andre Vaquero]

Hi,

On Sun, Apr 24, 2005 at 05:32:52PM +0800, raven@themaw.net wrote:
> 
> Ooops. Spotted something else.
> 
> The function is_multimount_entry should use a || as multi-mount entries 
> can be like:

I would also suggest a change in the name of this function to something
like is_replicated_entry. Multi-mounts and replicated mounts are
different things, and in this case it could confuse the reader.

Thanks,

Daniel

Re: Bug#304245: patch - NFS mounts occasionally fail

[raven]

On Sun, 24 Apr 2005, Daniel Andre Vaquero wrote:

> Hi,
>
> On Sun, Apr 24, 2005 at 05:32:52PM +0800, raven@themaw.net wrote:
>>
>> Ooops. Spotted something else.
>>
>> The function is_multimount_entry should use a || as multi-mount entries
>> can be like:
>
> I would also suggest a change in the name of this function to something
> like is_replicated_entry. Multi-mounts and replicated mounts are
> different things, and in this case it could confuse the reader.

Perhaps. But they are handled by the same code.
The simple replicated mount entry is a special case of a multi-mount 
entry as far as the code is concerned. At least that's the way it is now.

Ian

Re: Bug#304245: patch - NFS mounts occasionally fail

[Daniel Andre Vaquero]

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

On Mon, Apr 25, 2005 at 02:10:14AM +0800, raven@themaw.net wrote:
> On Sun, 24 Apr 2005, Daniel Andre Vaquero wrote:
> 
> >I would also suggest a change in the name of this function to something
> >like is_replicated_entry. Multi-mounts and replicated mounts are
> >different things, and in this case it could confuse the reader.
> 
> Perhaps. But they are handled by the same code.
> The simple replicated mount entry is a special case of a multi-mount 
> entry as far as the code is concerned. At least that's the way it is now.

Sorry, I didn't see that it is now also being used in parse_sun.c. Then
I agree that it is OK.

I have tested version 4.1.4 plus the three new patches from kernel.org,
and found a bug when using bogus names in multi-mounts. The test case is:

/etc/auto.testing

foo     -ro,hard,intr \
        /dir/bar        bogus:/usr/src \
        /dir/baz        morebogus:/usr/share/doc

When the function get_best_mount tries to resolve the bogus names, there is a
host lookup failure. Then it returns the winner as local, and automount does
a mount --bind. But the correct behaviour would be a mount failure.

The corresponding log entries follows attached. I have also made and tested
a little patch to fix it (and a brace missing in
autofs-4.1.4-multi-parse-fix.patch), that is attached as well.

Daniel

[-- Attachment #2: bogus.log --]
[-- Type: text/plain, Size: 3788 bytes --]

Apr 24 20:01:51 mybox automount[18557]: attempting to mount entry /testing/foo
Apr 24 20:01:51 mybox automount[19519]: lookup(file): foo -> -ro,hard,intr         /dir/bar        bogus:/usr/src         /dir/baz        morebogus:/usr/share/doc
Apr 24 20:01:51 mybox automount[19519]: parse(sun): expanded entry: -ro,hard,intr         /dir/bar        bogus:/usr/src         /dir/baz        morebogus:/usr/share/doc
Apr 24 20:01:51 mybox automount[19519]: parse(sun): dequote("ro,hard,intr") -> ro,hard,intr
Apr 24 20:01:51 mybox automount[19519]: parse(sun): gathered options: ro,hard,intr
Apr 24 20:01:51 mybox automount[19519]: parse(sun): dequote("/dir/bar") -> /dir/bar
Apr 24 20:01:51 mybox automount[19519]: parse(sun): dequote("bogus:/usr/src") -> bogus:/usr/src
Apr 24 20:01:51 mybox automount[19519]: parse(sun): dequote("/dir/baz") -> /dir/baz
Apr 24 20:01:51 mybox automount[19519]: parse(sun): dequote("morebogus:/usr/share/doc") -> morebogus:/usr/share/doc
Apr 24 20:01:51 mybox automount[19519]: parse(sun): multimount: morebogus:/usr/share/doc on /dir/baz with options ro,hard,intr
Apr 24 20:01:51 mybox automount[19519]: parse(sun): mounting root /testing/foo, mountpoint dir/baz, what morebogus:/usr/share/doc, fstype nfs, options ro,hard,intr 
Apr 24 20:01:51 mybox automount[19519]: mount(nfs): root=/testing/foo name=dir/baz what=morebogus:/usr/share/doc, fstype=nfs, options=ro,hard,intr
Apr 24 20:01:51 mybox automount[19519]: mount(nfs): nfs options="ro,hard,intr", nosymlink=0, ro=1
Apr 24 20:01:51 mybox automount[19519]: mount(nfs): is_local_mount: morebogus:/usr/share/doc
Apr 24 20:01:51 mybox automount[19519]: mount(nfs): host morebogus: lookup failure
Apr 24 20:01:51 mybox automount[19519]: mount(nfs): host morebogus:/usr/share/doc: is localhost
Apr 24 20:01:51 mybox automount[19519]: mount(nfs): from morebogus:/usr/share/doc elected /usr/share/doc
Apr 24 20:01:51 mybox automount[19519]: mount(nfs): dir/baz is local, doing bind
Apr 24 20:01:51 mybox automount[19519]: mount(bind): calling mkdir_path /testing/foo/dir/baz
Apr 24 20:01:51 mybox automount[19519]: mount(bind): calling mount --bind -s  -o ro /usr/share/doc /testing/foo/dir/baz
Apr 24 20:01:51 mybox automount[19519]: mount(bind): mounted /usr/share/doc type bind on /testing/foo/dir/baz
Apr 24 20:01:51 mybox automount[19519]: parse(sun): multimount: bogus:/usr/src on /dir/bar with options ro,hard,intr
Apr 24 20:01:51 mybox automount[19519]: parse(sun): mounting root /testing/foo, mountpoint dir/bar, what bogus:/usr/src, fstype nfs, options ro,hard,intr 
Apr 24 20:01:51 mybox automount[19519]: mount(nfs): root=/testing/foo name=dir/bar what=bogus:/usr/src, fstype=nfs, options=ro,hard,intr
Apr 24 20:01:51 mybox automount[19519]: mount(nfs): nfs options="ro,hard,intr", nosymlink=0, ro=1
Apr 24 20:01:51 mybox automount[19519]: mount(nfs): is_local_mount: bogus:/usr/src
Apr 24 20:01:52 mybox automount[19519]: mount(nfs): host bogus: lookup failure
Apr 24 20:01:52 mybox automount[19519]: mount(nfs): host bogus:/usr/src: is localhost
Apr 24 20:01:52 mybox automount[19519]: mount(nfs): from bogus:/usr/src elected /usr/src
Apr 24 20:01:52 mybox automount[19519]: mount(nfs): dir/bar is local, doing bind
Apr 24 20:01:52 mybox automount[19519]: mount(bind): calling mkdir_path /testing/foo/dir/bar
Apr 24 20:01:52 mybox automount[19519]: mount(bind): calling mount --bind -s  -o ro /usr/src /testing/foo/dir/bar
Apr 24 20:01:52 mybox automount[19519]: mount(bind): mounted /usr/src type bind on /testing/foo/dir/bar
Apr 24 20:01:52 mybox automount[18557]: handle_child: got pid 19519, sig 0 (0), stat 0 
Apr 24 20:01:52 mybox automount[18557]: sig_child: found pending iop pid 19519: signalled 0 (sig 0), exit status 0
Apr 24 20:01:52 mybox automount[18557]: send_ready: token=1402 

[-- Attachment #3: autofs-4.1.4-non-replicated-local-fix.patch --]
[-- Type: text/plain, Size: 941 bytes --]

diff -Npru autofs-4.1.4.orig/modules/mount_nfs.c autofs-4.1.4/modules/mount_nfs.c
--- autofs-4.1.4.orig/modules/mount_nfs.c	2005-04-24 20:28:25.000000000 -0300
+++ autofs-4.1.4/modules/mount_nfs.c	2005-04-24 19:59:13.000000000 -0300
@@ -191,7 +191,7 @@ int get_best_mount(char *what, const cha
 				*pstrip = '\0';
 
 		/* Check if the host is the localhost */
-		if (is_local_mount(p)) {
+		if (is_local_mount(p) > 0) {
 			debug(MODPREFIX "host %s: is localhost", p);
 
 			/* Strip off hostname and ':' */
diff -Npru autofs-4.1.4.orig/modules/parse_sun.c autofs-4.1.4/modules/parse_sun.c
--- autofs-4.1.4.orig/modules/parse_sun.c	2005-04-24 20:28:25.000000000 -0300
+++ autofs-4.1.4/modules/parse_sun.c	2005-04-24 20:35:01.000000000 -0300
@@ -841,6 +841,7 @@ int parse_mount(const char *root, const 
 			if (*p != '/') {
 				l = 0;
 				path = dequote("/", 1);
+			}
 			else
 				path = dequote(p, l = chunklen(p, 0));
 			if (!path) {

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

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

Re: Bug#304245: patch - NFS mounts occasionally fail

[raven]

On Sun, 24 Apr 2005, Daniel Andre Vaquero wrote:

> On Mon, Apr 25, 2005 at 02:10:14AM +0800, raven@themaw.net wrote:
>> On Sun, 24 Apr 2005, Daniel Andre Vaquero wrote:
>>
>>> I would also suggest a change in the name of this function to something
>>> like is_replicated_entry. Multi-mounts and replicated mounts are
>>> different things, and in this case it could confuse the reader.
>>
>> Perhaps. But they are handled by the same code.
>> The simple replicated mount entry is a special case of a multi-mount
>> entry as far as the code is concerned. At least that's the way it is now.
>
> Sorry, I didn't see that it is now also being used in parse_sun.c. Then
> I agree that it is OK.

I've removed those patches just now.

After thinking about it I think you were right.
I'm going to leave these tests seperate and rename the test as you 
suggest.

>
> I have tested version 4.1.4 plus the three new patches from kernel.org,
> and found a bug when using bogus names in multi-mounts. The test case is:
>
> /etc/auto.testing
>
> foo     -ro,hard,intr \
>        /dir/bar        bogus:/usr/src \
>        /dir/baz        morebogus:/usr/share/doc
>
> When the function get_best_mount tries to resolve the bogus names, there is a
> host lookup failure. Then it returns the winner as local, and automount does
> a mount --bind. But the correct behaviour would be a mount failure.
>
> The corresponding log entries follows attached. I have also made and tested
> a little patch to fix it (and a brace missing in
> autofs-4.1.4-multi-parse-fix.patch), that is attached as well.

Thank you very much for that.
I will attend to it right now.

Ian

Re: Bug#304245: patch - NFS mounts occasionally fail

[raven]

On Mon, 25 Apr 2005 raven@themaw.net wrote:

> On Sun, 24 Apr 2005, Daniel Andre Vaquero wrote:
>
>>  On Mon, Apr 25, 2005 at 02:10:14AM +0800, raven@themaw.net wrote:
>> >  On Sun, 24 Apr 2005, Daniel Andre Vaquero wrote:
>> > 
>> > >  I would also suggest a change in the name of this function to 
>> > >  something
>> > >  like is_replicated_entry. Multi-mounts and replicated mounts are
>> > >  different things, and in this case it could confuse the reader.
>> > 
>> >  Perhaps. But they are handled by the same code.
>> >  The simple replicated mount entry is a special case of a multi-mount
>> >  entry as far as the code is concerned. At least that's the way it is 
>> >  now.
>> 
>>  Sorry, I didn't see that it is now also being used in parse_sun.c. Then
>>  I agree that it is OK.
>
> I've removed those patches just now.
>
> After thinking about it I think you were right.
> I'm going to leave these tests seperate and rename the test as you suggest.
>
>> 
>>  I have tested version 4.1.4 plus the three new patches from kernel.org,
>>  and found a bug when using bogus names in multi-mounts. The test case is:
>> 
>>  /etc/auto.testing
>> 
>>  foo     -ro,hard,intr \
>>         /dir/bar        bogus:/usr/src \
>>         /dir/baz        morebogus:/usr/share/doc
>> 
>>  When the function get_best_mount tries to resolve the bogus names, there 
>>  is a
>>  host lookup failure. Then it returns the winner as local, and automount 
>>  does
>>  a mount --bind. But the correct behaviour would be a mount failure.
>> 
>>  The corresponding log entries follows attached. I have also made and 
>>  tested
>>  a little patch to fix it (and a brace missing in
>>  autofs-4.1.4-multi-parse-fix.patch), that is attached as well.
>
> Thank you very much for that.
> I will attend to it right now.

All done.
It should be alright now, I hope.

Ian

Re: Bug#304245: patch - NFS mounts occasionally fail

[Daniel Andre Vaquero]

On Mon, Apr 25, 2005 at 12:12:32PM +0800, raven@themaw.net wrote:
> 
> All done.
> It should be alright now, I hope.

It seems to work properly. Thanks.

Daniel