mount.nfs: Use default minor version when -t nfs4 is specified

PATCH Superseded Steve Dickson 2 participants 3 messages Jun 8, 2017 – Jun 9, 2017

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Steve Dickson

author

2017-06-08 18:33 UTC #0 lore raw

PATCH v2 1/1 mount.nfs: Use default minor version when -t nfs4 is specified

When the nfs4 filesystem specified, the default major and minor versions should be used.

Signed-off-by Steve Dickson <[email protected]>

---
 utils/mount/stropts.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

utils/mount/stropts.c +12 -4

index c0266e5..81fb945 100644
@@ -73,6 +73,13 @@
 #define NFS_DEF_BG_TIMEOUT_MINUTES	(10000u)
 #endif
 
+#ifndef NFS_DEFAULT_MAJOR
+#define NFS_DEFAULT_MAJOR	4
+#endif
+#ifndef NFS_DEFAULT_MINOR
+#define NFS_DEFAULT_MINOR	2
+#endif
+
 extern int nfs_mount_data_version;
 extern char *progname;
 extern int verbose;

@@ -124,8 +131,8 @@ static void nfs_default_version(struct nfsmount_info *mi)
 	}
 
 #endif /* MOUNT_CONFIG */
-	mi->version.major = 4;
-	mi->version.minor = 2;
+	mi->version.major = NFS_DEFAULT_MAJOR;
+	mi->version.minor = NFS_DEFAULT_MINOR;
 }
 
 /*

@@ -316,8 +323,9 @@ static int nfs_set_version(struct nfsmount_info *mi)
 		return 0;
 
 	if (strncmp(mi->type, "nfs4", 4) == 0) {
-		mi->version.major = 4;
-		mi->version.v_mode = V_GENERAL;
+		/* Set to default values */
+		mi->version.major = NFS_DEFAULT_MAJOR;
+		mi->version.minor = NFS_DEFAULT_MINOR;
 	}
 	/*
 	 * Before 2.6.32, the kernel NFS client didn't

Signature · 1 line

2.9.4

NeilBrown

2017-06-09 06:20 UTC #1 lore raw

Quoted 4 lines · Steve Dickson

When the nfs4 filesystem specified, the default major and minor versions should be used.
Signed-off-by Steve Dickson <[email protected]>

Hi Steve,
I think this is definitely heading in the right direction. I particularly like that you've #defined for the default major/minor.

However as I reflected on it, I realised that "-o v4" needs to set the same version as "-t nfs4", and even with this patch it doesn't.

I think we need to focus on nfs_default_version() and make sure it does the right thing.

If v.mode == V_DEFAULT, we need to copy both 'major' and 'minor' from config_default_vers if they are set. If config_default_vers.v_mode == V_GENERAL, .minor won't be set so in that case we need to use NFS_DEFAULT_MINOR;

If v.mode == V_GENERAL, then only copy the minor if it is set and the major is correct. So we need to check for V_SPECIFIC, and other wise use the default minor.

If v.mode == V_SPECIFIC, nfs_default_version isn't called, so it doesn't matter.

I think this gets it right...

Thoughts?

Thanks,
NeilBrown

utils/mount/stropts.c +17 -5

index c0266e51ad1c..7dfdd9e62115 100644
@@ -73,6 +73,13 @@
 #define NFS_DEF_BG_TIMEOUT_MINUTES	(10000u)
 #endif
 
+#ifndef NFS_DEFAULT_MAJOR
+#define NFS_DEFAULT_MAJOR	4
+#endif
+#ifndef NFS_DEFAULT_MINOR
+#define NFS_DEFAULT_MINOR	2
+#endif
+
 extern int nfs_mount_data_version;
 extern char *progname;
 extern int verbose;

@@ -110,22 +117,27 @@ static void nfs_default_version(struct nfsmount_info *mi)
 	}
 
 	if (mi->version.v_mode == V_DEFAULT &&
-		config_default_vers.v_mode != V_DEFAULT) {
+	    config_default_vers.v_mode != V_DEFAULT) {
 		mi->version.major = config_default_vers.major;
-		mi->version.minor = config_default_vers.minor;
+		if (config_default_vers.v_mode == V_SPECIFIC)
+			mi->version.minor = config_default_vers.minor;
+		else
+			mi->version.minor = NFS_DEFAULT_MINOR;
 		return;
 	}
 
 	if (mi->version.v_mode == V_GENERAL) {
-		if (config_default_vers.v_mode != V_DEFAULT &&
+		if (config_default_vers.v_mode == V_SPECIFIC &&
 		    mi->version.major == config_default_vers.major)
 			mi->version.minor = config_default_vers.minor;
+		else
+			mi->version.minor = NFS_DEFAULT_MINOR;
 		return;
 	}
 
 #endif /* MOUNT_CONFIG */
-	mi->version.major = 4;
-	mi->version.minor = 2;
+	mi->version.major = NFS_DEFAULT_MAJOR;
+	mi->version.minor = NFS_DEFAULT_MINOR;
 }
 
 /*

Steve Dickson

author

2017-06-09 13:24 UTC #2 lore raw

Quoted 5 lines · NeilBrown

Quoted 3 lines · Steve Dickson
When the nfs4 filesystem specified, the default major and minor versions should be used.

Quoted 8 lines

Quoted 1 line
Signed-off-by Steve Dickson <[email protected]>
Hi Steve,
I think this is definitely heading in the right direction. I particularly like that you've #defined for the default major/minor.
However as I reflected on it, I realised that "-o v4" needs to set the same version as "-t nfs4", and even with this patch it doesn't.

Good catch!

Quoted 4 lines

I think we need to focus on nfs_default_version() and make sure it does the right thing.

Quoted 8 lines

If v.mode == V_DEFAULT, we need to copy both 'major' and 'minor' from config_default_vers if they are set. If config_default_vers.v_mode == V_GENERAL, .minor won't be set so in that case we need to use NFS_DEFAULT_MINOR;
If v.mode == V_GENERAL, then only copy the minor if it is set and the major is correct. So we need to check for V_SPECIFIC, and other wise use the default minor.

That's how it worked out...

Quoted 5 lines

If v.mode == V_SPECIFIC, nfs_default_version isn't called, so it doesn't matter.
I think this gets it right...

Also nfs_do_mount_v4() needs to check the minor
version as well...

thanks!

steved.

Quoted 58 lines

Thoughts?

Thanks,
NeilBrown

utils/mount/stropts.c +17 -5

index c0266e51ad1c..7dfdd9e62115 100644
@@ -73,6 +73,13 @@
 #define NFS_DEF_BG_TIMEOUT_MINUTES	(10000u)
 #endif
 
+#ifndef NFS_DEFAULT_MAJOR
+#define NFS_DEFAULT_MAJOR	4
+#endif
+#ifndef NFS_DEFAULT_MINOR
+#define NFS_DEFAULT_MINOR	2
+#endif
+
 extern int nfs_mount_data_version;
 extern char *progname;
 extern int verbose;

@@ -110,22 +117,27 @@ static void nfs_default_version(struct nfsmount_info *mi)
 	}
 
 	if (mi->version.v_mode == V_DEFAULT &&
-		config_default_vers.v_mode != V_DEFAULT) {
+	    config_default_vers.v_mode != V_DEFAULT) {
 		mi->version.major = config_default_vers.major;
-		mi->version.minor = config_default_vers.minor;
+		if (config_default_vers.v_mode == V_SPECIFIC)
+			mi->version.minor = config_default_vers.minor;
+		else
+			mi->version.minor = NFS_DEFAULT_MINOR;
 		return;
 	}
 
 	if (mi->version.v_mode == V_GENERAL) {
-		if (config_default_vers.v_mode != V_DEFAULT &&
+		if (config_default_vers.v_mode == V_SPECIFIC &&
 		    mi->version.major == config_default_vers.major)
 			mi->version.minor = config_default_vers.minor;
+		else
+			mi->version.minor = NFS_DEFAULT_MINOR;
 		return;
 	}
 
 #endif /* MOUNT_CONFIG */
-	mi->version.major = 4;
-	mi->version.minor = 2;
+	mi->version.major = NFS_DEFAULT_MAJOR;
+	mi->version.minor = NFS_DEFAULT_MINOR;
 }
 
 /*