[PATCH 00/31] mount.nfs patches for next nfs-utils release

[PATCH 00/31] mount.nfs patches for next nfs-utils release

[Chuck Lever]

Hi Steve-

This series contains some patches I've had for a little while, and a
bunch of new fixes I made last week to address RH bugzillas 486266 and
505535.

In the short term, these patches should address most of the recent
complaints about the new mount.nfs.  Text-based mounts should behave
a lot more like legacy mounts -- fewer hangs and better negotiation
semantics.

Longer term, we are planning to sink the mount version/transport
negotiation into the kernel.

---

Chuck Lever (31):
      mount.nfs: Squelch compiler warnings in nfs_strerror()
      mount.nfs: Squelch unused parameter warnings on empty functions
      mount.nfs: Fix compiler warning in stropts.c
      umount.nfs: Use correct data type in nfsumount()
      mount.nfs: remove unused @addrlen argument from nfs_string_to_sockaddr()
      mount.nfs: Remove unused @salen parameter from nfs_ca_gai()
      mount.nfs: Fix some nfs_error() nits in network.c
      mount.nfs: Remove unused parameter in try_mount()
      mount.nfs: Use correct data type in discover_nfs_mount_data_version()
      support: Introduce sockaddr helpers to get and set IP port numbers
      mount.nfs: Don't update extra_opts after text-based negotiation
      mount.nfs: Clean up after restructuring version/protocol negotiation
      mount.nfs: Clean up nfs_is_permanent_error()
      mount.nfs: rearchitect mount version/protocol negotiation logic
      mount.nfs: make nfs_options2pmap return errors
      mount.nfs: force rpcbind queries if options aren't specified
      mount.nfs: If port= specifies an unregistered port, retry, then fail
      getport: Convert TCP connection refused to RPC_CANTRECV
      getport: Restore historical TCP connect timeout error code
      mount.nfs: Add more debugging output around nfs_getport()
      getport: Clear shared error fields before trying rpcbind queries
      getport: RPC_PROGNOTREGISTERED is a permanent error
      support: Set proper retransmit timeout for datagram transports
      support: Don't return RPC_UNKNOWNHOST from rpc_socket.c
      support: Use HAVE_LIBTIRPC to switch in bindresvport_sa(3t)
      New versions of libtool add extra aclocal scripts
      getport: Remove unneeded @salen arguments
      getport: replace getnameinfo(NI_NUMERICHOST) with inet_ntop(3)
      getport: Remove AI_ADDRCONFIG from nfs_gp_loopback_address()
      getport: RPCB_GETADDR's r_addr should contain rpcbind port, not zero
      getport: RPCB_GETADDR r_owner should be an empty string


 .gitignore               |    5 +
 support/include/nfsrpc.h |   23 ++-
 support/nfs/getport.c    |  276 +++++++++++++++------------------
 support/nfs/rpc_socket.c |   77 +++++++--
 utils/mount/error.c      |   12 +
 utils/mount/error.h      |    2 
 utils/mount/fstab.c      |    2 
 utils/mount/mount.c      |    8 -
 utils/mount/network.c    |  380 +++++++++++++++++++++++++++++++---------------
 utils/mount/network.h    |    5 -
 utils/mount/nfsumount.c  |    7 +
 utils/mount/parse_dev.c  |    5 -
 utils/mount/stropts.c    |  250 ++++++++++--------------------
 13 files changed, 560 insertions(+), 492 deletions(-)

-- 
Chuck Lever <chuck.lever@oracle.com>

[PATCH 01/31] getport: RPCB_GETADDR r_owner should be an empty string

[Chuck Lever]

Some servers reject RPCB_GETADDR requests with a non-empty r_owner
field.  "RPC: Server can't decode arguments"

An empty string is already used by libtirpc and the kernel
for RPCB_GETADDR requests.

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

 support/nfs/getport.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/support/nfs/getport.c b/support/nfs/getport.c
index cf1677e..926bab8 100644
--- a/support/nfs/getport.c
+++ b/support/nfs/getport.c
@@ -458,10 +458,6 @@ static int nfs_gp_ping(CLIENT *client, struct timeval timeout)
 /*
  * Initialize the rpcb argument for a GETADDR request.
  *
- * The rpcbind daemon ignores the parms.r_owner field in GETADDR
- * requests, but we plant an eye-catcher to help distinguish these
- * requests in network traces.
- *
  * Returns 1 if successful, and caller must free strings pointed
  * to by r_netid and r_addr; otherwise 0.
  */
@@ -489,7 +485,7 @@ static int nfs_gp_init_rpcb_parms(const struct sockaddr *sap,
 	parms->r_vers	= version;
 	parms->r_netid	= netid;
 	parms->r_addr	= addr;
-	parms->r_owner	= "nfs-utils";	/* eye-catcher */
+	parms->r_owner	= "";
 
 	return 1;
 }

[PATCH 02/31] getport: RPCB_GETADDR's r_addr should contain rpcbind port, not zero

[Chuck Lever]

Similar to a change made to the kernel's rpcbind client.  See
kernel commit 143b6c4008a7928de7e139c3a77a90e4cad8db2c.

The r_addr argument of RPCB_GETADDR procedures contains the
rpcbind server's address and port number.

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

 support/nfs/getport.c |   37 +++++++++++++++++++++++--------------
 1 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/support/nfs/getport.c b/support/nfs/getport.c
index 926bab8..2b80dce 100644
--- a/support/nfs/getport.c
+++ b/support/nfs/getport.c
@@ -201,7 +201,7 @@ static in_port_t nfs_gp_get_rpcb_port(const unsigned short protocol)
  * client.  Otherwise returns NULL, and rpc_createerr.cf_stat is set to
  * reflect the error.
  */
-static CLIENT *nfs_gp_get_rpcbclient(const struct sockaddr *sap,
+static CLIENT *nfs_gp_get_rpcbclient(struct sockaddr *sap,
 				     const socklen_t salen,
 				     const unsigned short transport,
 				     const rpcvers_t version,
@@ -214,14 +214,10 @@ static CLIENT *nfs_gp_get_rpcbclient(const struct sockaddr *sap,
 		"sunrpc",
 		NULL,
 	};
-	struct sockaddr_storage address;
-	struct sockaddr *saddr = (struct sockaddr *)&address;
 	rpcprog_t rpcb_prog = nfs_getrpcbyname(RPCBPROG, rpcb_pgmtbl);
 
-	memcpy(saddr, sap, (size_t)salen);
-	nfs_gp_set_port(saddr, nfs_gp_get_rpcb_port(transport));
-
-	return nfs_get_rpcclient(saddr, salen, transport, rpcb_prog,
+	nfs_gp_set_port(sap, nfs_gp_get_rpcb_port(transport));
+	return nfs_get_rpcclient(sap, salen, transport, rpcb_prog,
 					version, timeout);
 }
 
@@ -663,7 +659,7 @@ static unsigned short nfs_gp_getport(CLIENT *client,
 }
 
 /**
- * nfs_rcp_ping - Determine if RPC service is responding to requests
+ * nfs_rpc_ping - Determine if RPC service is responding to requests
  * @sap: pointer to address of server to query (port is already filled in)
  * @salen: length of server address
  * @program: requested RPC program number
@@ -678,6 +674,8 @@ int nfs_rpc_ping(const struct sockaddr *sap, const socklen_t salen,
 		 const rpcprog_t program, const rpcvers_t version,
 		 const unsigned short protocol, const struct timeval *timeout)
 {
+	struct sockaddr_storage address;
+	struct sockaddr *saddr = (struct sockaddr *)&address;
 	CLIENT *client;
 	struct timeval tout = { -1, 0 };
 	int result = 0;
@@ -685,7 +683,9 @@ int nfs_rpc_ping(const struct sockaddr *sap, const socklen_t salen,
 	if (timeout != NULL)
 		tout = *timeout;
 
-	client = nfs_get_rpcclient(sap, salen, protocol, program, version, &tout);
+	memcpy(saddr, sap, (size_t)salen);
+	client = nfs_get_rpcclient(saddr, salen, protocol,
+						program, version, &tout);
 	if (client != NULL) {
 		result = nfs_gp_ping(client, tout);
 		CLNT_DESTROY(client);
@@ -740,14 +740,17 @@ unsigned short nfs_getport(const struct sockaddr *sap,
 			   const rpcvers_t version,
 			   const unsigned short protocol)
 {
+	struct sockaddr_storage address;
+	struct sockaddr *saddr = (struct sockaddr *)&address;
 	struct timeval timeout = { -1, 0 };
 	unsigned short port = 0;
 	CLIENT *client;
 
-	client = nfs_gp_get_rpcbclient(sap, salen, protocol,
+	memcpy(saddr, sap, (size_t)salen);
+	client = nfs_gp_get_rpcbclient(saddr, salen, protocol,
 						default_rpcb_version, &timeout);
 	if (client != NULL) {
-		port = nfs_gp_getport(client, sap, salen, program,
+		port = nfs_gp_getport(client, saddr, salen, program,
 					version, protocol, timeout);
 		CLNT_DESTROY(client);
 	}
@@ -930,6 +933,8 @@ unsigned short nfs_rpcb_getaddr(const struct sockaddr *sap,
 				const unsigned short protocol,
 				const struct timeval *timeout)
 {
+	struct sockaddr_storage address;
+	struct sockaddr *saddr = (struct sockaddr *)&address;
 	CLIENT *client;
 	struct rpcb parms;
 	struct timeval tout = { -1, 0 };
@@ -938,7 +943,9 @@ unsigned short nfs_rpcb_getaddr(const struct sockaddr *sap,
 	if (timeout != NULL)
 		tout = *timeout;
 
-	client = nfs_gp_get_rpcbclient(sap, salen, transport, RPCBVERS_4, &tout);
+	memcpy(saddr, sap, (size_t)salen);
+	client = nfs_gp_get_rpcbclient(saddr, salen, transport,
+							RPCBVERS_4, &tout);
 	if (client != NULL) {
 		if (nfs_gp_init_rpcb_parms(addr, addrlen, program, version,
 						protocol, &parms) != 0) {
@@ -1004,6 +1011,8 @@ unsigned long nfs_pmap_getport(const struct sockaddr_in *sin,
 			       const unsigned long protocol,
 			       const struct timeval *timeout)
 {
+	struct sockaddr_in address;
+	struct sockaddr *saddr = (struct sockaddr *)&address;
 	CLIENT *client;
 	struct pmap parms = {
 		.pm_prog	= program,
@@ -1016,8 +1025,8 @@ unsigned long nfs_pmap_getport(const struct sockaddr_in *sin,
 	if (timeout != NULL)
 		tout = *timeout;
 
-	client = nfs_gp_get_rpcbclient((struct sockaddr *)sin,
-					(socklen_t)sizeof(*sin),
+	memcpy(saddr, sin, sizeof(address));
+	client = nfs_gp_get_rpcbclient(saddr, (socklen_t)sizeof(*sin),
 					transport, PMAPVERS, &tout);
 	if (client != NULL) {
 		port = nfs_gp_pmap_getport(client, &parms, tout);

[PATCH 03/31] getport: Remove AI_ADDRCONFIG from nfs_gp_loopback_address()

[Chuck Lever]

AI_ADDRCONFIG was used ostensibly to figure out if the local system
had IPv6 available when generating a loopback address.

A legacy version of nfs_gp_loopback_address() was created to handle
ANYADDR address generation for old versions of glibc where
AI_ADDRCONFIG doesn't exist.  This means we have to be careful to
test both the normal and legacy versions when committing changes in
this area.

But it turns out that even contemporary versions of glibc ignore
AI_ADDRCONFIG when the hostname string is NULL.  getaddrinfo(3)
always returns an AF_INET and an AF_INET6 loopback address in this
case, no matter how the system is configured.

Change nfs_gp_loopback_address() to have one version that simply looks
up "localhost" instead of doing anything fancy.  If "localhost" is an
IPv6 address, we'll use that.  Otherwise, it should nearly always be
an AF_INET loopback address.

This eliminates the need for AI_ADDRCONFIG, and removes the duplicate
version of nfs_gp_loopback_address().  Note that callers never used
the port number in the returned socket address, so get rid of the
"sunrpc" service string too.

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

 support/nfs/getport.c |   54 +++++++++----------------------------------------
 1 files changed, 10 insertions(+), 44 deletions(-)

diff --git a/support/nfs/getport.c b/support/nfs/getport.c
index 2b80dce..9d85e89 100644
--- a/support/nfs/getport.c
+++ b/support/nfs/getport.c
@@ -65,59 +65,32 @@ static const rpcvers_t default_rpcb_version = RPCBVERS_4;
 static const rpcvers_t default_rpcb_version = PMAPVERS;
 #endif	/* !HAVE_LIBTIRPC */
 
-#ifdef HAVE_DECL_AI_ADDRCONFIG
 /*
- * getaddrinfo(3) generates a usable loopback address based on how the
- * local network interfaces are configured.  RFC 3484 requires that the
- * results are sorted so that the first result has the best likelihood
- * of working, so we try just that first result.
+ * There's no easy way to tell how the local system's networking
+ * and rpcbind is configured (ie. whether we want to use IPv6 or
+ * IPv4 loopback to contact RPC services on the local host).  We
+ * punt and simply try to look up "localhost".
  *
  * Returns TRUE on success.
  */
 static int nfs_gp_loopback_address(struct sockaddr *sap, socklen_t *salen)
 {
 	struct addrinfo *gai_results;
-	struct addrinfo gai_hint = {
-		.ai_flags	= AI_ADDRCONFIG,
-	};
-	socklen_t len = *salen;
 	int ret = 0;
 
-	if (getaddrinfo(NULL, "sunrpc", &gai_hint, &gai_results))
+	if (getaddrinfo("localhost", NULL, NULL, &gai_results))
 		return 0;
 
-	switch (gai_results->ai_addr->sa_family) {
-	case AF_INET:
-	case AF_INET6:
-		if (len >= gai_results->ai_addrlen) {
-			memcpy(sap, gai_results->ai_addr,
-					gai_results->ai_addrlen);
-			*salen = gai_results->ai_addrlen;
-			ret = 1;
-		}
+	if (*salen >= gai_results->ai_addrlen) {
+		memcpy(sap, gai_results->ai_addr,
+				gai_results->ai_addrlen);
+		*salen = gai_results->ai_addrlen;
+		ret = 1;
 	}
 
 	freeaddrinfo(gai_results);
 	return ret;
 }
-#else
-/*
- * Old versions of getaddrinfo(3) don't support AI_ADDRCONFIG, so we
- * have a fallback for building on legacy systems.
- */
-static int nfs_gp_loopback_address(struct sockaddr *sap, socklen_t *salen)
-{
-	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
-
-	memset(sin, 0, sizeof(*sin));
-
-	sin->sin_family = AF_INET;
-	sin->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
-	*salen = sizeof(*sin);
-
-	return 1;
-}
-#endif
 
 /*
  * Plant port number in @sap.  @port is already in network byte order.
@@ -839,13 +812,6 @@ int nfs_getport_ping(struct sockaddr *sap, const socklen_t salen,
  * isn't listening on /var/run/rpcbind.sock), send a query via UDP to localhost
  * (UDP doesn't leave a socket in TIME_WAIT, and the timeout is a relatively
  * short 3 seconds).
- *
- * getaddrinfo(3) generates a usable loopback address.  RFC 3484 requires that
- * the results are sorted so that the first result has the best likelihood of
- * working, so we try just that first result.  If IPv6 is all that is
- * available, we are sure to generate an AF_INET6 loopback address and use
- * rpcbindv4/v3 GETADDR.  AF_INET6 requests go via rpcbind v4/3 in order to
- * detect if the requested RPC service supports AF_INET6 or not.
  */
 unsigned short nfs_getlocalport(const rpcprot_t program,
 				const rpcvers_t version,

[PATCH 04/31] getport: replace getnameinfo(NI_NUMERICHOST) with inet_ntop(3)

[Chuck Lever]

getnameinfo(3) with the NI_NUMERICHOST flag is used in
support/nfs/getport.c to convert socket addresses to universal address
strings.

Older versions of glibc do not have getnameinfo(3), however.  In order
for nfs-utils to build on older systems we switch in legacy code via
HAVE_GETNAMEINFO and use inet_ntoa(3).

A problem with this is that we have to double our test matrix to be
sure that both versions of these routines build and operate correctly.
Another minor problem is that inet_ntoa(3) is officially deprecated.

So let's always use a single implementation based on inet_ntop(3).
Universal address strings do not support link-local / scope IDs, so we
don't lose any functionality by using inet_ntop(3) here.

This means we open code a bit of logic that is available in most
modern versions of glibc, but in return we can use exactly the same
code for all builds (on systems with getnameinfo(3) and without).

An additional benefit is we can avoid using NI_MAXHOST for character
buffers that live on the stack: it's 1025 bytes.  Instead,
INET6_ADDRSTRLEN is used, which is just 46 bytes, plus an additional
eight bytes for the port information.  We add beefier buffer overflow
detection logic as well.

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

 support/nfs/getport.c |   70 ++++++++++++++++---------------------------------
 1 files changed, 23 insertions(+), 47 deletions(-)

diff --git a/support/nfs/getport.c b/support/nfs/getport.c
index 9d85e89..056d3c7 100644
--- a/support/nfs/getport.c
+++ b/support/nfs/getport.c
@@ -330,81 +330,57 @@ int nfs_universal2port(const char *uaddr)
  * the returned string.  Otherwise NULL is returned and
  * rpc_createerr.cf_stat is set to reflect the error.
  *
+ * inet_ntop(3) is used here, since getnameinfo(3) is not available
+ * in some earlier glibc releases, and we don't require support for
+ * scope IDs for universal addresses.
  */
-#ifdef HAVE_GETNAMEINFO
-
 char *nfs_sockaddr2universal(const struct sockaddr *sap,
 			     const socklen_t salen)
 {
-	struct sockaddr_un *sun = (struct sockaddr_un *)sap;
-	char buf[NI_MAXHOST];
+	const struct sockaddr_in6 *sin6 = (const struct sockaddr_in6 *)sap;
+	const struct sockaddr_un *sun = (const struct sockaddr_un *)sap;
+	const struct sockaddr_in *sin = (const struct sockaddr_in *)sap;
+	char buf[INET6_ADDRSTRLEN + 8 /* for port information */];
 	uint16_t port;
+	size_t count;
+	char *result;
+	int len;
 
 	switch (sap->sa_family) {
 	case AF_LOCAL:
 		return strndup(sun->sun_path, sizeof(sun->sun_path));
 	case AF_INET:
-		if (getnameinfo(sap, salen, buf, (socklen_t)sizeof(buf),
-					NULL, 0, NI_NUMERICHOST) != 0)
+		if (inet_ntop(AF_INET, (const void *)&sin->sin_addr.s_addr,
+					buf, (socklen_t)sizeof(buf)) == NULL)
 			goto out_err;
-		port = ntohs(((struct sockaddr_in *)sap)->sin_port);
+		port = ntohs(sin->sin_port);
 		break;
 	case AF_INET6:
-		if (getnameinfo(sap, salen, buf, (socklen_t)sizeof(buf),
-					NULL, 0, NI_NUMERICHOST) != 0)
+		if (inet_ntop(AF_INET6, (const void *)&sin6->sin6_addr,
+					buf, (socklen_t)sizeof(buf)) == NULL)
 			goto out_err;
-		port = ntohs(((struct sockaddr_in6 *)sap)->sin6_port);
+		port = ntohs(sin6->sin6_port);
 		break;
 	default:
 		goto out_err;
 	}
 
-	(void)snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), ".%u.%u",
+	count = sizeof(buf) - strlen(buf);
+	len = snprintf(buf + strlen(buf), count, ".%u.%u",
 			(unsigned)(port >> 8), (unsigned)(port & 0xff));
-
-	return strdup(buf);
-
-out_err:
-	rpc_createerr.cf_stat = RPC_N2AXLATEFAILURE;
-	return NULL;
-}
-
-#else	/* HAVE_GETNAMEINFO */
-
-char *nfs_sockaddr2universal(const struct sockaddr *sap,
-			     const socklen_t salen)
-{
-	struct sockaddr_un *sun = (struct sockaddr_un *)sap;
-	char buf[NI_MAXHOST];
-	uint16_t port;
-	char *addr;
-
-	switch (sap->sa_family) {
-	case AF_LOCAL:
-		return strndup(sun->sun_path, sizeof(sun->sun_path));
-	case AF_INET:
-		addr = inet_ntoa(((struct sockaddr_in *)sap)->sin_addr);
-		if (addr != NULL && strlen(addr) > sizeof(buf))
-			goto out_err;
-		strcpy(buf, addr);
-		port = ntohs(((struct sockaddr_in *)sap)->sin_port);
-		break;
-	default:
+	/* before glibc 2.0.6, snprintf(3) could return -1 */
+	if (len < 0 || (size_t)len > count)
 		goto out_err;
-	}
 
-	(void)snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), ".%u.%u",
-			(unsigned)(port >> 8), (unsigned)(port & 0xff));
-
-	return strdup(buf);
+	result = strdup(buf);
+	if (result != NULL)
+		return result;
 
 out_err:
 	rpc_createerr.cf_stat = RPC_N2AXLATEFAILURE;
 	return NULL;
 }
 
-#endif	/* HAVE_GETNAMEINFO */
-
 /*
  * Send a NULL request to the indicated RPC service.
  *

[PATCH 05/31] getport: Remove unneeded @salen arguments

[Chuck Lever]

Clean up: Now that getnameinfo(3) is no longer used, the @salen
argument to nfs_sockaddr2universal() is no longer needed.

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

 support/include/nfsrpc.h |    4 +---
 support/nfs/getport.c    |   42 +++++++++++++++++-------------------------
 2 files changed, 18 insertions(+), 28 deletions(-)

diff --git a/support/include/nfsrpc.h b/support/include/nfsrpc.h
index 543c35b..da3b4df 100644
--- a/support/include/nfsrpc.h
+++ b/support/include/nfsrpc.h
@@ -73,8 +73,7 @@ extern CLIENT		*nfs_get_priv_rpcclient( const struct sockaddr *,
 /*
  * Convert a socket address to a universal address
  */
-extern char		*nfs_sockaddr2universal(const struct sockaddr *,
-				const socklen_t);
+extern char		*nfs_sockaddr2universal(const struct sockaddr *);
 
 /*
  * Extract port number from a universal address
@@ -114,7 +113,6 @@ extern unsigned short	nfs_rpcb_getaddr(const struct sockaddr *,
 				const socklen_t,
 				const unsigned short,
 				const struct sockaddr *,
-				const socklen_t,
 				const rpcprog_t,
 				const rpcvers_t,
 				const unsigned short,
diff --git a/support/nfs/getport.c b/support/nfs/getport.c
index 056d3c7..bc0f121 100644
--- a/support/nfs/getport.c
+++ b/support/nfs/getport.c
@@ -321,7 +321,6 @@ int nfs_universal2port(const char *uaddr)
 /**
  * nfs_sockaddr2universal - convert a sockaddr to a "universal address"
  * @sap: pointer to a socket address
- * @salen: length of socket address
  *
  * Universal addresses (defined in RFC 1833) are used when calling an
  * rpcbind daemon via protocol versions 3 or 4..
@@ -334,8 +333,7 @@ int nfs_universal2port(const char *uaddr)
  * in some earlier glibc releases, and we don't require support for
  * scope IDs for universal addresses.
  */
-char *nfs_sockaddr2universal(const struct sockaddr *sap,
-			     const socklen_t salen)
+char *nfs_sockaddr2universal(const struct sockaddr *sap)
 {
 	const struct sockaddr_in6 *sin6 = (const struct sockaddr_in6 *)sap;
 	const struct sockaddr_un *sun = (const struct sockaddr_un *)sap;
@@ -407,7 +405,6 @@ static int nfs_gp_ping(CLIENT *client, struct timeval timeout)
  * to by r_netid and r_addr; otherwise 0.
  */
 static int nfs_gp_init_rpcb_parms(const struct sockaddr *sap,
-				  const socklen_t salen,
 				  const rpcprog_t program,
 				  const rpcvers_t version,
 				  const unsigned short protocol,
@@ -419,7 +416,7 @@ static int nfs_gp_init_rpcb_parms(const struct sockaddr *sap,
 	if (netid == NULL)
 		return 0;
 
-	addr = nfs_sockaddr2universal(sap, salen);
+	addr = nfs_sockaddr2universal(sap);
 	if (addr == NULL) {
 		free(netid);
 		return 0;
@@ -540,7 +537,6 @@ static unsigned long nfs_gp_pmap_getport(CLIENT *client,
 
 static unsigned short nfs_gp_getport_rpcb(CLIENT *client,
 					  const struct sockaddr *sap,
-					  const socklen_t salen,
 					  const rpcprog_t program,
 					  const rpcvers_t version,
 					  const unsigned short protocol,
@@ -549,8 +545,8 @@ static unsigned short nfs_gp_getport_rpcb(CLIENT *client,
 	unsigned short port = 0;
 	struct rpcb parms;
 
-	if (nfs_gp_init_rpcb_parms(sap, salen, program,
-					version, protocol, &parms) != 0) {
+	if (nfs_gp_init_rpcb_parms(sap, program, version,
+					protocol, &parms) != 0) {
 		port = nfs_gp_rpcb_getaddr(client, &parms, timeout);
 		nfs_gp_free_rpcb_parms(&parms);
 	}
@@ -586,7 +582,6 @@ static unsigned long nfs_gp_getport_pmap(CLIENT *client,
  */
 static unsigned short nfs_gp_getport(CLIENT *client,
 				     const struct sockaddr *sap,
-				     const socklen_t salen,
 				     const rpcprog_t program,
 				     const rpcvers_t version,
 				     const unsigned short protocol,
@@ -595,7 +590,7 @@ static unsigned short nfs_gp_getport(CLIENT *client,
 	switch (sap->sa_family) {
 #ifdef HAVE_LIBTIRPC
 	case AF_INET6:
-		return nfs_gp_getport_rpcb(client, sap, salen, program,
+		return nfs_gp_getport_rpcb(client, sap, program,
 						version, protocol, timeout);
 #endif	/* HAVE_LIBTIRPC */
 	case AF_INET:
@@ -699,7 +694,7 @@ unsigned short nfs_getport(const struct sockaddr *sap,
 	client = nfs_gp_get_rpcbclient(saddr, salen, protocol,
 						default_rpcb_version, &timeout);
 	if (client != NULL) {
-		port = nfs_gp_getport(client, saddr, salen, program,
+		port = nfs_gp_getport(client, saddr, program,
 					version, protocol, timeout);
 		CLNT_DESTROY(client);
 	}
@@ -737,7 +732,7 @@ int nfs_getport_ping(struct sockaddr *sap, const socklen_t salen,
 	client = nfs_gp_get_rpcbclient(sap, salen, protocol,
 						default_rpcb_version, &timeout);
 	if (client != NULL) {
-		port = nfs_gp_getport(client, sap, salen, program,
+		port = nfs_gp_getport(client, sap, program,
 					version, protocol, timeout);
 		CLNT_DESTROY(client);
 		client = NULL;
@@ -812,7 +807,7 @@ unsigned short nfs_getlocalport(const rpcprot_t program,
 	if (client != NULL) {
 		struct rpcb parms;
 
-		if (nfs_gp_init_rpcb_parms(sap, salen, program, version,
+		if (nfs_gp_init_rpcb_parms(sap, program, version,
 						protocol, &parms) != 0) {
 			port = nfs_gp_rpcb_getaddr(client, &parms, timeout);
 			nfs_gp_free_rpcb_parms(&parms);
@@ -838,7 +833,6 @@ unsigned short nfs_getlocalport(const rpcprot_t program,
  * @salen: length of server address
  * @transport: transport protocol to use for the query
  * @addr: pointer to r_addr address
- * @addrlen: length of address
  * @program: requested RPC program number
  * @version: requested RPC version number
  * @protocol: requested IPPROTO_ value of transport protocol
@@ -869,7 +863,6 @@ unsigned short nfs_rpcb_getaddr(const struct sockaddr *sap,
 				const socklen_t salen,
 				const unsigned short transport,
 				const struct sockaddr *addr,
-				const socklen_t addrlen,
 				const rpcprog_t program,
 				const rpcvers_t version,
 				const unsigned short protocol,
@@ -889,7 +882,7 @@ unsigned short nfs_rpcb_getaddr(const struct sockaddr *sap,
 	client = nfs_gp_get_rpcbclient(saddr, salen, transport,
 							RPCBVERS_4, &tout);
 	if (client != NULL) {
-		if (nfs_gp_init_rpcb_parms(addr, addrlen, program, version,
+		if (nfs_gp_init_rpcb_parms(addr, program, version,
 						protocol, &parms) != 0) {
 			port = nfs_gp_rpcb_getaddr(client, &parms, tout);
 			nfs_gp_free_rpcb_parms(&parms);
@@ -902,15 +895,14 @@ unsigned short nfs_rpcb_getaddr(const struct sockaddr *sap,
 
 #else	/* !HAVE_LIBTIRPC */
 
-unsigned short nfs_rpcb_getaddr(const struct sockaddr *sap,
-				const socklen_t salen,
-				const unsigned short transport,
-				const struct sockaddr *addr,
-				const socklen_t addrlen,
-				const rpcprog_t program,
-				const rpcvers_t version,
-				const unsigned short protocol,
-				const struct timeval *timeout)
+unsigned short nfs_rpcb_getaddr(__attribute__((unused)) const struct sockaddr *sap,
+				__attribute__((unused)) const socklen_t salen,
+				__attribute__((unused)) const unsigned short transport,
+				__attribute__((unused)) const struct sockaddr *addr,
+				__attribute__((unused)) const rpcprog_t program,
+				__attribute__((unused)) const rpcvers_t version,
+				__attribute__((unused)) const unsigned short protocol,
+				__attribute__((unused)) const struct timeval *timeout)
 {
 	rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
 	return 0;

[PATCH 06/31] New versions of libtool add extra aclocal scripts

[Chuck Lever]

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

 .gitignore |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/.gitignore b/.gitignore
index cfc329a..632609e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -13,6 +13,11 @@ ltmain.sh
 Makefile.in
 missing
 support/include/config.h.in
+aclocal/libtool.m4
+aclocal/ltoptions.m4
+aclocal/ltsugar.m4
+aclocal/ltversion.m4
+aclocal/lt~obsolete.m4
 # files generated by configure
 confdefs.h
 config.status

[PATCH 07/31] support: Use HAVE_LIBTIRPC to switch in bindresvport_sa(3t)

[Chuck Lever]

commit 383a026d99624c88c0e802103ef4c4865db8eb71, which fixed an
earlier commit, is still not quite correct.

bindresvport_sa(3t) is available whenever libtirpc is linked.
There's no need to use IPV6_SUPPORTED here.

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

 support/nfs/rpc_socket.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/support/nfs/rpc_socket.c b/support/nfs/rpc_socket.c
index cebf83d..2f76542 100644
--- a/support/nfs/rpc_socket.c
+++ b/support/nfs/rpc_socket.c
@@ -132,7 +132,7 @@ static int nfs_bind(const int sock, const sa_family_t family)
 	return -1;
 }
 
-#ifdef IPV6_SUPPORTED
+#ifdef HAVE_LIBTIRPC
 
 /*
  * Bind a socket using an unused privileged source port.
@@ -162,7 +162,7 @@ static int nfs_bindresvport(const int sock, const sa_family_t family)
 	return -1;
 }
 
-#else	/* !IPV6_SUPPORTED */
+#else	/* !HAVE_LIBTIRPC */
 
 /*
  * Bind a socket using an unused privileged source port.
@@ -180,7 +180,7 @@ static int nfs_bindresvport(const int sock, const sa_family_t family)
 	return bindresvport(sock, NULL);
 }
 
-#endif	/* !IPV6_SUPPORTED */
+#endif	/* !HAVE_LIBTIRPC */
 
 /*
  * Perform a non-blocking connect on the socket fd.

[PATCH 08/31] support: Don't return RPC_UNKNOWNHOST from rpc_socket.c

[Chuck Lever]

RPC_UNKNOWNHOST means a hostname isn't known -- basically it's
EAI_NONAME from getaddrinfo(3).  Since the functions in rpc_socket.c
don't take a hostname argument, RPC_UNKNOWNHOST is not an appropriate
return code from these functions.

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

 support/nfs/rpc_socket.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/support/nfs/rpc_socket.c b/support/nfs/rpc_socket.c
index 2f76542..a2255c3 100644
--- a/support/nfs/rpc_socket.c
+++ b/support/nfs/rpc_socket.c
@@ -458,7 +458,7 @@ CLIENT *nfs_get_rpcclient(const struct sockaddr *sap,
 		}
 		break;
 	default:
-		rpc_createerr.cf_stat = RPC_UNKNOWNHOST;
+		rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
 		return NULL;
 	}
 
@@ -521,7 +521,7 @@ CLIENT *nfs_get_priv_rpcclient(const struct sockaddr *sap,
 		}
 		break;
 	default:
-		rpc_createerr.cf_stat = RPC_UNKNOWNHOST;
+		rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
 		return NULL;
 	}
 

[PATCH 09/31] support: Set proper retransmit timeout for datagram transports

[Chuck Lever]

Instead of setting the total timeout and the retransmit timeout to the
same value for datagram transports, use a 1 second retransmit timeout,
so we actually get a retransmit or two before failing.

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

 support/nfs/rpc_socket.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/support/nfs/rpc_socket.c b/support/nfs/rpc_socket.c
index a2255c3..ac4e6d8 100644
--- a/support/nfs/rpc_socket.c
+++ b/support/nfs/rpc_socket.c
@@ -326,7 +326,9 @@ static CLIENT *nfs_get_udpclient(const struct sockaddr *sap,
 					version, *timeout, &sock);
 #endif	/* !HAVE_LIBTIRPC */
 	if (client != NULL) {
-		CLNT_CONTROL(client, CLSET_RETRY_TIMEOUT, (char *)timeout);
+		struct timeval retry_timeout = { 1, 0 };
+		CLNT_CONTROL(client, CLSET_RETRY_TIMEOUT,
+						(char *)&retry_timeout);
 		CLNT_CONTROL(client, CLSET_FD_CLOSE, NULL);
 	} else
 		(void)close(sock);

[PATCH 10/31] getport: RPC_PROGNOTREGISTERED is a permanent error

[Chuck Lever]

rpcbind returns RPC_PROGNOTREGISTERED if it knows for certain that an
RPC program is not supported for a given transport.  This is a
permanent and authoritative error, so the library's rpcbind query API
should never retry the query -- it will only get the same answer.

A similar change was submitted for libtirpc.  Unlike rpcb_getaddr(3t),
mount.nfs's rpcbind client only retries once (with RPCB3PROC_GETADDR),
but an extra TCP socket in this case would leave another port in
TIME_WAIT.  It's infrequent enough, but might as well get rid of it.

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

 support/nfs/getport.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/support/nfs/getport.c b/support/nfs/getport.c
index bc0f121..71f02f3 100644
--- a/support/nfs/getport.c
+++ b/support/nfs/getport.c
@@ -469,7 +469,7 @@ static unsigned short nfs_gp_rpcb_getaddr(CLIENT *client,
 		case RPC_SUCCESS:
 			if ((uaddr == NULL) || (uaddr[0] == '\0')) {
 				rpc_createerr.cf_stat = RPC_PROGNOTREGISTERED;
-				continue;
+				return 0;
 			}
 
 			port = nfs_universal2port(uaddr);

[PATCH 11/31] getport: Clear shared error fields before trying rpcbind queries

[Chuck Lever]

Some RPC errors set fields in rpc_createerr.cf_error in addition
to cf_stat.  Be sure to clear _all_ error fields in rpc_createerr
each time through the rpcbind API.

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

 support/include/nfsrpc.h |    9 +++++++++
 support/nfs/getport.c    |   18 ++++++++++++++++++
 support/nfs/rpc_socket.c |    4 ++++
 utils/mount/network.c    |    3 +--
 4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/support/include/nfsrpc.h b/support/include/nfsrpc.h
index da3b4df..b3cdb8c 100644
--- a/support/include/nfsrpc.h
+++ b/support/include/nfsrpc.h
@@ -49,6 +49,15 @@
 #define NSMPROG		((rpcprog_t)100024)
 #endif
 
+/**
+ * nfs_clear_rpc_createerr - zap all error reporting fields
+ *
+ */
+static inline void nfs_clear_rpc_createerr(void)
+{
+	memset(&rpc_createerr, 0, sizeof(rpc_createerr));
+}
+
 /*
  * Look up an RPC program name in /etc/rpc
  */
diff --git a/support/nfs/getport.c b/support/nfs/getport.c
index 71f02f3..aa9c154 100644
--- a/support/nfs/getport.c
+++ b/support/nfs/getport.c
@@ -627,6 +627,8 @@ int nfs_rpc_ping(const struct sockaddr *sap, const socklen_t salen,
 	if (timeout != NULL)
 		tout = *timeout;
 
+	nfs_clear_rpc_createerr();
+
 	memcpy(saddr, sap, (size_t)salen);
 	client = nfs_get_rpcclient(saddr, salen, protocol,
 						program, version, &tout);
@@ -690,6 +692,8 @@ unsigned short nfs_getport(const struct sockaddr *sap,
 	unsigned short port = 0;
 	CLIENT *client;
 
+	nfs_clear_rpc_createerr();
+
 	memcpy(saddr, sap, (size_t)salen);
 	client = nfs_gp_get_rpcbclient(saddr, salen, protocol,
 						default_rpcb_version, &timeout);
@@ -729,6 +733,8 @@ int nfs_getport_ping(struct sockaddr *sap, const socklen_t salen,
 	CLIENT *client;
 	int result = 0;
 	
+	nfs_clear_rpc_createerr();
+
 	client = nfs_gp_get_rpcbclient(sap, salen, protocol,
 						default_rpcb_version, &timeout);
 	if (client != NULL) {
@@ -745,6 +751,8 @@ int nfs_getport_ping(struct sockaddr *sap, const socklen_t salen,
 		memcpy(saddr, sap, (size_t)salen);
 		nfs_gp_set_port(saddr, htons(port));
 
+		nfs_clear_rpc_createerr();
+
 		client = nfs_get_rpcclient(saddr, salen, protocol,
 						program, version, &timeout);
 		if (client != NULL) {
@@ -803,6 +811,8 @@ unsigned short nfs_getlocalport(const rpcprot_t program,
 	CLIENT *client;
 	struct timeval timeout = { -1, 0 };
 
+	nfs_clear_rpc_createerr();
+
 	client = nfs_gp_get_rpcbclient(sap, salen, 0, RPCBVERS_4, &timeout);
 	if (client != NULL) {
 		struct rpcb parms;
@@ -817,6 +827,8 @@ unsigned short nfs_getlocalport(const rpcprot_t program,
 #endif	/* NFS_GP_LOCAL */
 
 	if (port == 0) {
+		nfs_clear_rpc_createerr();
+
 		if (nfs_gp_loopback_address(lb_addr, &lb_len)) {
 			port = nfs_getport(lb_addr, lb_len,
 						program, version, protocol);
@@ -878,6 +890,8 @@ unsigned short nfs_rpcb_getaddr(const struct sockaddr *sap,
 	if (timeout != NULL)
 		tout = *timeout;
 
+	nfs_clear_rpc_createerr();
+
 	memcpy(saddr, sap, (size_t)salen);
 	client = nfs_gp_get_rpcbclient(saddr, salen, transport,
 							RPCBVERS_4, &tout);
@@ -904,6 +918,8 @@ unsigned short nfs_rpcb_getaddr(__attribute__((unused)) const struct sockaddr *s
 				__attribute__((unused)) const unsigned short protocol,
 				__attribute__((unused)) const struct timeval *timeout)
 {
+	nfs_clear_rpc_createerr();
+
 	rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
 	return 0;
 }
@@ -959,6 +975,8 @@ unsigned long nfs_pmap_getport(const struct sockaddr_in *sin,
 	if (timeout != NULL)
 		tout = *timeout;
 
+	nfs_clear_rpc_createerr();
+
 	memcpy(saddr, sin, sizeof(address));
 	client = nfs_gp_get_rpcbclient(saddr, (socklen_t)sizeof(*sin),
 					transport, PMAPVERS, &tout);
diff --git a/support/nfs/rpc_socket.c b/support/nfs/rpc_socket.c
index ac4e6d8..a080487 100644
--- a/support/nfs/rpc_socket.c
+++ b/support/nfs/rpc_socket.c
@@ -443,6 +443,8 @@ CLIENT *nfs_get_rpcclient(const struct sockaddr *sap,
 	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
 	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
 
+	nfs_clear_rpc_createerr();
+
 	switch (sap->sa_family) {
 	case AF_LOCAL:
 		return nfs_get_localclient(sap, salen, program,
@@ -506,6 +508,8 @@ CLIENT *nfs_get_priv_rpcclient(const struct sockaddr *sap,
 	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
 	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
 
+	nfs_clear_rpc_createerr();
+
 	switch (sap->sa_family) {
 	case AF_LOCAL:
 		return nfs_get_localclient(sap, salen, program,
diff --git a/utils/mount/network.c b/utils/mount/network.c
index 04a62ab..3080378 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -538,7 +538,6 @@ static int nfs_probe_port(const struct sockaddr *sap, const socklen_t salen,
 	memcpy(saddr, sap, salen);
 	p_prot = prot ? &prot : protos;
 	p_vers = vers ? &vers : versions;
-	rpc_createerr.cf_stat = 0;
 
 	for (;;) {
 		p_port = nfs_getport(saddr, salen, prog, *p_vers, *p_prot);
@@ -581,7 +580,7 @@ out_ok:
 		pmap->pm_prot = *p_prot;
 	if (!port)
 		pmap->pm_port = p_port;
-	rpc_createerr.cf_stat = 0;
+	nfs_clear_rpc_createerr();
 	return 1;
 }
 

[PATCH 12/31] mount.nfs: Add more debugging output around nfs_getport()

[Chuck Lever]

So we can see how rpcbind queries are failing during mount processing,
add some debugging messages (enabled with "mount.nfs -v") around the
nfs_getport() calls.

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

 utils/mount/network.c |   28 ++++++++++++++++++++++++----
 1 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 3080378..9661995 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -509,6 +509,21 @@ static void nfs_pp_debug(const struct sockaddr *sap, const socklen_t salen,
 			port);
 }
 
+static void nfs_pp_debug2(const char *str)
+{
+	if (!verbose)
+		return;
+
+	if (rpc_createerr.cf_error.re_status == RPC_CANTRECV ||
+	    rpc_createerr.cf_error.re_status == RPC_CANTSEND)
+		nfs_error(_("%s: portmap query %s%s - %s"),
+				progname, str, clnt_spcreateerror(""),
+				strerror(rpc_createerr.cf_error.re_errno));
+	else
+		nfs_error(_("%s: portmap query %s%s"),
+				progname, str, clnt_spcreateerror(""));
+}
+
 /*
  * Use the portmapper to discover whether or not the service we want is
  * available. The lists 'versions' and 'protos' define ordered sequences
@@ -540,6 +555,9 @@ static int nfs_probe_port(const struct sockaddr *sap, const socklen_t salen,
 	p_vers = vers ? &vers : versions;
 
 	for (;;) {
+		if (verbose)
+			printf(_("%s: prog %lu, trying vers=%lu, prot=%u\n"),
+				progname, prog, *p_vers, *p_prot);
 		p_port = nfs_getport(saddr, salen, prog, *p_vers, *p_prot);
 		if (p_port) {
 			if (!port || port == p_port) {
@@ -555,22 +573,24 @@ static int nfs_probe_port(const struct sockaddr *sap, const socklen_t salen,
 		    rpc_createerr.cf_stat != RPC_TIMEDOUT &&
 		    rpc_createerr.cf_stat != RPC_CANTRECV &&
 		    rpc_createerr.cf_stat != RPC_PROGVERSMISMATCH)
-			goto out_bad;
+			break;
 
 		if (!prot) {
-			if (*++p_prot)
+			if (*++p_prot) {
+				nfs_pp_debug2("retrying");
 				continue;
+			}
 			p_prot = protos;
 		}
 		if (rpc_createerr.cf_stat == RPC_TIMEDOUT ||
 		    rpc_createerr.cf_stat == RPC_CANTRECV)
-			goto out_bad;
+			break;
 
 		if (vers || !*++p_vers)
 			break;
 	}
 
-out_bad:
+	nfs_pp_debug2("failed");
 	return 0;
 
 out_ok:

[PATCH 13/31] getport: Restore historical TCP connect timeout error code

[Chuck Lever]

The latest versions of mount.nfs appear not to fall back to
UDP if TCP isn't available on the server.

Our new nfs_getport() implementation is missing a bit of logic
from the original mount getport() implementation.  Without it,
nfs_probe_port() sees a TCP connect timeout as a permanent error,
so it fails immediately instead of attempting to try again with
UDP.

Similar changes for our new ping API (see the old clnt_ping()
function, which is still in utils/mount/network.c).

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

 support/nfs/getport.c |   26 ++++++++++++++++++++++++--
 1 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/support/nfs/getport.c b/support/nfs/getport.c
index aa9c154..e39f809 100644
--- a/support/nfs/getport.c
+++ b/support/nfs/getport.c
@@ -66,6 +66,23 @@ static const rpcvers_t default_rpcb_version = PMAPVERS;
 #endif	/* !HAVE_LIBTIRPC */
 
 /*
+ * Historical: Map TCP connect timeouts to timeout
+ * error code used by UDP.
+ */
+static void
+nfs_gp_map_tcp_errorcodes(const unsigned short protocol)
+{
+	if (protocol != IPPROTO_TCP)
+		return;
+
+	switch (rpc_createerr.cf_error.re_errno) {
+	case ETIMEDOUT:
+		rpc_createerr.cf_stat = RPC_TIMEDOUT;
+		break;
+	}
+}
+
+/*
  * There's no easy way to tell how the local system's networking
  * and rpcbind is configured (ie. whether we want to use IPv6 or
  * IPv4 loopback to contact RPC services on the local host).  We
@@ -188,10 +205,13 @@ static CLIENT *nfs_gp_get_rpcbclient(struct sockaddr *sap,
 		NULL,
 	};
 	rpcprog_t rpcb_prog = nfs_getrpcbyname(RPCBPROG, rpcb_pgmtbl);
+	CLIENT *clnt;
 
 	nfs_gp_set_port(sap, nfs_gp_get_rpcb_port(transport));
-	return nfs_get_rpcclient(sap, salen, transport, rpcb_prog,
-					version, timeout);
+	clnt = nfs_get_rpcclient(sap, salen, transport, rpcb_prog,
+							version, timeout);
+	nfs_gp_map_tcp_errorcodes(transport);
+	return clnt;
 }
 
 /*
@@ -634,6 +654,7 @@ int nfs_rpc_ping(const struct sockaddr *sap, const socklen_t salen,
 						program, version, &tout);
 	if (client != NULL) {
 		result = nfs_gp_ping(client, tout);
+		nfs_gp_map_tcp_errorcodes(protocol);
 		CLNT_DESTROY(client);
 	}
 
@@ -757,6 +778,7 @@ int nfs_getport_ping(struct sockaddr *sap, const socklen_t salen,
 						program, version, &timeout);
 		if (client != NULL) {
 			result = nfs_gp_ping(client, timeout);
+			nfs_gp_map_tcp_errorcodes(protocol);
 			CLNT_DESTROY(client);
 		}
 	}

[PATCH 14/31] getport: Convert TCP connection refused to RPC_CANTRECV

[Chuck Lever]

In a similar vein to the timeout logic we just restored, a refused
TCP connection should be mapped to an equivalent UDP error code:
RPC_CANTRECV.

This is new behavior for TCP connections; the legacy mount command
appears to have simply failed immediately if a TCP connection was
refused during an rpcbind query.

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

 support/nfs/getport.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/support/nfs/getport.c b/support/nfs/getport.c
index e39f809..f5ba4ef 100644
--- a/support/nfs/getport.c
+++ b/support/nfs/getport.c
@@ -79,6 +79,9 @@ nfs_gp_map_tcp_errorcodes(const unsigned short protocol)
 	case ETIMEDOUT:
 		rpc_createerr.cf_stat = RPC_TIMEDOUT;
 		break;
+	case ECONNREFUSED:
+		rpc_createerr.cf_stat = RPC_CANTRECV;
+		break;
 	}
 }
 

[PATCH 15/31] mount.nfs: If port= specifies an unregistered port, retry, then fail

[Chuck Lever]

Suppose a port= option is specified on the mount command line, but not
enough other mount options are specified to avoid an rpcbind query to
discover the NFS service.

If the NFS service isn't registered on [100003, 3, "tcp", port] (even
if the server is listening on the specified port), the legacy mount.nfs
command fails immediately with:

 mount.nfs: mount to NFS server 'server' failed: RPC Error: Success

What's more, this mount request should succeeded if an NFS service is
registered on the specified port for another version and/or protocol.

So instead, let's retry the rpcbind query with the other versions and
transport protocols to be absolutely sure that port won't work with
either version or transport.  Then, if all fails, report:

 mount.nfs: mount to NFS server 'server' failed:
				RPC Error: Program not registered

This change also affects text-based mounts that require negotiation
by the mount.nfs command.

Note that if the mount options specify all four pmap parameters for
NFS, the rpcbind query for the NFS service is skipped entirely.  The
mount command then hangs and times out later if NFS service is not
listening on the requested tuple.  This is unchanged from previous
behavior.

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

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

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 9661995..73d64a2 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -567,7 +567,8 @@ static int nfs_probe_port(const struct sockaddr *sap, const socklen_t salen,
 				if (nfs_rpc_ping(saddr, salen, prog,
 							*p_vers, *p_prot, NULL))
 					goto out_ok;
-			}
+			} else
+				rpc_createerr.cf_stat = RPC_PROGNOTREGISTERED;
 		}
 		if (rpc_createerr.cf_stat != RPC_PROGNOTREGISTERED &&
 		    rpc_createerr.cf_stat != RPC_TIMEDOUT &&

[PATCH 16/31] mount.nfs: force rpcbind queries if options aren't specified

[Chuck Lever]

nfs_options2pmap() fills in default values if the passed-in mount
options don't specify values.  This short-circuits the version, port,
and transport negotiation logic in nfs_probe_bothports().

Instead, nfs_options2pmap() should plant zeros in these pmap fields
to force nfs_probe_bothports() and nfs_advise_mount() to discover, via
rpcbind queries, what the server supports.

This fixes some scenarios where umount.nfs fails to connect to servers
that don't have all rpcbind ports open, in addition to fixing other
corner cases during mount.nfs version/protocol negotiation.

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

 utils/mount/network.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 73d64a2..1e05263 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -1230,13 +1230,16 @@ static rpcvers_t nfs_nfs_version(struct mount_options *options)
  * Returns the NFS transport protocol specified by the given mount options
  *
  * Returns the IPPROTO_ value specified by the given mount options, or
- * IPPROTO_UDP if all fails.
+ * zero if all fails.  The protocol value will be filled in by an
+ * rpcbind query in this case.
  */
 static unsigned short nfs_nfs_protocol(struct mount_options *options)
 {
 	char *option;
 
 	switch (po_rightmost(options, nfs_transport_opttbl)) {
+	case 0:	/* udp */
+		return IPPROTO_UDP;
 	case 1: /* tcp */
 		return IPPROTO_TCP;
 	case 2: /* proto */
@@ -1249,7 +1252,7 @@ static unsigned short nfs_nfs_protocol(struct mount_options *options)
 		}
 	}
 
-	return IPPROTO_UDP;
+	return 0;
 }
 
 /*
@@ -1290,7 +1293,8 @@ static rpcprog_t nfs_mount_program(struct mount_options *options)
 
 /*
  * Returns the RPC version number specified by the given mount options,
- * or the version "3" if all fails.
+ * or zero if all fails.  The version value will be filled in by an
+ * rpcbind query in this case.
  */
 static rpcvers_t nfs_mount_version(struct mount_options *options)
 {
@@ -1300,15 +1304,15 @@ static rpcvers_t nfs_mount_version(struct mount_options *options)
 		if (tmp >= 1 && tmp <= 4)
 			return tmp;
 
-	return nfsvers_to_mnt(nfs_nfs_version(options));
+	return 0;
 }
 
 /*
  * Returns the transport protocol to use for the mount service
  *
  * Returns the IPPROTO_ value specified by the mountproto option, or
- * if that doesn't exist, the IPPROTO_ value specified for NFS
- * itself.
+ * copies the NFS protocol setting.  If the protocol value is zero,
+ * the value will be filled in by an rpcbind query.
  */
 static unsigned short nfs_mount_protocol(struct mount_options *options)
 {
@@ -1330,7 +1334,7 @@ static unsigned short nfs_mount_protocol(struct mount_options *options)
  * mount options, or zero if all fails.  Zero results in a portmap
  * query to discover the server's mountd service port.
  *
- * port=0 will guarantee an rpcbind request precedes the mount
+ * mountport=0 will guarantee an rpcbind request precedes the mount
  * RPC so the client can determine the server's port number.
  */
 static unsigned short nfs_mount_port(struct mount_options *options)

[PATCH 17/31] mount.nfs: make nfs_options2pmap return errors

[Chuck Lever]

Up until now, nfs_options2pmap() has been passed mount options that
have already gone through the kernel's parser successfully.  So, it
never had to check for invalid mount option values.

However, we are about to pass it options that come right from the
user.  So nfs_options2pmap() will now need to report an error and
fail if it encounters a bogus value for any of the options it cares
about.

 =====

Note that nfs_options2pmap() will allow a bogus value for an option
if the same option is specified farther to the right with a useable
value.

For example, if a user specifies "proto=foo,...,tcp" then
nfs_options2pmap() uses "tcp" and ignores "proto=foo".

However, if the options are specified in the other order:
"tcp,...,proto=foo" then nfs_options2pmap() will fail.  This is a simple
and unambiguous extension of the "rightmost wins" rule.

Since mount.nfs strips out these options out and replaces them with
the rpcbind-negotiated options before invoking mount(2), the kernel
should never receive bogus values for these options from mount.nfs in
such cases.

This is probably slightly more flexible behavior than the legacy
mount implementation, but should be harmless.  All mount options
unrelated to pmap are ignored by nfs_options2pmap().

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

 utils/mount/network.c   |  317 ++++++++++++++++++++++++++++++++---------------
 utils/mount/network.h   |    2 
 utils/mount/nfsumount.c |    5 +
 utils/mount/stropts.c   |    5 +
 4 files changed, 227 insertions(+), 102 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 1e05263..5b6f4c2 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -1176,175 +1176,282 @@ out_failed:
 	if (verbose)
 		nfs_error(_("%s: failed to construct callback address"));
 	return 0;
-
 }
 
 /*
- * "nfsprog" is only supported by the legacy mount command.  The
+ * "nfsprog" is supported only by the legacy mount command.  The
  * kernel mount client does not support this option.
  *
- * Returns the value set by the nfsprog= option, the value of
- * the RPC NFS program specified in /etc/rpc, or a baked-in
- * default program number, if all fails.
+ * Returns TRUE if @program contains a valid value for this option,
+ * or FALSE if the option was specified with an invalid value.
  */
-static rpcprog_t nfs_nfs_program(struct mount_options *options)
+static int
+nfs_nfs_program(struct mount_options *options, unsigned long *program)
 {
 	long tmp;
 
-	if (po_get_numeric(options, "nfsprog", &tmp) == PO_FOUND)
-		if (tmp >= 0)
-			return tmp;
-	return nfs_getrpcbyname(NFSPROG, nfs_nfs_pgmtbl);
-}
+	switch (po_get_numeric(options, "nfsprog", &tmp)) {
+	case PO_NOT_FOUND:
+		break;
+	case PO_FOUND:
+		if (tmp > 0) {
+			*program = tmp;
+			return 1;
+		}
+	case PO_BAD_VALUE:
+		return 0;
+	}
 
+	/*
+	 * NFS RPC program wasn't specified.  The RPC program
+	 * cannot be determined via an rpcbind query.
+	 */
+	*program = nfs_getrpcbyname(NFSPROG, nfs_nfs_pgmtbl);
+	return 1;
+}
 
 /*
- * Returns the RPC version number specified by the given mount
- * options for the NFS service, or zero if all fails.
+ * Returns TRUE if @version contains a valid value for this option,
+ * or FALSE if the option was specified with an invalid value.
  */
-static rpcvers_t nfs_nfs_version(struct mount_options *options)
+static int
+nfs_nfs_version(struct mount_options *options, unsigned long *version)
 {
 	long tmp;
 
 	switch (po_rightmost(options, nfs_version_opttbl)) {
 	case 0:	/* v2 */
-		return 2;
+		*version = 2;
+		return 1;
 	case 1: /* v3 */
-		return 3;
+		*version = 3;
+		return 1;
 	case 2:	/* vers */
-		if (po_get_numeric(options, "vers", &tmp) == PO_FOUND)
-			if (tmp >= 2 && tmp <= 3)
-				return tmp;
-		break;
+		switch (po_get_numeric(options, "vers", &tmp)) {
+		case PO_FOUND:
+			if (tmp >= 2 && tmp <= 3) {
+				*version = tmp;
+				return 1;
+			}
+			return 0;
+		case PO_NOT_FOUND:
+			nfs_error(_("%s: option parsing error\n"),
+					progname);
+		case PO_BAD_VALUE:
+			return 0;
+		}
 	case 3: /* nfsvers */
-		if (po_get_numeric(options, "nfsvers", &tmp) == PO_FOUND)
-			if (tmp >= 2 && tmp <= 3)
-				return tmp;
-		break;
+		switch (po_get_numeric(options, "nfsvers", &tmp)) {
+		case PO_FOUND:
+			if (tmp >= 2 && tmp <= 3) {
+				*version = tmp;
+				return 1;
+			}
+			return 0;
+		case PO_NOT_FOUND:
+			nfs_error(_("%s: option parsing error\n"),
+					progname);
+		case PO_BAD_VALUE:
+			return 0;
+		}
 	}
 
-	return 0;
+	/*
+	 * NFS version wasn't specified.  The pmap version value
+	 * will be filled in later by an rpcbind query in this case.
+	 */
+	*version = 0;
+	return 1;
 }
 
 /*
- * Returns the NFS transport protocol specified by the given mount options
- *
- * Returns the IPPROTO_ value specified by the given mount options, or
- * zero if all fails.  The protocol value will be filled in by an
- * rpcbind query in this case.
+ * Returns TRUE if @protocol contains a valid value for this option,
+ * or FALSE if the option was specified with an invalid value.
  */
-static unsigned short nfs_nfs_protocol(struct mount_options *options)
+static int
+nfs_nfs_protocol(struct mount_options *options, unsigned long *protocol)
 {
 	char *option;
 
 	switch (po_rightmost(options, nfs_transport_opttbl)) {
 	case 0:	/* udp */
-		return IPPROTO_UDP;
+		*protocol = IPPROTO_UDP;
+		return 1;
 	case 1: /* tcp */
-		return IPPROTO_TCP;
+		*protocol = IPPROTO_TCP;
+		return 1;
 	case 2: /* proto */
 		option = po_get(options, "proto");
 		if (option) {
-			if (strcmp(option, "tcp") == 0)
-				return IPPROTO_TCP;
-			if (strcmp(option, "udp") == 0)
-				return IPPROTO_UDP;
+			if (strcmp(option, "tcp") == 0) {
+				*protocol = IPPROTO_TCP;
+				return 1;
+			}
+			if (strcmp(option, "udp") == 0) {
+				*protocol = IPPROTO_UDP;
+				return 1;
+			}
+			return 0;
 		}
 	}
 
-	return 0;
+	/*
+	 * NFS transport protocol wasn't specified.  The pmap
+	 * protocol value will be filled in later by an rpcbind
+	 * query in this case.
+	 */
+	*protocol = 0;
+	return 1;
 }
 
 /*
- * Returns the NFS server's port number specified by the given
- * mount options, or zero if all fails.  Zero results in a portmap
- * query to discover the server's mountd service port.
- *
- * port=0 will guarantee an rpcbind request precedes the first
- * NFS RPC so the client can determine the server's port number.
+ * Returns TRUE if @port contains a valid value for this option,
+ * or FALSE if the option was specified with an invalid value.
  */
-static unsigned short nfs_nfs_port(struct mount_options *options)
+static int
+nfs_nfs_port(struct mount_options *options, unsigned long *port)
 {
 	long tmp;
 
-	if (po_get_numeric(options, "port", &tmp) == PO_FOUND)
-		if (tmp >= 0 && tmp <= 65535)
-			return tmp;
-	return 0;
+	switch (po_get_numeric(options, "port", &tmp)) {
+	case PO_NOT_FOUND:
+		break;
+	case PO_FOUND:
+		if (tmp >= 1 && tmp <= 65535) {
+			*port = tmp;
+			return 1;
+		}
+	case PO_BAD_VALUE:
+		return 0;
+	}
+
+	/*
+	 * NFS service port wasn't specified.  The pmap port value
+	 * will be filled in later by an rpcbind query in this case.
+	 */
+	*port = 0;
+	return 1;
 }
 
 /*
- * "mountprog" is only supported by the legacy mount command.  The
+ * "mountprog" is supported only by the legacy mount command.  The
  * kernel mount client does not support this option.
  *
- * Returns the value set by the mountprog= option, the value of
- * the RPC mount program specified in /etc/rpc, or a baked-in
- * default program number, if all fails.
+ * Returns TRUE if @program contains a valid value for this option,
+ * or FALSE if the option was specified with an invalid value.
  */
-static rpcprog_t nfs_mount_program(struct mount_options *options)
+static int
+nfs_mount_program(struct mount_options *options, unsigned long *program)
 {
 	long tmp;
 
-	if (po_get_numeric(options, "mountprog", &tmp) == PO_FOUND)
-		if (tmp >= 0)
-			return tmp;
-	return nfs_getrpcbyname(MOUNTPROG, nfs_mnt_pgmtbl);
+	switch (po_get_numeric(options, "mountprog", &tmp)) {
+	case PO_NOT_FOUND:
+		break;
+	case PO_FOUND:
+		if (tmp > 0) {
+			*program = tmp;
+			return 1;
+		}
+	case PO_BAD_VALUE:
+		return 0;
+	}
+
+	/*
+	 * MNT RPC program wasn't specified.  The RPC program
+	 * cannot be determined via an rpcbind query.
+	 */
+	*program = nfs_getrpcbyname(MOUNTPROG, nfs_mnt_pgmtbl);
+	return 1;
 }
 
 /*
- * Returns the RPC version number specified by the given mount options,
- * or zero if all fails.  The version value will be filled in by an
- * rpcbind query in this case.
+ * Returns TRUE if @version contains a valid value for this option,
+ * or FALSE if the option was specified with an invalid value.
  */
-static rpcvers_t nfs_mount_version(struct mount_options *options)
+static int
+nfs_mount_version(struct mount_options *options, unsigned long *version)
 {
 	long tmp;
 
-	if (po_get_numeric(options, "mountvers", &tmp) == PO_FOUND)
-		if (tmp >= 1 && tmp <= 4)
-			return tmp;
+	switch (po_get_numeric(options, "mountvers", &tmp)) {
+	case PO_NOT_FOUND:
+		break;
+	case PO_FOUND:
+		if (tmp >= 1 && tmp <= 4) {
+			*version = tmp;
+			return 1;
+		}
+	case PO_BAD_VALUE:
+		return 0;
+	}
 
-	return 0;
+	/*
+	 * MNT version wasn't specified.  The pmap version value
+	 * will be filled in later by an rpcbind query in this case.
+	 */
+	*version = 0;
+	return 1;
 }
 
 /*
- * Returns the transport protocol to use for the mount service
- *
- * Returns the IPPROTO_ value specified by the mountproto option, or
- * copies the NFS protocol setting.  If the protocol value is zero,
- * the value will be filled in by an rpcbind query.
+ * Returns TRUE if @protocol contains a valid value for this option,
+ * or FALSE if the option was specified with an invalid value.
  */
-static unsigned short nfs_mount_protocol(struct mount_options *options)
+static int
+nfs_mount_protocol(struct mount_options *options, unsigned long *protocol)
 {
 	char *option;
 
 	option = po_get(options, "mountproto");
 	if (option) {
-		if (strcmp(option, "tcp") == 0)
-			return IPPROTO_TCP;
-		if (strcmp(option, "udp") == 0)
-			return IPPROTO_UDP;
+		if (strcmp(option, "tcp") == 0) {
+			*protocol = IPPROTO_TCP;
+			return 1;
+		}
+		if (strcmp(option, "udp") == 0) {
+			*protocol = IPPROTO_UDP;
+			return 1;
+		}
+		return 0;
 	}
 
-	return nfs_nfs_protocol(options);
+	/*
+	 * MNT transport protocol wasn't specified.  If the NFS
+	 * transport protocol was specified, use that; otherwise
+	 * set @protocol to zero.  The pmap protocol value will
+	 * be filled in later by an rpcbind query in this case.
+	 */
+	return nfs_nfs_protocol(options, protocol);
 }
 
 /*
- * Returns the mountd server's port number specified by the given
- * mount options, or zero if all fails.  Zero results in a portmap
- * query to discover the server's mountd service port.
- *
- * mountport=0 will guarantee an rpcbind request precedes the mount
- * RPC so the client can determine the server's port number.
+ * Returns TRUE if @port contains a valid value for this option,
+ * or FALSE if the option was specified with an invalid value.
  */
-static unsigned short nfs_mount_port(struct mount_options *options)
+static int
+nfs_mount_port(struct mount_options *options, unsigned long *port)
 {
 	long tmp;
 
-	if (po_get_numeric(options, "mountport", &tmp) == PO_FOUND)
-		if (tmp >= 0 && tmp <= 65535)
-			return tmp;
-	return 0;
+	switch (po_get_numeric(options, "mountport", &tmp)) {
+	case PO_NOT_FOUND:
+		break;
+	case PO_FOUND:
+		if (tmp >= 1 && tmp <= 65535) {
+			*port = tmp;
+			return 1;
+		}
+	case PO_BAD_VALUE:
+		return 0;
+	}
+
+	/*
+	 * MNT service port wasn't specified.  The pmap port value
+	 * will be filled in later by an rpcbind query in this case.
+	 */
+	*port = 0;
+	return 1;
 }
 
 /**
@@ -1353,17 +1460,29 @@ static unsigned short nfs_mount_port(struct mount_options *options)
  * @nfs_pmap: OUT: pointer to pmap arguments for NFS server
  * @mnt_pmap: OUT: pointer to pmap arguments for mountd server
  *
+ * Returns TRUE if the pmap options specified in @options have valid
+ * values; otherwise FALSE is returned.
  */
-void nfs_options2pmap(struct mount_options *options,
-		      struct pmap *nfs_pmap, struct pmap *mnt_pmap)
+int nfs_options2pmap(struct mount_options *options,
+		     struct pmap *nfs_pmap, struct pmap *mnt_pmap)
 {
-	nfs_pmap->pm_prog = nfs_nfs_program(options);
-	nfs_pmap->pm_vers = nfs_nfs_version(options);
-	nfs_pmap->pm_prot = nfs_nfs_protocol(options);
-	nfs_pmap->pm_port = nfs_nfs_port(options);
-
-	mnt_pmap->pm_prog = nfs_mount_program(options);
-	mnt_pmap->pm_vers = nfs_mount_version(options);
-	mnt_pmap->pm_prot = nfs_mount_protocol(options);
-	mnt_pmap->pm_port = nfs_mount_port(options);
+	if (!nfs_nfs_program(options, &nfs_pmap->pm_prog))
+		return 0;
+	if (!nfs_nfs_version(options, &nfs_pmap->pm_vers))
+		return 0;
+	if (!nfs_nfs_protocol(options, &nfs_pmap->pm_prot))
+		return 0;
+	if (!nfs_nfs_port(options, &nfs_pmap->pm_port))
+		return 0;
+
+	if (!nfs_mount_program(options, &mnt_pmap->pm_prog))
+		return 0;
+	if (!nfs_mount_version(options, &mnt_pmap->pm_vers))
+		return 0;
+	if (!nfs_mount_protocol(options, &mnt_pmap->pm_prot))
+		return 0;
+	if (!nfs_mount_port(options, &mnt_pmap->pm_port))
+		return 0;
+
+	return 1;
 }
diff --git a/utils/mount/network.h b/utils/mount/network.h
index b3f9bd2..ca5fa3b 100644
--- a/utils/mount/network.h
+++ b/utils/mount/network.h
@@ -57,7 +57,7 @@ int clnt_ping(struct sockaddr_in *, const unsigned long,
 
 struct mount_options;
 
-void nfs_options2pmap(struct mount_options *,
+int nfs_options2pmap(struct mount_options *,
 		      struct pmap *, struct pmap *);
 
 int start_statd(void);
diff --git a/utils/mount/nfsumount.c b/utils/mount/nfsumount.c
index 9b48cc9..ba27900 100644
--- a/utils/mount/nfsumount.c
+++ b/utils/mount/nfsumount.c
@@ -174,7 +174,10 @@ static int nfs_umount_do_umnt(struct mount_options *options,
 	socklen_t salen = sizeof(address);
 	struct pmap nfs_pmap, mnt_pmap;
 
-	nfs_options2pmap(options, &nfs_pmap, &mnt_pmap);
+	if (!nfs_options2pmap(options, &nfs_pmap, &mnt_pmap)) {
+		nfs_error(_("%s: bad mount options"), progname);
+		return EX_FAIL;
+	}
 
 	*hostname = nfs_umount_hostname(options, *hostname);
 	if (!*hostname) {
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index ec95b78..30ac5ae 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -450,7 +450,10 @@ static struct mount_options *nfs_rewrite_mount_options(char *str)
 		goto err;
 	}
 
-	nfs_options2pmap(options, &nfs_pmap, &mnt_pmap);
+	if (!nfs_options2pmap(options, &nfs_pmap, &mnt_pmap)) {
+		errno = EINVAL;
+		goto err;
+	}
 
 	/* The kernel NFS client doesn't support changing the RPC program
 	 * number for these services, so reset these fields before probing

[PATCH 18/31] mount.nfs: rearchitect mount version/protocol negotiation logic

[Chuck Lever]

Text-based mounts try a mount operation first with default settings,
then negotiate via rpcbind queries and retry the mount, if the default
settings don't work.  This method introduces long delays in certain
common scenarios, and makes it difficult to tell when it is
appropriate to fail immediately or negotiate and retry.

To address these behavioral regressions, make text-based mounts
operate the same way that legacy mounts work.  Perform rpcbind queries
with short timeouts first, then use the results to determine
transport, version, and port number settings for the mount.

This allows the mount.nfs command to detect server settings, or
whether negotiation is even possible, quickly.  It also makes it
simple to determine when to fail vs. when to retry.

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

 utils/mount/stropts.c |   42 +++++++++++++++++++++++++-----------------
 1 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 30ac5ae..6801569 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -437,6 +437,7 @@ static struct mount_options *nfs_rewrite_mount_options(char *str)
 	struct sockaddr *mnt_saddr = (struct sockaddr *)&mnt_address;
 	socklen_t mnt_salen;
 	struct pmap mnt_pmap;
+	char *option;
 
 	options = po_split(str);
 	if (!options) {
@@ -444,23 +445,39 @@ static struct mount_options *nfs_rewrite_mount_options(char *str)
 		return NULL;
 	}
 
+	/*
+	 * Skip option negotiation for proto=rdma mounts.
+	 */
+	option = po_get(options, "proto");
+	if (option && strcmp(option, "rdma") == 0)
+		goto out;
+
+	/*
+	 * Extract just the options needed to contact server.
+	 * Bail now if any of these have bad values.
+	 */
 	if (!nfs_extract_server_addresses(options, nfs_saddr, &nfs_salen,
 						mnt_saddr, &mnt_salen)) {
 		errno = EINVAL;
 		goto err;
 	}
-
 	if (!nfs_options2pmap(options, &nfs_pmap, &mnt_pmap)) {
 		errno = EINVAL;
 		goto err;
 	}
 
-	/* The kernel NFS client doesn't support changing the RPC program
-	 * number for these services, so reset these fields before probing
-	 * the server's ports.  */
+	/*
+	 * The kernel NFS client doesn't support changing the RPC
+	 * program number for these services, so force the value of
+	 * these fields before probing the server's ports.
+	 */
 	nfs_pmap.pm_prog = NFS_PROGRAM;
 	mnt_pmap.pm_prog = MOUNTPROG;
 
+	/*
+	 * If the server's rpcbind service isn't available, we can't
+	 * negotiate.  Bail now if we can't contact it.
+	 */
 	if (!nfs_probe_bothports(mnt_saddr, mnt_salen, &mnt_pmap,
 				 nfs_saddr, nfs_salen, &nfs_pmap)) {
 		errno = ESPIPE;
@@ -472,6 +489,7 @@ static struct mount_options *nfs_rewrite_mount_options(char *str)
 		goto err;
 	}
 
+out:
 	errno = 0;
 	return options;
 
@@ -532,6 +550,9 @@ static int nfs_retry_nfs23mount(struct nfsmount_info *mi)
 		printf(_("%s: text-based options (retry): '%s'\n"),
 			progname, retry_str);
 
+	if (mi->fake)
+		return 1;
+
 	if (!nfs_sys_mount(mi, "nfs", retry_str)) {
 		po_destroy(retry_options);
 		free(retry_str);
@@ -570,19 +591,6 @@ static int nfs_try_nfs23mount(struct nfsmount_info *mi)
 		printf(_("%s: text-based options: '%s'\n"),
 			progname, *extra_opts);
 
-	if (mi->fake)
-		return 1;
-
-	if (nfs_sys_mount(mi, "nfs", *extra_opts))
-		return 1;
-
-	/*
-	 * The kernel returns EOPNOTSUPP if the RPC bind failed,
-	 * and EPROTONOSUPPORT if the version isn't supported.
-	 */
-	if (errno != EOPNOTSUPP && errno != EPROTONOSUPPORT)
-		return 0;
-
 	return nfs_retry_nfs23mount(mi);
 }
 

[PATCH 19/31] mount.nfs: Clean up nfs_is_permanent_error()

[Chuck Lever]

Clean up: Move nfs_is_permanent_error() closer to the functions that
call it, and update a documenting comment to reflect recent
restructuring in this area.

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

 utils/mount/stropts.c |   45 +++++++++++++++++++++++++--------------------
 1 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 6801569..6560f1c 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -287,26 +287,6 @@ static int nfs_validate_options(struct nfsmount_info *mi)
 }
 
 /*
- * Distinguish between permanent and temporary errors.
- *
- * Returns 0 if the passed-in error is temporary, thus the
- * mount system call should be retried; returns one if the
- * passed-in error is permanent, thus the mount system call
- * should not be retried.
- */
-static int nfs_is_permanent_error(int error)
-{
-	switch (error) {
-	case ESTALE:
-	case ETIMEDOUT:
-	case ECONNREFUSED:
-		return 0;	/* temporary */
-	default:
-		return 1;	/* permanent */
-	}
-}
-
-/*
  * Get NFS/mnt server addresses from mount options
  *
  * Returns 1 and fills in @nfs_saddr, @nfs_salen, @mnt_saddr, and @mnt_salen
@@ -634,6 +614,31 @@ static int nfs_try_mount(struct nfsmount_info *mi)
 }
 
 /*
+ * Distinguish between permanent and temporary errors.
+ *
+ * Basically, we retry if communication with the server has
+ * failed so far, but fail immediately if there is a local
+ * error (like a bad mount option).
+ *
+ * ESTALE is also a temporary error because some servers
+ * return ESTALE when a share is temporarily offline.
+ *
+ * Returns 1 if we should fail immediately, or 0 if we
+ * should retry.
+ */
+static int nfs_is_permanent_error(int error)
+{
+	switch (error) {
+	case ESTALE:
+	case ETIMEDOUT:
+	case ECONNREFUSED:
+		return 0;	/* temporary */
+	default:
+		return 1;	/* permanent */
+	}
+}
+
+/*
  * Handle "foreground" NFS mounts.
  *
  * Retry the mount request for as long as the 'retry=' option says.

[PATCH 20/31] mount.nfs: Clean up after restructuring version/protocol negotiation

[Chuck Lever]

Fix up comments and function names to reflect the new version/protocol
negotiation scheme.  We can now remove a bunch of mount processing
that is specific to v2/v3, removing about 100 lines of logic from
stropts.c.

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

 utils/mount/stropts.c |  157 ++++++++-----------------------------------------
 1 files changed, 25 insertions(+), 132 deletions(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 6560f1c..e94265a 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -400,15 +400,15 @@ static int nfs_construct_new_options(struct mount_options *options,
  *
  * To handle version and transport protocol fallback properly, we
  * need to parse some of the mount options in order to set up a
- * portmap probe.  Mount options that nfs_rewrite_mount_options()
+ * portmap probe.  Mount options that nfs_rewrite_pmap_mount_options()
  * doesn't recognize are left alone.
  *
- * Returns a new group of mount options if successful; otherwise
- * NULL is returned if some failure occurred.
+ * Returns TRUE if rewriting was successful; otherwise
+ * FALSE is returned if some failure occurred.
  */
-static struct mount_options *nfs_rewrite_mount_options(char *str)
+static int
+nfs_rewrite_pmap_mount_options(struct mount_options *options)
 {
-	struct mount_options *options;
 	struct sockaddr_storage nfs_address;
 	struct sockaddr *nfs_saddr = (struct sockaddr *)&nfs_address;
 	socklen_t nfs_salen;
@@ -419,12 +419,6 @@ static struct mount_options *nfs_rewrite_mount_options(char *str)
 	struct pmap mnt_pmap;
 	char *option;
 
-	options = po_split(str);
-	if (!options) {
-		errno = EFAULT;
-		return NULL;
-	}
-
 	/*
 	 * Skip option negotiation for proto=rdma mounts.
 	 */
@@ -439,11 +433,11 @@ static struct mount_options *nfs_rewrite_mount_options(char *str)
 	if (!nfs_extract_server_addresses(options, nfs_saddr, &nfs_salen,
 						mnt_saddr, &mnt_salen)) {
 		errno = EINVAL;
-		goto err;
+		return 0;
 	}
 	if (!nfs_options2pmap(options, &nfs_pmap, &mnt_pmap)) {
 		errno = EINVAL;
-		goto err;
+		return 0;
 	}
 
 	/*
@@ -461,156 +455,55 @@ static struct mount_options *nfs_rewrite_mount_options(char *str)
 	if (!nfs_probe_bothports(mnt_saddr, mnt_salen, &mnt_pmap,
 				 nfs_saddr, nfs_salen, &nfs_pmap)) {
 		errno = ESPIPE;
-		goto err;
+		return 0;
 	}
 
 	if (!nfs_construct_new_options(options, &nfs_pmap, &mnt_pmap)) {
 		errno = EINVAL;
-		goto err;
+		return 0;
 	}
 
 out:
 	errno = 0;
-	return options;
-
-err:
-	po_destroy(options);
-	return NULL;
-}
-
-/*
- * Do the mount(2) system call.
- *
- * Returns 1 if successful, otherwise zero.
- * "errno" is set to reflect the individual error.
- */
-static int nfs_sys_mount(const struct nfsmount_info *mi, const char *type,
-			 const char *options)
-{
-	int result;
-
-	result = mount(mi->spec, mi->node, type,
-				mi->flags & ~(MS_USER|MS_USERS), options);
-	if (verbose && result) {
-		int save = errno;
-		nfs_error(_("%s: mount(2): %s"), progname, strerror(save));
-		errno = save;
-	}
-	return !result;
-}
-
-/*
- * Retry an NFS mount that failed because the requested service isn't
- * available on the server.
- *
- * Returns 1 if successful.  Otherwise, returns zero.
- * "errno" is set to reflect the individual error.
- *
- * Side effect: If the retry is successful, both 'options' and
- * 'extra_opts' are updated to reflect the mount options that worked.
- * If the retry fails, 'options' and 'extra_opts' are left unchanged.
- */
-static int nfs_retry_nfs23mount(struct nfsmount_info *mi)
-{
-	struct mount_options *retry_options;
-	char *retry_str = NULL;
-	char **extra_opts = mi->extra_opts;
-
-	retry_options = nfs_rewrite_mount_options(*extra_opts);
-	if (!retry_options)
-		return 0;
-
-	if (po_join(retry_options, &retry_str) == PO_FAILED) {
-		po_destroy(retry_options);
-		errno = EIO;
-		return 0;
-	}
-
-	if (verbose)
-		printf(_("%s: text-based options (retry): '%s'\n"),
-			progname, retry_str);
-
-	if (mi->fake)
-		return 1;
-
-	if (!nfs_sys_mount(mi, "nfs", retry_str)) {
-		po_destroy(retry_options);
-		free(retry_str);
-		return 0;
-	}
-
-	free(*extra_opts);
-	*extra_opts = retry_str;
-	po_replace(mi->options, retry_options);
 	return 1;
 }
 
 /*
- * Attempt an NFSv2/3 mount via a mount(2) system call.  If the kernel
- * claims the requested service isn't supported on the server, probe
- * the server to see what's supported, rewrite the mount options,
- * and retry the request.
+ * Do the mount(2) system call.
  *
- * Returns 1 if successful.  Otherwise, returns zero.
+ * Returns TRUE if successful, otherwise FALSE.
  * "errno" is set to reflect the individual error.
- *
- * Side effect: If the retry is successful, both 'options' and
- * 'extra_opts' are updated to reflect the mount options that worked.
- * If the retry fails, 'options' and 'extra_opts' are left unchanged.
  */
-static int nfs_try_nfs23mount(struct nfsmount_info *mi)
+static int nfs_try_mount(struct nfsmount_info *mi)
 {
 	char **extra_opts = mi->extra_opts;
+	int result;
 
-	if (po_join(mi->options, extra_opts) == PO_FAILED) {
-		errno = EIO;
-		return 0;
+	if (strncmp(mi->type, "nfs4", 4) != 0) {
+		if (!nfs_rewrite_pmap_mount_options(mi->options))
+			return 0;
 	}
 
-	if (verbose)
-		printf(_("%s: text-based options: '%s'\n"),
-			progname, *extra_opts);
-
-	return nfs_retry_nfs23mount(mi);
-}
-
-/*
- * Attempt an NFS v4 mount via a mount(2) system call.
- *
- * Returns 1 if successful.  Otherwise, returns zero.
- * "errno" is set to reflect the individual error.
- */
-static int nfs_try_nfs4mount(struct nfsmount_info *mi)
-{
-	char **extra_opts = mi->extra_opts;
-
 	if (po_join(mi->options, extra_opts) == PO_FAILED) {
 		errno = EIO;
 		return 0;
 	}
 
 	if (verbose)
-		printf(_("%s: text-based options: '%s'\n"),
+		printf(_("%s: trying text-based options '%s'\n"),
 			progname, *extra_opts);
 
 	if (mi->fake)
 		return 1;
 
-	return nfs_sys_mount(mi, "nfs4", *extra_opts);
-}
-
-/*
- * Perform either an NFSv2/3 mount, or an NFSv4 mount system call.
- *
- * Returns 1 if successful.  Otherwise, returns zero.
- * "errno" is set to reflect the individual error.
- */
-static int nfs_try_mount(struct nfsmount_info *mi)
-{
-	if (strncmp(mi->type, "nfs4", 4) == 0)
-		return nfs_try_nfs4mount(mi);
-	else
-		return nfs_try_nfs23mount(mi);
+	result = mount(mi->spec, mi->node, mi->type,
+			mi->flags & ~(MS_USER|MS_USERS), *extra_opts);
+	if (verbose && result) {
+		int save = errno;
+		nfs_error(_("%s: mount(2): %s"), progname, strerror(save));
+		errno = save;
+	}
+	return !result;
 }
 
 /*

[PATCH 21/31] mount.nfs: Don't update extra_opts after text-based negotiation

[Chuck Lever]

The umount.nfs command will negotiate the mount options again, so all
that is needed in /etc/mnttab is the original set of options used for
the mount, plus the additional mandatory options like addr=''.

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

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

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index e94265a..8d168db 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -283,7 +283,16 @@ static int nfs_validate_options(struct nfsmount_info *mi)
 	if (!nfs_append_sloppy_option(mi->options))
 		return 0;
 
-	return nfs_append_addr_option(sap, salen, mi->options);
+	if (!nfs_append_addr_option(sap, salen, mi->options))
+		return 0;
+
+	/*
+	 * Update option string to be recorded in /etc/mnttab
+	 */
+	if (po_join(mi->options, mi->extra_opts) == PO_FAILED)
+		return 0;
+
+	return 1;
 }
 
 /*
@@ -476,7 +485,7 @@ out:
  */
 static int nfs_try_mount(struct nfsmount_info *mi)
 {
-	char **extra_opts = mi->extra_opts;
+	char *options = NULL;
 	int result;
 
 	if (strncmp(mi->type, "nfs4", 4) != 0) {
@@ -484,20 +493,20 @@ static int nfs_try_mount(struct nfsmount_info *mi)
 			return 0;
 	}
 
-	if (po_join(mi->options, extra_opts) == PO_FAILED) {
+	if (po_join(mi->options, &options) == PO_FAILED) {
 		errno = EIO;
 		return 0;
 	}
 
 	if (verbose)
 		printf(_("%s: trying text-based options '%s'\n"),
-			progname, *extra_opts);
+			progname, options);
 
 	if (mi->fake)
 		return 1;
 
 	result = mount(mi->spec, mi->node, mi->type,
-			mi->flags & ~(MS_USER|MS_USERS), *extra_opts);
+			mi->flags & ~(MS_USER|MS_USERS), options);
 	if (verbose && result) {
 		int save = errno;
 		nfs_error(_("%s: mount(2): %s"), progname, strerror(save));

[PATCH 22/31] support: Introduce sockaddr helpers to get and set IP port numbers

[Chuck Lever]

Introduce address family-agnostic functions that get and set IP port
numbers in socket addresses.  We can already replace a few similar
functions in the mount command, and a few more will come up with
statd and sm-notify.

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

 support/include/nfsrpc.h |   10 +++++++
 support/nfs/getport.c    |   24 ++----------------
 support/nfs/rpc_socket.c |   63 +++++++++++++++++++++++++++++++++-------------
 utils/mount/network.c    |   15 -----------
 4 files changed, 58 insertions(+), 54 deletions(-)

diff --git a/support/include/nfsrpc.h b/support/include/nfsrpc.h
index b3cdb8c..dff6af7 100644
--- a/support/include/nfsrpc.h
+++ b/support/include/nfsrpc.h
@@ -59,6 +59,16 @@ static inline void nfs_clear_rpc_createerr(void)
 }
 
 /*
+ * Extract port value from a socket address
+ */
+extern uint16_t		nfs_get_port(const struct sockaddr *);
+
+/*
+ * Set port value in a socket address
+ */
+extern void		nfs_set_port(struct sockaddr *, const uint16_t);
+
+/*
  * Look up an RPC program name in /etc/rpc
  */
 extern rpcprog_t	nfs_getrpcbyname(const rpcprog_t, const char *table[]);
diff --git a/support/nfs/getport.c b/support/nfs/getport.c
index f5ba4ef..4d4b1c4 100644
--- a/support/nfs/getport.c
+++ b/support/nfs/getport.c
@@ -113,24 +113,6 @@ static int nfs_gp_loopback_address(struct sockaddr *sap, socklen_t *salen)
 }
 
 /*
- * Plant port number in @sap.  @port is already in network byte order.
- */
-static void nfs_gp_set_port(struct sockaddr *sap, const in_port_t port)
-{
-	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
-	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
-
-	switch (sap->sa_family) {
-	case AF_INET:
-		sin->sin_port = port;
-		break;
-	case AF_INET6:
-		sin6->sin6_port = port;
-		break;
-	}
-}
-
-/*
  * Look up a network service in /etc/services and return the
  * network-order port number of that service.
  */
@@ -210,7 +192,7 @@ static CLIENT *nfs_gp_get_rpcbclient(struct sockaddr *sap,
 	rpcprog_t rpcb_prog = nfs_getrpcbyname(RPCBPROG, rpcb_pgmtbl);
 	CLIENT *clnt;
 
-	nfs_gp_set_port(sap, nfs_gp_get_rpcb_port(transport));
+	nfs_set_port(sap, ntohs(nfs_gp_get_rpcb_port(transport)));
 	clnt = nfs_get_rpcclient(sap, salen, transport, rpcb_prog,
 							version, timeout);
 	nfs_gp_map_tcp_errorcodes(transport);
@@ -773,7 +755,7 @@ int nfs_getport_ping(struct sockaddr *sap, const socklen_t salen,
 		struct sockaddr *saddr = (struct sockaddr *)&address;
 
 		memcpy(saddr, sap, (size_t)salen);
-		nfs_gp_set_port(saddr, htons(port));
+		nfs_set_port(saddr, port);
 
 		nfs_clear_rpc_createerr();
 
@@ -787,7 +769,7 @@ int nfs_getport_ping(struct sockaddr *sap, const socklen_t salen,
 	}
 
 	if (result)
-		nfs_gp_set_port(sap, htons(port));
+		nfs_set_port(sap, port);
 
 	return result;
 }
diff --git a/support/nfs/rpc_socket.c b/support/nfs/rpc_socket.c
index a080487..9c20f61 100644
--- a/support/nfs/rpc_socket.c
+++ b/support/nfs/rpc_socket.c
@@ -416,6 +416,49 @@ static CLIENT *nfs_get_tcpclient(const struct sockaddr *sap,
 }
 
 /**
+ * nfs_get_port - extract port value from a socket address
+ * @sap: pointer to socket address
+ *
+ * Returns port value in host byte order.
+ */
+uint16_t
+nfs_get_port(const struct sockaddr *sap)
+{
+       const struct sockaddr_in *sin = (const struct sockaddr_in *)sap;
+       const struct sockaddr_in6 *sin6 = (const struct sockaddr_in6 *)sap;
+
+       switch (sap->sa_family) {
+       case AF_INET:
+               return ntohs(sin->sin_port);
+       case AF_INET6:
+               return ntohs(sin6->sin6_port);
+       }
+       return 0;
+}
+
+/**
+ * nfs_set_port - set port value in a socket address
+ * @sap: pointer to socket address
+ * @port: port value to set
+ *
+ */
+void
+nfs_set_port(struct sockaddr *sap, const uint16_t port)
+{
+       struct sockaddr_in *sin = (struct sockaddr_in *)sap;
+       struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
+
+       switch (sap->sa_family) {
+       case AF_INET:
+               sin->sin_port = htons(port);
+               break;
+       case AF_INET6:
+               sin6->sin6_port = htons(port);
+               break;
+       }
+}
+
+/**
  * nfs_get_rpcclient - acquire an RPC client
  * @sap: pointer to socket address of RPC server
  * @salen: length of socket address
@@ -440,9 +483,6 @@ CLIENT *nfs_get_rpcclient(const struct sockaddr *sap,
 			  const rpcvers_t version,
 			  struct timeval *timeout)
 {
-	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
-	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
-
 	nfs_clear_rpc_createerr();
 
 	switch (sap->sa_family) {
@@ -450,13 +490,8 @@ CLIENT *nfs_get_rpcclient(const struct sockaddr *sap,
 		return nfs_get_localclient(sap, salen, program,
 						version, timeout);
 	case AF_INET:
-		if (sin->sin_port == 0) {
-			rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
-			return NULL;
-		}
-		break;
 	case AF_INET6:
-		if (sin6->sin6_port == 0) {
+		if (nfs_get_port(sap) == 0) {
 			rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
 			return NULL;
 		}
@@ -505,9 +540,6 @@ CLIENT *nfs_get_priv_rpcclient(const struct sockaddr *sap,
 			       const rpcvers_t version,
 			       struct timeval *timeout)
 {
-	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
-	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
-
 	nfs_clear_rpc_createerr();
 
 	switch (sap->sa_family) {
@@ -515,13 +547,8 @@ CLIENT *nfs_get_priv_rpcclient(const struct sockaddr *sap,
 		return nfs_get_localclient(sap, salen, program,
 						version, timeout);
 	case AF_INET:
-		if (sin->sin_port == 0) {
-			rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
-			return NULL;
-		}
-		break;
 	case AF_INET6:
-		if (sin6->sin6_port == 0) {
+		if (nfs_get_port(sap) == 0) {
 			rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
 			return NULL;
 		}
diff --git a/utils/mount/network.c b/utils/mount/network.c
index 5b6f4c2..ad6c146 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -170,21 +170,6 @@ static const unsigned long probe_mnt3_first[] = {
 	0,
 };
 
-static void nfs_set_port(struct sockaddr *sap, const unsigned short port)
-{
-	switch (sap->sa_family) {
-	case AF_INET:
-		((struct sockaddr_in *)sap)->sin_port = htons(port);
-		break;
-	case AF_INET6:
-		((struct sockaddr_in6 *)sap)->sin6_port = htons(port);
-		break;
-	default:
-		nfs_error(_("%s: unrecognized address family in %s"),
-			progname, __func__);
-	}
-}
-
 static int nfs_lookup(const char *hostname, const sa_family_t family,
 		      struct sockaddr *sap, socklen_t *salen)
 {

[PATCH 23/31] mount.nfs: Use correct data type in discover_nfs_mount_data_version()

[Chuck Lever]

Address compiler warning:

    mount.c: In function =E2=80=98discover_nfs_mount_data_version=E2=80=
=99:
    mount.c:162: warning: comparison between signed and unsigned
    mount.c:164: warning: comparison between signed and unsigned
    mount.c:166: warning: comparison between signed and unsigned
    mount.c:168: warning: comparison between signed and unsigned
    mount.c:170: warning: comparison between signed and unsigned
    mount.c:178: warning: comparison between signed and unsigned

linux_version_code() and MAKE_VERSION() both return an unsigned int.

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

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

diff --git a/utils/mount/mount.c b/utils/mount/mount.c
index 06e2804..d531d64 100644
--- a/utils/mount/mount.c
+++ b/utils/mount/mount.c
@@ -156,7 +156,7 @@ static void parse_opts(const char *options, int *fl=
ags, char **extra_opts);
  */
 static void discover_nfs_mount_data_version(void)
 {
-	int kernel_version =3D linux_version_code();
+	unsigned int kernel_version =3D linux_version_code();
=20
 	if (kernel_version) {
 		if (kernel_version < MAKE_VERSION(2, 1, 32))

[PATCH 24/31] mount.nfs: Remove unused parameter in try_mount()

[Chuck Lever]

Address compiler warning:

    mount.c: At top level:
    mount.c:420: warning: unused parameter =E2=80=98nomtab=E2=80=99

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

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

diff --git a/utils/mount/mount.c b/utils/mount/mount.c
index d531d64..a668cd9 100644
--- a/utils/mount/mount.c
+++ b/utils/mount/mount.c
@@ -417,7 +417,7 @@ static int chk_mountpoint(char *mount_point)
=20
 static int try_mount(char *spec, char *mount_point, int flags,
 			char *fs_type, char **extra_opts, char *mount_opts,
-			int fake, int nomtab, int bg)
+			int fake, int bg)
 {
 	int ret;
=20
@@ -582,7 +582,7 @@ int main(int argc, char *argv[])
 	}
=20
 	mnt_err =3D try_mount(spec, mount_point, flags, fs_type, &extra_opts,
-				mount_opts, fake, nomtab, FOREGROUND);
+				mount_opts, fake, FOREGROUND);
 	if (mnt_err =3D=3D EX_BG) {
 		printf(_("%s: backgrounding \"%s\"\n"),
 			progname, spec);
@@ -600,7 +600,7 @@ int main(int argc, char *argv[])
=20
 		mnt_err =3D try_mount(spec, mount_point, flags, fs_type,
 					&extra_opts, mount_opts, fake,
-					nomtab, BACKGROUND);
+					BACKGROUND);
 		if (verbose && mnt_err)
 			printf(_("%s: giving up \"%s\"\n"),
 				progname, spec);

[PATCH 25/31] mount.nfs: Fix some nfs_error() nits in network.c

[Chuck Lever]

Fix a couple of nfs_error() call sites in utils/mount/network.c.

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

 utils/mount/network.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index ad6c146..6e0a72a 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -783,8 +783,8 @@ int start_statd(void)
 				execl(START_STATD, START_STATD, NULL);
 				exit(1);
 			case -1: /* error */
-				nfs_error(_("fork failed: %s"),
-							strerror(errno));
+				nfs_error(_("%s: fork failed: %s"),
+						progname, strerror(errno));
 				break;
 			default: /* parent */
 				waitpid(pid, NULL,0);
@@ -1159,7 +1159,8 @@ int nfs_callback_address(const struct sockaddr *sap, const socklen_t salen,
 out_failed:
 	*buflen = 0;
 	if (verbose)
-		nfs_error(_("%s: failed to construct callback address"));
+		nfs_error(_("%s: failed to construct callback address"),
+				progname);
 	return 0;
 }
 

[PATCH 26/31] mount.nfs: Remove unused @salen parameter from nfs_ca_gai()

[Chuck Lever]

Address compiler warning:

    network.c:1124: warning: unused parameter =E2=80=98salen=E2=80=99

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

 utils/mount/network.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 6e0a72a..11c7162 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -1103,7 +1103,7 @@ static int nfs_ca_sockname(const struct sockaddr =
*sap, const socklen_t salen,
  *
  * Returns 1 and fills in @buf if successful; otherwise, zero.
  */
-static int nfs_ca_gai(const struct sockaddr *sap, const socklen_t sale=
n,
+static int nfs_ca_gai(const struct sockaddr *sap,
 		      struct sockaddr *buf, socklen_t *buflen)
 {
 	struct addrinfo *gai_results;
@@ -1144,7 +1144,7 @@ int nfs_callback_address(const struct sockaddr *s=
ap, const socklen_t salen,
 	struct sockaddr_in6 *sin6 =3D (struct sockaddr_in6 *)buf;
=20
 	if (nfs_ca_sockname(sap, salen, buf, buflen) =3D=3D 0)
-		if (nfs_ca_gai(sap, salen, buf, buflen) =3D=3D 0)
+		if (nfs_ca_gai(sap, buf, buflen) =3D=3D 0)
 			goto out_failed;
=20
 	/*

[PATCH 27/31] mount.nfs: remove unused @addrlen argument from nfs_string_to_sockaddr()

[Chuck Lever]

Address compiler warning:

    network.c: In function =E2=80=98nfs_string_to_sockaddr=E2=80=99:
    network.c:272: warning: unused parameter =E2=80=98addrlen=E2=80=99

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

 utils/mount/network.c |    5 ++---
 utils/mount/network.h |    3 +--
 utils/mount/stropts.c |    6 ++----
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 11c7162..4ec7c7b 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -255,7 +255,6 @@ int nfs_gethostbyname(const char *hostname, struct =
sockaddr_in *sin)
 /**
  * nfs_string_to_sockaddr - convert string address to sockaddr
  * @address:	pointer to presentation format address to convert
- * @addrlen:	length of presentation address
  * @sap:	pointer to socket address buffer to fill in
  * @salen:	IN: length of address buffer
  *		OUT: length of converted socket address
@@ -269,8 +268,8 @@ int nfs_gethostbyname(const char *hostname, struct =
sockaddr_in *sin)
  * See RFC 4038 section 5.1 or RFC 3513 section 2.2 for more details
  * on presenting IPv6 addresses as text strings.
  */
-int nfs_string_to_sockaddr(const char *address, const size_t addrlen,
-			   struct sockaddr *sap, socklen_t *salen)
+int nfs_string_to_sockaddr(const char *address, struct sockaddr *sap,
+			   socklen_t *salen)
 {
 	struct addrinfo *gai_results;
 	struct addrinfo gai_hint =3D {
diff --git a/utils/mount/network.h b/utils/mount/network.h
index ca5fa3b..db5134c 100644
--- a/utils/mount/network.h
+++ b/utils/mount/network.h
@@ -45,8 +45,7 @@ int nfs_probe_bothports(const struct sockaddr *, cons=
t socklen_t,
 			const socklen_t, struct pmap *);
 int nfs_gethostbyname(const char *, struct sockaddr_in *);
 int nfs_name_to_address(const char *, struct sockaddr *, socklen_t *);
-int nfs_string_to_sockaddr(const char *, const size_t,
-			   struct sockaddr *, socklen_t *);
+int nfs_string_to_sockaddr(const char *, struct sockaddr *, socklen_t =
*);
 int nfs_present_sockaddr(const struct sockaddr *,
 			 const socklen_t, char *, const size_t);
 int nfs_callback_address(const struct sockaddr *, const socklen_t,
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 8d168db..e277b3d 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -312,16 +312,14 @@ static int nfs_extract_server_addresses(struct mo=
unt_options *options,
 	option =3D po_get(options, "addr");
 	if (option =3D=3D NULL)
 		return 0;
-	if (!nfs_string_to_sockaddr(option, strlen(option),
-						nfs_saddr, nfs_salen))
+	if (!nfs_string_to_sockaddr(option, nfs_saddr, nfs_salen))
 		return 0;
=20
 	option =3D po_get(options, "mountaddr");
 	if (option =3D=3D NULL) {
 		memcpy(mnt_saddr, nfs_saddr, *nfs_salen);
 		*mnt_salen =3D *nfs_salen;
-	} else if (!nfs_string_to_sockaddr(option, strlen(option),
-						mnt_saddr, mnt_salen))
+	} else if (!nfs_string_to_sockaddr(option, mnt_saddr, mnt_salen))
 		return 0;
=20
 	return 1;

[PATCH 28/31] umount.nfs: Use correct data type in nfsumount()

[Chuck Lever]

Address compiler warning:

    nfsumount.c: In function =E2=80=98nfsumount=E2=80=99:
    nfsumount.c:347: warning: comparison between signed and unsigned

The result type of pointer arithmetic and the return type of strlen(3)
are both size_t.

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

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

diff --git a/utils/mount/nfsumount.c b/utils/mount/nfsumount.c
index ba27900..f81db14 100644
--- a/utils/mount/nfsumount.c
+++ b/utils/mount/nfsumount.c
@@ -336,7 +336,7 @@ int nfsumount(int argc, char *argv[])
 			char *opt =3D hasmntopt(&mc->m, "user");
 			struct passwd *pw;
 			char *comma;
-			int len;
+			size_t len;
 			if (!opt)
 				goto only_root;
 			if (opt[4] !=3D '=3D')

[PATCH 29/31] mount.nfs: Fix compiler warning in stropts.c

[Chuck Lever]

Address compiler warning:

    stropts.c: In function =E2=80=98nfs_append_generic_address_option=E2=
=80=99:
    stropts.c:138: warning: comparison between signed and unsigned

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

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

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index e277b3d..9d82bb1 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -130,12 +130,14 @@ static int nfs_append_generic_address_option(cons=
t struct sockaddr *sap,
 {
 	char address[NI_MAXHOST];
 	char new_option[512];
+	int len;
=20
 	if (!nfs_present_sockaddr(sap, salen, address, sizeof(address)))
 		goto out_err;
=20
-	if (snprintf(new_option, sizeof(new_option), "%s=3D%s",
-					keyword, address) >=3D sizeof(new_option))
+	len =3D snprintf(new_option, sizeof(new_option), "%s=3D%s",
+						keyword, address);
+	if (len < 0 || (size_t)len >=3D sizeof(new_option))
 		goto out_err;
=20
 	if (po_append(options, new_option) !=3D PO_SUCCEEDED)

[PATCH 30/31] mount.nfs: Squelch unused parameter warnings on empty functions

[Chuck Lever]

Address compiler warnings:

    fstab.c:288: warning: unused parameter =E2=80=98sig=E2=80=99
   =20
    parse_dev.c:186: warning: unused parameter =E2=80=98dev=E2=80=99
    parse_dev.c:187: warning: unused parameter =E2=80=98hostname=E2=80=99
    parse_dev.c:187: warning: unused parameter =E2=80=98pathname=E2=80=99

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

 utils/mount/fstab.c     |    2 +-
 utils/mount/parse_dev.c |    5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/utils/mount/fstab.c b/utils/mount/fstab.c
index 7668167..2775d0b 100644
--- a/utils/mount/fstab.c
+++ b/utils/mount/fstab.c
@@ -285,7 +285,7 @@ handler (int sig) {
 }
=20
 static void
-setlkw_timeout (int sig) {
+setlkw_timeout (__attribute__((unused)) int sig) {
      /* nothing, fcntl will fail anyway */
 }
=20
diff --git a/utils/mount/parse_dev.c b/utils/mount/parse_dev.c
index c0a8e18..c8a58b1 100644
--- a/utils/mount/parse_dev.c
+++ b/utils/mount/parse_dev.c
@@ -183,8 +183,9 @@ static int nfs_parse_square_bracket(const char *dev=
,
  * with the mount request and failing with a cryptic error message
  * later.
  */
-static int nfs_parse_nfs_url(const char *dev,
-			     char **hostname, char **pathname)
+static int nfs_parse_nfs_url(__attribute__((unused)) const char *dev,
+			     __attribute__((unused)) char **hostname,
+			     __attribute__((unused)) char **pathname)
 {
 	nfs_error(_("%s: NFS URLs are not supported"), progname);
 	return 0;

[PATCH 31/31] mount.nfs: Squelch compiler warnings in nfs_strerror()

[Chuck Lever]

Address compiler warnings:

    error.c: In function =E2=80=98nfs_strerror=E2=80=99:
    error.c:341: warning: comparison between signed and unsigned
    error.c:342: warning: comparison between signed and unsigned

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

 utils/mount/error.c |   12 ++++++------
 utils/mount/error.h |    2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/utils/mount/error.c b/utils/mount/error.c
index 5c9d3f2..4cc9e09 100644
--- a/utils/mount/error.c
+++ b/utils/mount/error.c
@@ -300,6 +300,8 @@ void umount_error(int err, const char *dev)
 #define EDQUOT	ENOSPC
 #endif
=20
+#define ARRAY_SIZE(x)		(sizeof(x) / sizeof((x)[0]))=20
+
 static struct {
 	enum nfsstat stat;
 	int errnum;
@@ -329,19 +331,17 @@ static struct {
 #endif
 	/* Throw in some NFSv3 values for even more fun (HP returns these) */
 	{ 71,			EREMOTE		},
-
-	{ -1,			EIO		}
 };
=20
-char *nfs_strerror(int stat)
+char *nfs_strerror(unsigned int stat)
 {
-	int i;
+	unsigned int i;
 	static char buf[256];
=20
-	for (i =3D 0; nfs_errtbl[i].stat !=3D -1; i++) {
+	for (i =3D 0; i < ARRAY_SIZE(nfs_errtbl); i++) {
 		if (nfs_errtbl[i].stat =3D=3D stat)
 			return strerror(nfs_errtbl[i].errnum);
 	}
-	sprintf(buf, _("unknown nfs status return value: %d"), stat);
+	sprintf(buf, _("unknown nfs status return value: %u"), stat);
 	return buf;
 }
diff --git a/utils/mount/error.h b/utils/mount/error.h
index 7126de5..42b28cf 100644
--- a/utils/mount/error.h
+++ b/utils/mount/error.h
@@ -24,7 +24,7 @@
 #ifndef _NFS_UTILS_MOUNT_ERROR_H
 #define _NFS_UTILS_MOUNT_ERROR_H
=20
-char *nfs_strerror(int);
+char *nfs_strerror(unsigned int);
=20
 void mount_error(const char *, const char *, int);
 void rpc_mount_errors(char *, int, int);

Re: [PATCH 00/31] mount.nfs patches for next nfs-utils release

[Steve Dickson]

On 06/29/2009 01:34 PM, Chuck Lever wrote:
> Hi Steve-
> 
> This series contains some patches I've had for a little while, and a
> bunch of new fixes I made last week to address RH bugzillas 486266 and
> 505535.
> 
> In the short term, these patches should address most of the recent
> complaints about the new mount.nfs.  Text-based mounts should behave
> a lot more like legacy mounts -- fewer hangs and better negotiation
> semantics.
> 
> Longer term, we are planning to sink the mount version/transport
> negotiation into the kernel.
> 
> ---
> 
> Chuck Lever (31):
>       mount.nfs: Squelch compiler warnings in nfs_strerror()
>       mount.nfs: Squelch unused parameter warnings on empty functions
>       mount.nfs: Fix compiler warning in stropts.c
>       umount.nfs: Use correct data type in nfsumount()
>       mount.nfs: remove unused @addrlen argument from nfs_string_to_sockaddr()
>       mount.nfs: Remove unused @salen parameter from nfs_ca_gai()
>       mount.nfs: Fix some nfs_error() nits in network.c
>       mount.nfs: Remove unused parameter in try_mount()
>       mount.nfs: Use correct data type in discover_nfs_mount_data_version()
>       support: Introduce sockaddr helpers to get and set IP port numbers
>       mount.nfs: Don't update extra_opts after text-based negotiation
>       mount.nfs: Clean up after restructuring version/protocol negotiation
>       mount.nfs: Clean up nfs_is_permanent_error()
>       mount.nfs: rearchitect mount version/protocol negotiation logic
>       mount.nfs: make nfs_options2pmap return errors
>       mount.nfs: force rpcbind queries if options aren't specified
>       mount.nfs: If port= specifies an unregistered port, retry, then fail
>       getport: Convert TCP connection refused to RPC_CANTRECV
>       getport: Restore historical TCP connect timeout error code
>       mount.nfs: Add more debugging output around nfs_getport()
>       getport: Clear shared error fields before trying rpcbind queries
>       getport: RPC_PROGNOTREGISTERED is a permanent error
>       support: Set proper retransmit timeout for datagram transports
>       support: Don't return RPC_UNKNOWNHOST from rpc_socket.c
>       support: Use HAVE_LIBTIRPC to switch in bindresvport_sa(3t)
>       New versions of libtool add extra aclocal scripts
>       getport: Remove unneeded @salen arguments
>       getport: replace getnameinfo(NI_NUMERICHOST) with inet_ntop(3)
>       getport: Remove AI_ADDRCONFIG from nfs_gp_loopback_address()
>       getport: RPCB_GETADDR's r_addr should contain rpcbind port, not zero
>       getport: RPCB_GETADDR r_owner should be an empty string
> 
Committed... a long with a couple more patches that cleaned up some 
error cases and the uses of uninitialised data... 

steved.