diff mbox series

mount.nfs: improve EPROTO error message for RDMA mounts

Message ID 20240919151015.536917-1-dan.aloni@vastdata.com (mailing list archive)
State New
Headers show
Series mount.nfs: improve EPROTO error message for RDMA mounts | expand

Commit Message

Dan Aloni Sept. 19, 2024, 3:10 p.m. UTC
When mounting NFS shares using RDMA, users may encounter this rather
unclear error message:

    mount.nfs: Protocol error

Often there are either no RDMA interfaces existing, or that routing is
being done via other interfaces. This patch enhances the `mount_error`
function to provide a more informative message in such cases.

Signed-off-by: Dan Aloni <dan.aloni@vastdata.com>
---
 utils/mount/error.c     | 12 +++++++++++-
 utils/mount/error.h     |  4 +++-
 utils/mount/network.c   |  2 +-
 utils/mount/network.h   |  2 ++
 utils/mount/nfs4mount.c |  2 +-
 utils/mount/nfsmount.c  |  2 +-
 utils/mount/stropts.c   |  8 ++++----
 utils/mount/utils.c     |  6 +++---
 8 files changed, 26 insertions(+), 12 deletions(-)

Comments

Steve Dickson Sept. 20, 2024, 5:18 p.m. UTC | #1
On 9/19/24 11:10 AM, Dan Aloni wrote:
> When mounting NFS shares using RDMA, users may encounter this rather
> unclear error message:
> 
>      mount.nfs: Protocol error
> 
> Often there are either no RDMA interfaces existing, or that routing is
> being done via other interfaces. This patch enhances the `mount_error`
> function to provide a more informative message in such cases.
> 
> Signed-off-by: Dan Aloni <dan.aloni@vastdata.com>
Committed... (tag: nfs-utils-2-7-2-rc1)

steved.

> ---
>   utils/mount/error.c     | 12 +++++++++++-
>   utils/mount/error.h     |  4 +++-
>   utils/mount/network.c   |  2 +-
>   utils/mount/network.h   |  2 ++
>   utils/mount/nfs4mount.c |  2 +-
>   utils/mount/nfsmount.c  |  2 +-
>   utils/mount/stropts.c   |  8 ++++----
>   utils/mount/utils.c     |  6 +++---
>   8 files changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/utils/mount/error.c b/utils/mount/error.c
> index 9ddbcc096f72..d6cbdea1933b 100644
> --- a/utils/mount/error.c
> +++ b/utils/mount/error.c
> @@ -40,6 +40,7 @@
>   #include "nls.h"
>   #include "mount.h"
>   #include "error.h"
> +#include "network.h"
>   
>   #ifdef HAVE_RPCSVC_NFS_PROT_H
>   #include <rpcsvc/nfs_prot.h>
> @@ -199,7 +200,8 @@ void sys_mount_errors(char *server, int error, int will_retry, int bg)
>    * @error: errno value to report
>    *
>    */
> -void mount_error(const char *spec, const char *mount_point, int error)
> +void mount_error(const char *spec, const char *mount_point, int error,
> +		 struct mount_options *options)
>   {
>   	switch(error) {
>   	case EACCES:
> @@ -250,6 +252,14 @@ void mount_error(const char *spec, const char *mount_point, int error)
>   	case EALREADY:
>   		/* Error message has already been provided */
>   		break;
> +	case EPROTO:
> +		if (options && po_rightmost(options, nfs_transport_opttbl) == 2)
> +			nfs_error(_("%s: %s: is routing being done via an interface supporting RDMA?"),
> +				  progname, strerror(error));
> +		else
> +			nfs_error(_("%s: %s"),
> +				  progname, strerror(error));
> +		break;
>   	default:
>   		nfs_error(_("%s: %s for %s on %s"),
>   			  progname, strerror(error), spec, mount_point);
> diff --git a/utils/mount/error.h b/utils/mount/error.h
> index ef80fd079b48..f9f282233563 100644
> --- a/utils/mount/error.h
> +++ b/utils/mount/error.h
> @@ -24,9 +24,11 @@
>   #ifndef _NFS_UTILS_MOUNT_ERROR_H
>   #define _NFS_UTILS_MOUNT_ERROR_H
>   
> +#include "parse_opt.h"
> +
>   char *nfs_strerror(unsigned int);
>   
> -void mount_error(const char *, const char *, int);
> +void mount_error(const char *, const char *, int, struct mount_options *);
>   void rpc_mount_errors(char *, int, int);
>   void sys_mount_errors(char *, int, int, int);
>   
> diff --git a/utils/mount/network.c b/utils/mount/network.c
> index 01ead49f0008..64293f6f8d51 100644
> --- a/utils/mount/network.c
> +++ b/utils/mount/network.c
> @@ -88,7 +88,7 @@ static const char *nfs_nfs_pgmtbl[] = {
>   	NULL,
>   };
>   
> -static const char *nfs_transport_opttbl[] = {
> +const char *nfs_transport_opttbl[] = {
>   	"udp",
>   	"tcp",
>   	"rdma",
> diff --git a/utils/mount/network.h b/utils/mount/network.h
> index 0fc98acd4bcb..26f4eec775df 100644
> --- a/utils/mount/network.h
> +++ b/utils/mount/network.h
> @@ -93,4 +93,6 @@ void mnt_closeclnt(CLIENT *, int);
>   int nfs_umount_do_umnt(struct mount_options *options,
>   		       char **hostname, char **dirname);
>   
> +extern const char *nfs_transport_opttbl[];
> +
>   #endif	/* _NFS_UTILS_MOUNT_NETWORK_H */
> diff --git a/utils/mount/nfs4mount.c b/utils/mount/nfs4mount.c
> index 3e4f1e255ca7..0fe142a7843e 100644
> --- a/utils/mount/nfs4mount.c
> +++ b/utils/mount/nfs4mount.c
> @@ -469,7 +469,7 @@ int nfs4mount(const char *spec, const char *node, int flags,
>   	if (!fake) {
>   		if (mount(spec, node, "nfs4",
>   				flags & ~(MS_USER|MS_USERS), &data)) {
> -			mount_error(spec, node, errno);
> +			mount_error(spec, node, errno, NULL);
>   			goto fail;
>   		}
>   	}
> diff --git a/utils/mount/nfsmount.c b/utils/mount/nfsmount.c
> index 3d95da9456de..a1c92fe83fa0 100644
> --- a/utils/mount/nfsmount.c
> +++ b/utils/mount/nfsmount.c
> @@ -858,7 +858,7 @@ noauth_flavors:
>   	if (!fake) {
>   		if (mount(spec, node, "nfs",
>   				flags & ~(MS_USER|MS_USERS), &data)) {
> -			mount_error(spec, node, errno);
> +			mount_error(spec, node, errno, NULL);
>   			goto fail;
>   		}
>   	}
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index a92c420011a4..2d5fa1f2e86e 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -1143,7 +1143,7 @@ static int nfsmount_fg(struct nfsmount_info *mi)
>   		}
>   	}
>   
> -	mount_error(mi->spec, mi->node, errno);
> +	mount_error(mi->spec, mi->node, errno, mi->options);
>   	return EX_FAIL;
>   }
>   
> @@ -1160,7 +1160,7 @@ static int nfsmount_parent(struct nfsmount_info *mi)
>   		return EX_SUCCESS;
>   
>   	if (nfs_is_permanent_error(errno)) {
> -		mount_error(mi->spec, mi->node, errno);
> +		mount_error(mi->spec, mi->node, errno, mi->options);
>   		return EX_FAIL;
>   	}
>   
> @@ -1237,7 +1237,7 @@ static int nfs_remount(struct nfsmount_info *mi)
>   {
>   	if (nfs_sys_mount(mi, mi->options))
>   		return EX_SUCCESS;
> -	mount_error(mi->spec, mi->node, errno);
> +	mount_error(mi->spec, mi->node, errno, mi->options);
>   	return EX_FAIL;
>   }
>   
> @@ -1261,7 +1261,7 @@ static int nfsmount_start(struct nfsmount_info *mi)
>   	 * NFS v2 has been deprecated
>   	 */
>   	if (mi->version.major == 2) {
> -		mount_error(mi->spec, mi->node, EOPNOTSUPP);
> +		mount_error(mi->spec, mi->node, EOPNOTSUPP, mi->options);
>   		return EX_FAIL;
>   	}
>   
> diff --git a/utils/mount/utils.c b/utils/mount/utils.c
> index 865a4a05f3fc..b7562a474f88 100644
> --- a/utils/mount/utils.c
> +++ b/utils/mount/utils.c
> @@ -123,15 +123,15 @@ int chk_mountpoint(const char *mount_point)
>   	struct stat sb;
>   
>   	if (stat(mount_point, &sb) < 0){
> -		mount_error(NULL, mount_point, errno);
> +		mount_error(NULL, mount_point, errno, NULL);
>   		return 1;
>   	}
>   	if (S_ISDIR(sb.st_mode) == 0){
> -		mount_error(NULL, mount_point, ENOTDIR);
> +		mount_error(NULL, mount_point, ENOTDIR, NULL);
>   		return 1;
>   	}
>   	if (getuid() != 0 && geteuid() != 0 && access(mount_point, X_OK) < 0) {
> -		mount_error(NULL, mount_point, errno);
> +		mount_error(NULL, mount_point, errno, NULL);
>   		return 1;
>   	}
>
diff mbox series

Patch

diff --git a/utils/mount/error.c b/utils/mount/error.c
index 9ddbcc096f72..d6cbdea1933b 100644
--- a/utils/mount/error.c
+++ b/utils/mount/error.c
@@ -40,6 +40,7 @@ 
 #include "nls.h"
 #include "mount.h"
 #include "error.h"
+#include "network.h"
 
 #ifdef HAVE_RPCSVC_NFS_PROT_H
 #include <rpcsvc/nfs_prot.h>
@@ -199,7 +200,8 @@  void sys_mount_errors(char *server, int error, int will_retry, int bg)
  * @error: errno value to report
  *
  */
-void mount_error(const char *spec, const char *mount_point, int error)
+void mount_error(const char *spec, const char *mount_point, int error,
+		 struct mount_options *options)
 {
 	switch(error) {
 	case EACCES:
@@ -250,6 +252,14 @@  void mount_error(const char *spec, const char *mount_point, int error)
 	case EALREADY:
 		/* Error message has already been provided */
 		break;
+	case EPROTO:
+		if (options && po_rightmost(options, nfs_transport_opttbl) == 2)
+			nfs_error(_("%s: %s: is routing being done via an interface supporting RDMA?"),
+				  progname, strerror(error));
+		else
+			nfs_error(_("%s: %s"),
+				  progname, strerror(error));
+		break;
 	default:
 		nfs_error(_("%s: %s for %s on %s"),
 			  progname, strerror(error), spec, mount_point);
diff --git a/utils/mount/error.h b/utils/mount/error.h
index ef80fd079b48..f9f282233563 100644
--- a/utils/mount/error.h
+++ b/utils/mount/error.h
@@ -24,9 +24,11 @@ 
 #ifndef _NFS_UTILS_MOUNT_ERROR_H
 #define _NFS_UTILS_MOUNT_ERROR_H
 
+#include "parse_opt.h"
+
 char *nfs_strerror(unsigned int);
 
-void mount_error(const char *, const char *, int);
+void mount_error(const char *, const char *, int, struct mount_options *);
 void rpc_mount_errors(char *, int, int);
 void sys_mount_errors(char *, int, int, int);
 
diff --git a/utils/mount/network.c b/utils/mount/network.c
index 01ead49f0008..64293f6f8d51 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -88,7 +88,7 @@  static const char *nfs_nfs_pgmtbl[] = {
 	NULL,
 };
 
-static const char *nfs_transport_opttbl[] = {
+const char *nfs_transport_opttbl[] = {
 	"udp",
 	"tcp",
 	"rdma",
diff --git a/utils/mount/network.h b/utils/mount/network.h
index 0fc98acd4bcb..26f4eec775df 100644
--- a/utils/mount/network.h
+++ b/utils/mount/network.h
@@ -93,4 +93,6 @@  void mnt_closeclnt(CLIENT *, int);
 int nfs_umount_do_umnt(struct mount_options *options,
 		       char **hostname, char **dirname);
 
+extern const char *nfs_transport_opttbl[];
+
 #endif	/* _NFS_UTILS_MOUNT_NETWORK_H */
diff --git a/utils/mount/nfs4mount.c b/utils/mount/nfs4mount.c
index 3e4f1e255ca7..0fe142a7843e 100644
--- a/utils/mount/nfs4mount.c
+++ b/utils/mount/nfs4mount.c
@@ -469,7 +469,7 @@  int nfs4mount(const char *spec, const char *node, int flags,
 	if (!fake) {
 		if (mount(spec, node, "nfs4",
 				flags & ~(MS_USER|MS_USERS), &data)) {
-			mount_error(spec, node, errno);
+			mount_error(spec, node, errno, NULL);
 			goto fail;
 		}
 	}
diff --git a/utils/mount/nfsmount.c b/utils/mount/nfsmount.c
index 3d95da9456de..a1c92fe83fa0 100644
--- a/utils/mount/nfsmount.c
+++ b/utils/mount/nfsmount.c
@@ -858,7 +858,7 @@  noauth_flavors:
 	if (!fake) {
 		if (mount(spec, node, "nfs",
 				flags & ~(MS_USER|MS_USERS), &data)) {
-			mount_error(spec, node, errno);
+			mount_error(spec, node, errno, NULL);
 			goto fail;
 		}
 	}
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index a92c420011a4..2d5fa1f2e86e 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -1143,7 +1143,7 @@  static int nfsmount_fg(struct nfsmount_info *mi)
 		}
 	}
 
-	mount_error(mi->spec, mi->node, errno);
+	mount_error(mi->spec, mi->node, errno, mi->options);
 	return EX_FAIL;
 }
 
@@ -1160,7 +1160,7 @@  static int nfsmount_parent(struct nfsmount_info *mi)
 		return EX_SUCCESS;
 
 	if (nfs_is_permanent_error(errno)) {
-		mount_error(mi->spec, mi->node, errno);
+		mount_error(mi->spec, mi->node, errno, mi->options);
 		return EX_FAIL;
 	}
 
@@ -1237,7 +1237,7 @@  static int nfs_remount(struct nfsmount_info *mi)
 {
 	if (nfs_sys_mount(mi, mi->options))
 		return EX_SUCCESS;
-	mount_error(mi->spec, mi->node, errno);
+	mount_error(mi->spec, mi->node, errno, mi->options);
 	return EX_FAIL;
 }
 
@@ -1261,7 +1261,7 @@  static int nfsmount_start(struct nfsmount_info *mi)
 	 * NFS v2 has been deprecated
 	 */
 	if (mi->version.major == 2) {
-		mount_error(mi->spec, mi->node, EOPNOTSUPP);
+		mount_error(mi->spec, mi->node, EOPNOTSUPP, mi->options);
 		return EX_FAIL;
 	}
 
diff --git a/utils/mount/utils.c b/utils/mount/utils.c
index 865a4a05f3fc..b7562a474f88 100644
--- a/utils/mount/utils.c
+++ b/utils/mount/utils.c
@@ -123,15 +123,15 @@  int chk_mountpoint(const char *mount_point)
 	struct stat sb;
 
 	if (stat(mount_point, &sb) < 0){
-		mount_error(NULL, mount_point, errno);
+		mount_error(NULL, mount_point, errno, NULL);
 		return 1;
 	}
 	if (S_ISDIR(sb.st_mode) == 0){
-		mount_error(NULL, mount_point, ENOTDIR);
+		mount_error(NULL, mount_point, ENOTDIR, NULL);
 		return 1;
 	}
 	if (getuid() != 0 && geteuid() != 0 && access(mount_point, X_OK) < 0) {
-		mount_error(NULL, mount_point, errno);
+		mount_error(NULL, mount_point, errno, NULL);
 		return 1;
 	}