diff mbox

[7/8] mount: don't treat temporary name resolution failure as permanent.

Message ID 20160714022643.5874.27102.stgit@noble (mailing list archive)
State New, archived
Headers show

Commit Message

NeilBrown July 14, 2016, 2:26 a.m. UTC
If getaddrinfo() returns EAI_AGAIN, we shouldn't just give up, but
should continue normal retries as the nameserver may be unavailable
for the same reason as the NFS server.

So move the getaddrinfo() call from nfs_validate_options() into
nfs_try_mounts() which is always called soon after, except in the
'remount' case when we don't want it anyway.

If EAI_AGAIN is returned, set errno to EAGAIN and allow this to be a
temporary failure.  Otherwise report error and set errno to EALREADY
so no further message is given.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 utils/mount/stropts.c |   54 ++++++++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 23 deletions(-)



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

Comments

NeilBrown July 19, 2016, 11:01 p.m. UTC | #1
On Thu, Jul 14 2016, NeilBrown wrote:

> If getaddrinfo() returns EAI_AGAIN, we shouldn't just give up, but
> should continue normal retries as the nameserver may be unavailable
> for the same reason as the NFS server.
>
> So move the getaddrinfo() call from nfs_validate_options() into
> nfs_try_mounts() which is always called soon after, except in the
> 'remount' case when we don't want it anyway.
>
> If EAI_AGAIN is returned, set errno to EAGAIN and allow this to be a
> temporary failure.  Otherwise report error and set errno to EALREADY
> so no further message is given.
>
> Signed-off-by: NeilBrown <neilb@suse.com>


> +
> +		if (!nfs_append_addr_option(mi->address->ai_addr,
> +					    mi->address->ai_addrlen, mi->options))
> +			return 0;
> +		mi->address = address;

This patch is badly buggy.  I'm de-referencing mi->address before
assigning it.  How did that ever pass quality control I ask myself.
(fortunately SUSE does better QA than I do).

I'll submit a revised version ... maybe put all the patches affecting
the mountpoint export option at the end of the list.

NeilBrown
diff mbox

Patch

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index d60b484ab960..522c7ae015f1 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -84,6 +84,7 @@  struct nfsmount_info {
 				*type;		/* "nfs" or "nfs4" */
 	char			*hostname;	/* server's hostname */
 	struct addrinfo		*address;	/* server's addresses */
+	sa_family_t		family;		/* Address family */
 
 	struct mount_options	*options;	/* parsed mount options */
 	char			**extra_opts;	/* string for /etc/mtab */
@@ -371,39 +372,19 @@  static int nfs_set_version(struct nfsmount_info *mi)
  */
 static int nfs_validate_options(struct nfsmount_info *mi)
 {
-	struct addrinfo hint = {
-		.ai_protocol	= (int)IPPROTO_UDP,
-	};
-	sa_family_t family;
-	int error;
-
 	if (!nfs_parse_devname(mi->spec, &mi->hostname, NULL))
 		return 0;
 
-	if (!nfs_nfs_proto_family(mi->options, &family))
+	if (!nfs_nfs_proto_family(mi->options, &mi->family))
 		return 0;
 
 	/*
 	 * A remount is not going to be able to change the server's address,
 	 * nor should we try to resolve another address for the server as we
 	 * may end up with a different address.
+	 * A non-remount will set 'addr' from ->hostname
 	 */
-	if (mi->flags & MS_REMOUNT) {
-		po_remove_all(mi->options, "addr");
-	} else {
-		hint.ai_family = (int)family;
-		error = getaddrinfo(mi->hostname, NULL, &hint, &mi->address);
-		if (error != 0) {
-			nfs_error(_("%s: Failed to resolve server %s: %s"),
-				progname, mi->hostname, gai_strerror(error));
-			mi->address = NULL;
-			return 0;
-		}
-
-		if (!nfs_append_addr_option(mi->address->ai_addr,
-						mi->address->ai_addrlen, mi->options))
-			return 0;
-	}
+	po_remove_all(mi->options, "addr");
 
 	if (!nfs_set_version(mi))
 		return 0;
@@ -903,6 +884,32 @@  static int nfs_try_mount(struct nfsmount_info *mi)
 {
 	int result = 0;
 
+	if (mi->address == NULL) {
+		struct addrinfo hint = {
+			.ai_protocol	= (int)IPPROTO_UDP,
+		};
+		int error;
+		struct addrinfo *address;
+
+		hint.ai_family = (int)mi->family;
+		error = getaddrinfo(mi->hostname, NULL, &hint, &address);
+		if (error != 0) {
+			if (error == EAI_AGAIN)
+				errno = EAGAIN;
+			else {
+				nfs_error(_("%s: Failed to resolve server %s: %s"),
+					  progname, mi->hostname, gai_strerror(error));
+				errno = EALREADY;
+			}
+			return 0;
+		}
+
+		if (!nfs_append_addr_option(mi->address->ai_addr,
+					    mi->address->ai_addrlen, mi->options))
+			return 0;
+		mi->address = address;
+	}
+
 	switch (mi->version.major) {
 		case 2:
 		case 3:
@@ -941,6 +948,7 @@  static int nfs_is_permanent_error(int error)
 	case ETIMEDOUT:
 	case ECONNREFUSED:
 	case EHOSTUNREACH:
+	case EAGAIN:
 		return 0;	/* temporary */
 	default:
 		return 1;	/* permanent */