On Fri, Jun 09 2017, Steve Dickson wrote:
> When v4 is specified on the command line the
> default minor version needs to be used.
>
> Signed-off-by: Steve Dickson <steved@xxxxxxxxxx>
> ---
> utils/mount/stropts.c | 27 ++++++++++++++++++++-------
> 1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 81fb945..d2d303f 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -119,14 +119,22 @@ static void nfs_default_version(struct nfsmount_info *mi)
> if (mi->version.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.minor)
> + mi->version.minor = config_default_vers.minor;
> + else if (!mi->version.minor)
> + mi->version.minor = NFS_DEFAULT_MINOR;
No, this looks wrong. A minor number of '0' can be perfectly valid.
Deciding to turn a minor of '0' into '2' is simply wrong.
You can only tell if .minor is valid by looking at .v_mode.
If .v_mode is V_GENERAL or V_DEFAULT, then .minor is not valid
and a default should be used. If it is V_SPECIFIC, then .minor is
either valid or irrelevant, depending on .major.
> return;
> }
>
> if (mi->version.v_mode == V_GENERAL) {
> if (config_default_vers.v_mode != V_DEFAULT &&
> - mi->version.major == config_default_vers.major)
> - mi->version.minor = config_default_vers.minor;
> + mi->version.major == config_default_vers.major) {
> + if (mi->version.minor)
> + mi->version.minor = config_default_vers.minor;
Again, don't test version.minor like this.
> + else if (!mi->version.minor)
> + mi->version.minor = NFS_DEFAULT_MINOR;
> + } else if (!mi->version.minor)
> + mi->version.minor = NFS_DEFAULT_MINOR;
> return;
> }
>
> @@ -740,10 +748,15 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi,
> }
>
> if (mi->version.v_mode != V_SPECIFIC) {
> - if (mi->version.v_mode == V_GENERAL)
> - snprintf(version_opt, sizeof(version_opt) - 1,
> - "vers=%lu", mi->version.major);
> - else
> + if (mi->version.v_mode == V_GENERAL) {
> + if (mi->version.major > 3)
This test is pointless as .v_mode is only ever V_GENERAL when
.major == 4
And given that this is nfs_do_mount_v4(), we can be certain that
version.major == 4.
Prior to
Commit: 0d71b058092f ("NFS: Extend the -overs= mount option to allow 4.x minorversions")
in Linux 3.4, writing "vers=4.1" wasn't supported.
So I think it would be safest to use
vers=4 if minor == 0
vers=4,minorversion=1 if minor == 1
vers=4.2 if minor == 2
This is despite the fact that the comment in the kernel says
In future,
* the mount program should always supply
* a NFSv4 minor version number.
I think vers=4.%d is appropriate for v4.2 and later, but not for earlier
versions.
Alternately we could test the kernel version and behave differently, but
I think that is worse.
Thanks,
NeilBrown
> + snprintf(version_opt, sizeof(version_opt) - 1,
> + "vers=%lu.%lu", mi->version.major,
> + mi->version.minor);
> + else
> + snprintf(version_opt, sizeof(version_opt) - 1,
> + "vers=%lu", mi->version.major);
> + } else
> snprintf(version_opt, sizeof(version_opt) - 1,
> "vers=%lu.%lu", mi->version.major,
> mi->version.minor);
> --
> 2.9.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: PGP signature
