2017-06-08 18:33:17

by Steve Dickson

[permalink] [raw]
Subject: [PATCH V2] 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(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index c0266e5..81fb945 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -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
--
2.9.4



2017-06-09 06:20:26

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH V2] mount.nfs: Use default minor version when -t nfs4 is specified

On Thu, Jun 08 2017, Steve Dickson wrote:

> 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

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index c0266e51ad1c..7dfdd9e62115 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -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;
}

/*


Attachments:
signature.asc (832.00 B)

2017-06-09 13:24:36

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH V2] mount.nfs: Use default minor version when -t nfs4 is specified



On 06/09/2017 02:20 AM, NeilBrown wrote:
> On Thu, Jun 08 2017, Steve Dickson wrote:
>
>> 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.
Good catch!
>
> 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.
That's how it worked out...
>
> 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.

>
> Thoughts?
>
> Thanks,
> NeilBrown
>
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index c0266e51ad1c..7dfdd9e62115 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -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;
> }
>
> /*
>