Re: [PATCH pynfs v2 5/7] nfs4.1: default to minorversion 2

Chuck Lever III
2024-10-15T19:21:05Z

> On Oct 15, 2024, at 10:23 AM, Jeff Layton <jlayton@kernel.org> wrote:
>
> On Tue, 2024-10-15 at 13:43 +0000, Chuck Lever III wrote:
>>
>>> On Oct 14, 2024, at 4:50 PM, Jeff Layton <jlayton@kernel.org> wrote:
>>>
>>> Minorversion 2 consists of all optional features, so we can safely just
>>> default to that in pynfs's 4.1 NFS4Client.
>>>
>>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>>> ---
>>> nfs4.1/nfs4client.py | 2 +-
>>> nfs4.1/testserver.py | 2 +-
>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/nfs4.1/nfs4client.py b/nfs4.1/nfs4client.py
>>> index 941cf4000a5f0da254cd826a1d41e37f652e7714..f4fabcc11be1328f47d6d738f78586b3e8541296 100644
>>> --- a/nfs4.1/nfs4client.py
>>> +++ b/nfs4.1/nfs4client.py
>>> @@ -27,7 +27,7 @@ op4 = nfs_ops.NFS4ops()
>>> SHOW_TRAFFIC = 0
>>>
>>> class NFS4Client(rpc.Client, rpc.Server):
>>> - def __init__(self, host=b'localhost', port=2049, minorversion=1, ctrl_proc=16, summary=None, secure=False):
>>> + def __init__(self, host=b'localhost', port=2049, minorversion=2, ctrl_proc=16, summary=None, secure=False):
>>> rpc.Client.__init__(self, 100003, 4)
>>> self.prog = 0x40000000
>>> self.versions = [1] # List of supported versions of prog
>>> diff --git a/nfs4.1/testserver.py b/nfs4.1/testserver.py
>>> index 085f0072388ad8a4b477073641ae16268532bc6a..0970c64efe34dcec1e5457b7025faf0cb139670c 100755
>>> --- a/nfs4.1/testserver.py
>>> +++ b/nfs4.1/testserver.py
>>> @@ -74,7 +74,7 @@ def scan_options(p):
>>> help="Store test results in xml format [%default]")
>>> p.add_option("--debug_fail", action="store_true", default=False,
>>> help="Force some checks to fail")
>>> - p.add_option("--minorversion", type="int", default=1,
>>> + p.add_option("--minorversion", type="int", default=2,
>>> metavar="MINORVERSION", help="Choose NFSv4 minor version")
>>>
>>> g = OptionGroup(p, "Security flavor options",
>>>
>>> --
>>> 2.47.0
>>>
>>>
>>
>> I'm not convinced we want to combine the NFSv4.1 and NFSv4.2
>> tests.
>>
>> How are we planning to deal with NFSv4 extensions?
>>
>
> IMO, it made sense to have different directories and tests for v4.0 vs.
> v4.1, given the protocol differences, but v4.2 is a set of extensions
> to the v4.1 protocol. I don't think we're well served by creating all a
> bunch of extra infrastructure for that when we can just extend the v4.1
> stuff.
>
> The tests in this patchset treat v4.2 functionality as optional. If the
> server advertises it, they will test it. That may not make sense for
> everything, but it should work well enough here.

I'm still not convinced, but I guess it shouldn't be actively
harmful to take this approach for now. No objection.

--
Chuck Lever