Re: [PATCH 2/2] mount.nfs: Assume v2/v3 if mount-related options are present

Chuck Lever
2009-10-09T15:19:20Z

On Oct 9, 2009, at 11:12 AM, Steve Dickson wrote:

>
>
> On 10/09/2009 11:03 AM, Chuck Lever wrote:
>>
>> On Oct 9, 2009, at 9:29 AM, Steve Dickson wrote:
>>
>>>
>>>
>>> On 10/08/2009 01:37 PM, Chuck Lever wrote:
>>>> Don't try NFSv4 if any MNT protocol related options were
>>>> presented by
>>>> the user.
>>>>
>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>>> ---
>>>>
>>>> utils/mount/stropts.c | 7 +++++++
>>>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>>>> index 0685caa..3401f63 100644
>>>> --- a/utils/mount/stropts.c
>>>> +++ b/utils/mount/stropts.c
>>>> @@ -564,6 +564,13 @@ static int nfs_try_mount_v4(struct
>>>> nfsmount_info
>>>> *mi)
>>>> }
>>>>
>>>> if (mi->version == 0) {
>>>> + if (po_contains(options, "mounthost") ||
>>>> + po_contains(options, "mountaddr") ||
>>>> + po_contains(options, "mountvers") ||
>>>> + po_contains(options, "mountproto")) {
>>>> + errno = EPROTONOSUPPORT;
>>>> + goto out_fail;
>>>> + }
>>> I think it make senses to assume the version negation should
>>> start version 3 when mountXXXX options exist instead of
>>> failing a mount they probably didn't want..
>>
>> Yes, that's exactly what this patch does. NFSv4 negotiation is
>> skipped
>> if any mountfoo options are presented by the user.
>>
>> It's arguable where to put this check. This seemed like the most
>> straightforward way to deal with it.
> I guess a comment would have made it a bit clear...

Yes, it does need a comment. I thought that placing this inside the
mi->version == 0 check would have been clear enough, but I guess
not. :-)

> and I was thinking
> the check should be made before the nfs_try_mount_v4() verses having
> nfs_try_mount_v4() fail in a recoverable way...

I'll cobble up a version that does this check in nfs_try_mount()
instead, just to see what it looks like.

Alternately we can do this before starting the retry loop, but setting
mi->version = 3 would be less clear, I think, then skipping the v4
negotiation in the loop.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com