2017-06-06 19:48:16

by Steve Dickson

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

When the nfs4 filesystem specified, the default minor
version should be used not v4.0.

Signed-off-by: Steve Dickson <[email protected]>
---
utils/mount/stropts.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index c0266e5..57efb26 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -317,7 +317,8 @@ static int nfs_set_version(struct nfsmount_info *mi)

if (strncmp(mi->type, "nfs4", 4) == 0) {
mi->version.major = 4;
- mi->version.v_mode = V_GENERAL;
+ mi->version.minor = 2;
+ mi->version.v_mode = V_SPECIFIC;
}
/*
* Before 2.6.32, the kernel NFS client didn't
--
2.9.4



2017-06-08 15:36:41

by Steve Dickson

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



On 06/06/2017 10:48 PM, NeilBrown wrote:
> On Tue, Jun 06 2017, Steve Dickson wrote:
>
>> When the nfs4 filesystem specified, the default minor
>> version should be used not v4.0.
>>
>> Signed-off-by: Steve Dickson <[email protected]>
>> ---
>> utils/mount/stropts.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>> index c0266e5..57efb26 100644
>> --- a/utils/mount/stropts.c
>> +++ b/utils/mount/stropts.c
>> @@ -317,7 +317,8 @@ static int nfs_set_version(struct nfsmount_info *mi)
>>
>> if (strncmp(mi->type, "nfs4", 4) == 0) {
>> mi->version.major = 4;
>> - mi->version.v_mode = V_GENERAL;
>> + mi->version.minor = 2;
>> + mi->version.v_mode = V_SPECIFIC;
>
> I think this is wrong.
>
> By setting the mode to SPECIFIC, you are saying that if the server
> doesn't support v4.2, then fail the mount. That cannot be right.
Ok... I though since nfs4 was being specify it should be
V_SPECIFIC but I do see your point about negotiating down.

It turns out setting vers4.2=n in /etc/nfs.conf breaks
all v4 mounts... So I think we have issues in that area
ATM... ;-)

>
> Given that (currently) v_mode is not V_SPECIFIC, nfs_set_version()
> will go on and call nfs_default_version(), which will use the default
> value from the config file - just as Chuck suggests.
>
> If there is no default in the config file .... nfs_default_version()
> will do nothing. So version.minor will probably remain at zero.
> So setting
>> + mi->version.minor = 2;
>
> (where 2 is the maximum supported version) is probably correct.
> Setting
>> + mi->version.v_mode = V_SPECIFIC;
> is unnecessary and wrong.
>
> If there an easy way to find out the maximum minor version that the
> kernel supports? We should really default version.minor to that.
> Once we get up to v4.20, it'll seem odd to try to mount 4.20, 4.19,
> 4.18,... until something succeeds....
Maybe it should be something like the server does under /proc/fs/nfsd/version

steved.

>
> Thanks,
> NeilBrown
>
>> }
>> /*
>> * Before 2.6.32, the kernel NFS client didn't
>> --
>> 2.9.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

2017-06-08 16:04:18

by Steve Dickson

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



On 06/08/2017 11:36 AM, Steve Dickson wrote:
>
>
> On 06/06/2017 10:48 PM, NeilBrown wrote:
>> On Tue, Jun 06 2017, Steve Dickson wrote:
>>
>>> When the nfs4 filesystem specified, the default minor
>>> version should be used not v4.0.
>>>
>>> Signed-off-by: Steve Dickson <[email protected]>
>>> ---
>>> utils/mount/stropts.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>>> index c0266e5..57efb26 100644
>>> --- a/utils/mount/stropts.c
>>> +++ b/utils/mount/stropts.c
>>> @@ -317,7 +317,8 @@ static int nfs_set_version(struct nfsmount_info *mi)
>>>
>>> if (strncmp(mi->type, "nfs4", 4) == 0) {
>>> mi->version.major = 4;
>>> - mi->version.v_mode = V_GENERAL;
>>> + mi->version.minor = 2;
>>> + mi->version.v_mode = V_SPECIFIC;
>>
>> I think this is wrong.
>>
>> By setting the mode to SPECIFIC, you are saying that if the server
>> doesn't support v4.2, then fail the mount. That cannot be right.
> Ok... I though since nfs4 was being specify it should be
> V_SPECIFIC but I do see your point about negotiating down.
>
> It turns out setting vers4.2=n in /etc/nfs.conf breaks
> all v4 mounts... So I think we have issues in that area
> ATM... ;-)
>
>>
>> Given that (currently) v_mode is not V_SPECIFIC, nfs_set_version()
>> will go on and call nfs_default_version(), which will use the default
>> value from the config file - just as Chuck suggests.
>>
>> If there is no default in the config file .... nfs_default_version()
>> will do nothing. So version.minor will probably remain at zero.
>> So setting
>>> + mi->version.minor = 2;
>>
>> (where 2 is the maximum supported version) is probably correct.
It is because in nfs_default_version() there is this chunk of code

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;
return;
}
that ends up not setting the minor version at all which
is the reason the minor version is zero.

steved.

>> Setting
>>> + mi->version.v_mode = V_SPECIFIC;
>> is unnecessary and wrong.
>>
>> If there an easy way to find out the maximum minor version that the
>> kernel supports? We should really default version.minor to that.
>> Once we get up to v4.20, it'll seem odd to try to mount 4.20, 4.19,
>> 4.18,... until something succeeds....
> Maybe it should be something like the server does under /proc/fs/nfsd/version
>
> steved.
>
>>
>> Thanks,
>> NeilBrown
>>
>>> }
>>> /*
>>> * Before 2.6.32, the kernel NFS client didn't
>>> --
>>> 2.9.4
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2017-06-06 19:52:46

by Chuck Lever III

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


> On Jun 6, 2017, at 3:48 PM, Steve Dickson <[email protected]> wrote:
>
> When the nfs4 filesystem specified, the default minor
> version should be used not v4.0.

Isn't the default specified in the mountnfs config file?


> Signed-off-by: Steve Dickson <[email protected]>
> ---
> utils/mount/stropts.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index c0266e5..57efb26 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -317,7 +317,8 @@ static int nfs_set_version(struct nfsmount_info *mi)
>
> if (strncmp(mi->type, "nfs4", 4) == 0) {
> mi->version.major = 4;
> - mi->version.v_mode = V_GENERAL;
> + mi->version.minor = 2;
> + mi->version.v_mode = V_SPECIFIC;
> }
> /*
> * Before 2.6.32, the kernel NFS client didn't
> --
> 2.9.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Chuck Lever




2017-06-06 21:02:49

by Steve Dickson

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



On 06/06/2017 03:52 PM, Chuck Lever wrote:
>
>> On Jun 6, 2017, at 3:48 PM, Steve Dickson <[email protected]> wrote:
>>
>> When the nfs4 filesystem specified, the default minor
>> version should be used not v4.0.
>
> Isn't the default specified in the mountnfs config file?
It can be... But in general its not...

So the question is when the nfs4 file system (aka -t nfs4)
what minor version does that mean? v4.0 which was the
default in the past or the new default v4.2?

steved.
>
>
>> Signed-off-by: Steve Dickson <[email protected]>
>> ---
>> utils/mount/stropts.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>> index c0266e5..57efb26 100644
>> --- a/utils/mount/stropts.c
>> +++ b/utils/mount/stropts.c
>> @@ -317,7 +317,8 @@ static int nfs_set_version(struct nfsmount_info *mi)
>>
>> if (strncmp(mi->type, "nfs4", 4) == 0) {
>> mi->version.major = 4;
>> - mi->version.v_mode = V_GENERAL;
>> + mi->version.minor = 2;
>> + mi->version.v_mode = V_SPECIFIC;
>> }
>> /*
>> * Before 2.6.32, the kernel NFS client didn't
>> --
>> 2.9.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Chuck Lever
>
>
>

2017-06-06 21:11:06

by Chuck Lever III

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


> On Jun 6, 2017, at 5:02 PM, Steve Dickson <[email protected]> wrote:
>
>
>
> On 06/06/2017 03:52 PM, Chuck Lever wrote:
>>
>>> On Jun 6, 2017, at 3:48 PM, Steve Dickson <[email protected]> wrote:
>>>
>>> When the nfs4 filesystem specified, the default minor
>>> version should be used not v4.0.
>>
>> Isn't the default specified in the mountnfs config file?
> It can be... But in general its not...
>
> So the question is when the nfs4 file system (aka -t nfs4)
> what minor version does that mean? v4.0 which was the
> default in the past or the new default v4.2?

IMO the admin (and the distributor) should be allowed to
decide which minorversion is the default for vers=4 or
"-t nfs4".

NFSv4.0 is a safe default for now, so I'm just arguing for
taking a little more time to make this change to minor
version 2 using the config file instead of hard coding it.

No strong opinion, I just think it makes better sense to
handle default minor version the way many other default
behaviors are configured.


> steved.
>>
>>
>>> Signed-off-by: Steve Dickson <[email protected]>
>>> ---
>>> utils/mount/stropts.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>>> index c0266e5..57efb26 100644
>>> --- a/utils/mount/stropts.c
>>> +++ b/utils/mount/stropts.c
>>> @@ -317,7 +317,8 @@ static int nfs_set_version(struct nfsmount_info *mi)
>>>
>>> if (strncmp(mi->type, "nfs4", 4) == 0) {
>>> mi->version.major = 4;
>>> - mi->version.v_mode = V_GENERAL;
>>> + mi->version.minor = 2;
>>> + mi->version.v_mode = V_SPECIFIC;
>>> }
>>> /*
>>> * Before 2.6.32, the kernel NFS client didn't
>>> --
>>> 2.9.4
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> --
>> Chuck Lever
>>
>>
>>

--
Chuck Lever




2017-06-07 02:48:57

by NeilBrown

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

On Tue, Jun 06 2017, Steve Dickson wrote:

> When the nfs4 filesystem specified, the default minor
> version should be used not v4.0.
>
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> utils/mount/stropts.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index c0266e5..57efb26 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -317,7 +317,8 @@ static int nfs_set_version(struct nfsmount_info *mi)
>
> if (strncmp(mi->type, "nfs4", 4) == 0) {
> mi->version.major = 4;
> - mi->version.v_mode = V_GENERAL;
> + mi->version.minor = 2;
> + mi->version.v_mode = V_SPECIFIC;

I think this is wrong.

By setting the mode to SPECIFIC, you are saying that if the server
doesn't support v4.2, then fail the mount. That cannot be right.

Given that (currently) v_mode is not V_SPECIFIC, nfs_set_version()
will go on and call nfs_default_version(), which will use the default
value from the config file - just as Chuck suggests.

If there is no default in the config file .... nfs_default_version()
will do nothing. So version.minor will probably remain at zero.
So setting
> + mi->version.minor = 2;

(where 2 is the maximum supported version) is probably correct.
Setting
> + mi->version.v_mode = V_SPECIFIC;
is unnecessary and wrong.

If there an easy way to find out the maximum minor version that the
kernel supports? We should really default version.minor to that.
Once we get up to v4.20, it'll seem odd to try to mount 4.20, 4.19,
4.18,... until something succeeds....

Thanks,
NeilBrown

> }
> /*
> * Before 2.6.32, the kernel NFS client didn't
> --
> 2.9.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


Attachments:
signature.asc (832.00 B)