[PATCH pynfs v2 0/7] pynfs: add CB_GETATTR tests and tests for delegated timestamps

[PATCH pynfs v2 0/7] pynfs: add CB_GETATTR tests and tests for delegated timestamps

[Jeff Layton]

I sent these a month or so ago, but Calum was on PTO. Sending again,
with some additions.

This patchset adds a couple of CB_GETATTR tests, and then updates them
to also test delegated mtime support. There is also a patch to make
the nfsv4.1 tests default to minorversion 2

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v2:
- check timestamps in WRT18, and pass_warn if they don't change
- have v4.1 tests default to minorversion 2
- have DELEG2 open the file r/w
- add support for the "delstid" draft symbols
- test delegated timestamps in new CB_GETATTR tests
- Link to v1: https://lore.kernel.org/r/20240905-cb_getattr-v1-0-0af05c68234f@kernel.org

---
Jeff Layton (7):
      WRT18: have it also check the ctime between writes
      DELEG2: fix write delegation test to open the file RW
      pynfs: update maintainer info
      nfs4.1: add two CB_GETATTR tests
      nfs4.1: default to minorversion 2
      nfs4.1: add support for the "delstid" draft
      st_deleg: test delegated timestamps in CB_GETATTR

 CONTRIBUTING                          |   6 +-
 nfs4.0/servertests/st_write.py        |  28 ++++++---
 nfs4.1/nfs4client.py                  |   8 ++-
 nfs4.1/nfs4lib.py                     |   3 +
 nfs4.1/server41tests/environment.py   |   3 +
 nfs4.1/server41tests/st_delegation.py | 102 ++++++++++++++++++++++++++++++-
 nfs4.1/testserver.py                  |   2 +-
 nfs4.1/xdrdef/nfs4.x                  | 111 ++++++++++++++++++++++++++++++++--
 8 files changed, 242 insertions(+), 21 deletions(-)
---
base-commit: c75f65983498a3254e3970da86eb6954415cac01
change-id: 20240905-cb_getattr-8db184a5b4bf

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>

[PATCH pynfs v2 1/7] WRT18: have it also check the ctime between writes

[Jeff Layton]

On many servers, the ctime doesn't have sufficient granularity to show
an apparent change between rapid writes, but some are able to do so.
Have the test also check the ctimes here and pass_warn if it doesn't
change after every write.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 nfs4.0/servertests/st_write.py | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/nfs4.0/servertests/st_write.py b/nfs4.0/servertests/st_write.py
index db1b1e585fdbf7169400ba676dd86b46e6c61750..e635717f76c93fc186521688717e25a377de7207 100644
--- a/nfs4.0/servertests/st_write.py
+++ b/nfs4.0/servertests/st_write.py
@@ -497,19 +497,27 @@ def testChangeGranularityWrite(t, env):
     c = env.c1
     c.init_connection()
     fh, stateid = c.create_confirm(t.word())
-    ops = c.use_obj(fh) + [c.getattr([FATTR4_CHANGE])] \
-        + [op.write(stateid, 0,  UNSTABLE4, _text)] + [c.getattr([FATTR4_CHANGE])] \
-        + [op.write(stateid, 10, UNSTABLE4, _text)] + [c.getattr([FATTR4_CHANGE])] \
-        + [op.write(stateid, 20, UNSTABLE4, _text)] + [c.getattr([FATTR4_CHANGE])] \
-        + [op.write(stateid, 30, UNSTABLE4, _text)] + [c.getattr([FATTR4_CHANGE])]
+    attrlist = [FATTR4_CHANGE, FATTR4_TIME_METADATA]
+    ops = c.use_obj(fh) + [c.getattr(attrlist)] \
+        + [op.write(stateid, 0,  UNSTABLE4, _text)] + [c.getattr(attrlist)] \
+        + [op.write(stateid, 10, UNSTABLE4, _text)] + [c.getattr(attrlist)] \
+        + [op.write(stateid, 20, UNSTABLE4, _text)] + [c.getattr(attrlist)] \
+        + [op.write(stateid, 30, UNSTABLE4, _text)] + [c.getattr(attrlist)]
     res = c.compound(ops)
     check(res)
-    chattr1 = res.resarray[1].obj_attributes
-    chattr2 = res.resarray[3].obj_attributes
-    chattr3 = res.resarray[5].obj_attributes
-    chattr4 = res.resarray[7].obj_attributes
+    chattr1 = res.resarray[1].obj_attributes[FATTR4_CHANGE]
+    chattr2 = res.resarray[3].obj_attributes[FATTR4_CHANGE]
+    chattr3 = res.resarray[5].obj_attributes[FATTR4_CHANGE]
+    chattr4 = res.resarray[7].obj_attributes[FATTR4_CHANGE]
     if chattr1 == chattr2 or chattr2 == chattr3 or chattr3 == chattr4:
-        t.fail("consecutive SETATTR(mode)'s don't all change change attribute")
+        t.fail("consecutive WRITE's don't change change attribute")
+
+    ctime1 = res.resarray[1].obj_attributes[FATTR4_TIME_METADATA]
+    ctime2 = res.resarray[3].obj_attributes[FATTR4_TIME_METADATA]
+    ctime3 = res.resarray[5].obj_attributes[FATTR4_TIME_METADATA]
+    ctime4 = res.resarray[7].obj_attributes[FATTR4_TIME_METADATA]
+    if compareTimes(ctime1, ctime2) == 0 or compareTimes(ctime2, ctime3) == 0 or compareTimes(ctime3, ctime4) == 0:
+        t.pass_warn("consecutive WRITE's don't all change time_metadata")
 
 def testStolenStateid(t, env):
     """WRITE with incorrect permissions and somebody else's stateid

-- 
2.47.0

[PATCH pynfs v2 2/7] DELEG2: fix write delegation test to open the file RW

[Jeff Layton]

The Linux NFS server currently will only grant a write delegation on a
r/w open, so this test fails to get a delegation there. The test should
work just as well with a r/w open.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 nfs4.1/server41tests/st_delegation.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nfs4.1/server41tests/st_delegation.py b/nfs4.1/server41tests/st_delegation.py
index a25a042578b49d005f4f7ea741e623b44c0a0e2a..80b0da28fbad85429fc1f4c0e759be82b0cc5c08 100644
--- a/nfs4.1/server41tests/st_delegation.py
+++ b/nfs4.1/server41tests/st_delegation.py
@@ -79,7 +79,7 @@ def testWriteDeleg(t, env):
     FLAGS: writedelegations deleg
     CODE: DELEG2
     """
-    _testDeleg(t, env, OPEN4_SHARE_ACCESS_WRITE,
+    _testDeleg(t, env, OPEN4_SHARE_ACCESS_READ|OPEN4_SHARE_ACCESS_WRITE,
        OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG, OPEN4_SHARE_ACCESS_READ)
 
 def testAnyDeleg(t, env):

-- 
2.47.0

[PATCH pynfs v2 3/7] pynfs: update maintainer info

[Jeff Layton]

Calum is the new maintainer. Update the CONTRIBUTING doc.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 CONTRIBUTING | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/CONTRIBUTING b/CONTRIBUTING
index 220b3e0629aa658e3235ab5d0a7c13e271480cd5..d15b2b8278797b18c91fad740290c9355a5f3d29 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -1,15 +1,15 @@
 Check out the latest source code from
 
-	git://git.linux-nfs.org/projects/bfields/pynfs.git
+	git://git.linux-nfs.org/projects/cdmackay/pynfs.git
 
 Commit your changes.  Please make sure each commit is individually
 correct and does only one thing.
 
-When you're done, send the resulting patches to bfields@fieldses.org and
+When you're done, send the resulting patches to calum.mackay@oracle.com and
 cc the linux-nfs@vger.kernel.org mailing list, using something like:
 
 	git format-patch origin..
-	git send-email --to="bfields@fieldses.org" \
+	git send-email --to="calum.mackay@oracle.com" \
 			--cc="linux-nfs@vger.kernel.org 0*
 
 We generally follow similar processes to the Linux kernel, so

-- 
2.47.0

[PATCH pynfs v2 4/7] nfs4.1: add two CB_GETATTR tests

[Jeff Layton]

Add tests for CB_GETATTR support.

Open a file r/w and request a write delegation. Do a getattr against it
to get the current size and change attr. From a second client, issue a
GETATTR against the file.

One test does this and has the delegation holder send back the same
attrs, another has it send back updated ones.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 nfs4.1/nfs4client.py                  |  6 +++
 nfs4.1/server41tests/st_delegation.py | 72 ++++++++++++++++++++++++++++++++++-
 2 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/nfs4.1/nfs4client.py b/nfs4.1/nfs4client.py
index 8ce64dd639c75ea14b24bd5b732ddbe0fcb0fa71..941cf4000a5f0da254cd826a1d41e37f652e7714 100644
--- a/nfs4.1/nfs4client.py
+++ b/nfs4.1/nfs4client.py
@@ -271,6 +271,12 @@ class NFS4Client(rpc.Client, rpc.Server):
         res = self.posthook(arg, env, res)
         return encode_status(NFS4_OK, res)
 
+    def op_cb_getattr(self, arg, env):
+        log_cb.info("In CB_GETATTR")
+        self.prehook(arg, env)
+        res = self.posthook(arg, env, res=CB_GETATTR4resok())
+        return encode_status(NFS4_OK, res)
+
     def op_cb_recall(self, arg, env):
         log_cb.info("In CB_RECALL")
         self.prehook(arg, env)
diff --git a/nfs4.1/server41tests/st_delegation.py b/nfs4.1/server41tests/st_delegation.py
index 80b0da28fbad85429fc1f4c0e759be82b0cc5c08..2aa73ba7acd0bd857a4fd5206b8857f980176d73 100644
--- a/nfs4.1/server41tests/st_delegation.py
+++ b/nfs4.1/server41tests/st_delegation.py
@@ -2,7 +2,7 @@ from .st_create_session import create_session
 from .st_open import open_claim4
 from xdrdef.nfs4_const import *
 
-from .environment import check, fail, create_file, open_file, close_file
+from .environment import check, fail, create_file, open_file, close_file, do_getattrdict
 from xdrdef.nfs4_type import *
 import nfs_ops
 op = nfs_ops.NFS4ops()
@@ -289,3 +289,73 @@ def testServerSelfConflict3(t, env):
     check(res, [NFS4_OK, NFS4ERR_DELAY])
     if not completed:
         fail("delegation break not received")
+
+def _testCbGetattr(t, env, change=0, size=0):
+    cb = threading.Event()
+    cbattrs = {}
+    def getattr_post_hook(arg, env, res):
+        res.obj_attributes = cbattrs
+        env.notify = cb.set
+        return res
+
+    sess1 = env.c1.new_client_session(b"%s_1" % env.testname(t))
+    sess1.client.cb_post_hook(OP_CB_GETATTR, getattr_post_hook)
+
+    fh, deleg = __create_file_with_deleg(sess1, env.testname(t),
+                                                OPEN4_SHARE_ACCESS_READ  |
+                                                OPEN4_SHARE_ACCESS_WRITE |
+                                                OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG)
+    print("__create_file_with_deleg: ", fh, deleg)
+    attrs1 = do_getattrdict(sess1, fh, [FATTR4_CHANGE, FATTR4_SIZE])
+    cbattrs = dict(attrs1)
+
+    if change != 0:
+        cbattrs[FATTR4_CHANGE] += 1
+        if size > 0:
+            cbattrs[FATTR4_SIZE] = size
+
+    # create a new client session and do a GETATTR
+    sess2 = env.c1.new_client_session(b"%s_2" % env.testname(t))
+    slot = sess2.compound_async([op.putfh(fh), op.getattr(1<<FATTR4_CHANGE | 1<<FATTR4_SIZE)])
+
+    # wait for the CB_GETATTR
+    completed = cb.wait(2)
+    res = sess2.listen(slot)
+    attrs2 = res.resarray[-1].obj_attributes
+    sess1.compound([op.putfh(fh), op.delegreturn(deleg.write.stateid)])
+    check(res, [NFS4_OK, NFS4ERR_DELAY])
+    if not completed:
+        fail("CB_GETATTR not received")
+    return attrs1, attrs2
+
+def testCbGetattrNoChange(t, env):
+    """Test CB_GETATTR with no changes
+
+    Get a write delegation, then do a getattr from a second client. Have the
+    client regurgitate back the same attrs (indicating no changes). Then test
+    that the attrs that the second client gets back match the first.
+
+    FLAGS: deleg
+    CODE: DELEG24
+    """
+    attrs1, attrs2 = _testCbGetattr(t, env)
+    if attrs1[FATTR4_SIZE] != attrs2[FATTR4_SIZE]:
+        fail("Bad size: %u != %u" % (attrs1[FATTR4_SIZE], attrs2[FATTR4_SIZE]))
+    if attrs1[FATTR4_CHANGE] != attrs2[FATTR4_CHANGE]:
+        fail("Bad change attribute: %u != %u" % (attrs1[FATTR4_CHANGE], attrs2[FATTR4_CHANGE]))
+
+def testCbGetattrWithChange(t, env):
+    """Test CB_GETATTR with simulated changes to file
+
+    Get a write delegation, then do a getattr from a second client. Modify the
+    attrs before sending them back to the server. Test that the second client
+    sees different attrs than the original one.
+
+    FLAGS: deleg
+    CODE: DELEG25
+    """
+    attrs1, attrs2 = _testCbGetattr(t, env, change=1, size=5)
+    if attrs2[FATTR4_SIZE] != 5:
+        fail("Bad size: %u != 5" % attrs2[FATTR4_SIZE])
+    if attrs1[FATTR4_CHANGE] == attrs2[FATTR4_CHANGE]:
+        fail("Bad change attribute: %u == %u" % (attrs1[FATTR4_CHANGE], attrs2[FATTR4_CHANGE]))

-- 
2.47.0

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

[Jeff Layton]

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

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

[Chuck Lever III]

> 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?


--
Chuck Lever

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

[Jeff Layton]

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.
-- 
Jeff Layton <jlayton@kernel.org>

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

[Chuck Lever III]

> 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

[PATCH pynfs v2 6/7] nfs4.1: add support for the "delstid" draft

[Jeff Layton]

Add the new .x snippet to nfs4.x, and the necessary info about the new
attributes.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 nfs4.1/nfs4lib.py                   |   3 +
 nfs4.1/server41tests/environment.py |   3 +
 nfs4.1/xdrdef/nfs4.x                | 111 ++++++++++++++++++++++++++++++++++--
 3 files changed, 113 insertions(+), 4 deletions(-)

diff --git a/nfs4.1/nfs4lib.py b/nfs4.1/nfs4lib.py
index b1a247b02f66a529da7f2bd98ec436f168b30873..d3a1550f1ce1e135b124a59fa1518c9ff89fd502 100644
--- a/nfs4.1/nfs4lib.py
+++ b/nfs4.1/nfs4lib.py
@@ -731,5 +731,8 @@ attr_info = { FATTR4_SUPPORTED_ATTRS : A("r", "fs"),
               FATTR4_MODE_SET_MASKED : A("w", "obj"),
               FATTR4_FS_CHARSET_CAP : A("r", "fs"),
               FATTR4_XATTR_SUPPORT : A("r", "obj"),
+              FATTR4_TIME_DELEG_ACCESS : A("w", "obj"),
+              FATTR4_TIME_DELEG_MODIFY : A("w", "obj"),
+              FATTR4_OPEN_ARGUMENTS : A("r", "fs"),
               }
 del A
diff --git a/nfs4.1/server41tests/environment.py b/nfs4.1/server41tests/environment.py
index 0b7c976d8582a0e4ef5bd1d21ce34247f26899e4..48284e029634fff60b7690e058cd4131bfea9b08 100644
--- a/nfs4.1/server41tests/environment.py
+++ b/nfs4.1/server41tests/environment.py
@@ -108,6 +108,9 @@ class Environment(testmod.Environment):
         AttrInfo('time_modify_set', 'w', settime4(0)),
         AttrInfo('mounted_on_fileid', 'r', 0),
         AttrInfo('xattr_support', 'r', False),
+        AttrInfo('time_deleg_access', 'r', nfstime4(0, 0)),
+        AttrInfo('time_deleg_modify', 'r', nfstime4(0, 0)),
+        AttrInfo('open_arguments', 'r', open_arguments4()),
         ]
 
     home = property(lambda s: use_obj(s.opts.home))
diff --git a/nfs4.1/xdrdef/nfs4.x b/nfs4.1/xdrdef/nfs4.x
index 7b4e755369d749c2570a624895c234b1454df0cb..ee3da8aa7a342e4d3829f4b1b1f82543275199c5 100644
--- a/nfs4.1/xdrdef/nfs4.x
+++ b/nfs4.1/xdrdef/nfs4.x
@@ -1742,10 +1742,12 @@ const
  = 0x20000;
 
 enum open_delegation_type4 {
-        OPEN_DELEGATE_NONE      = 0,
-        OPEN_DELEGATE_READ      = 1,
-        OPEN_DELEGATE_WRITE     = 2,
-        OPEN_DELEGATE_NONE_EXT  = 3 /* new to v4.1 */
+        OPEN_DELEGATE_NONE              = 0,
+        OPEN_DELEGATE_READ              = 1,
+        OPEN_DELEGATE_WRITE             = 2,
+        OPEN_DELEGATE_NONE_EXT          = 3, /* new to v4.1 */
+        OPEN_DELEGATE_READ_ATTRS_DELEG  = 4,
+        OPEN_DELEGATE_WRITE_ATTRS_DELEG = 5
 };
 
 enum open_claim_type4 {
@@ -1921,8 +1923,10 @@ switch (open_delegation_type4 delegation_type) {
         case OPEN_DELEGATE_NONE:
                 void;
         case OPEN_DELEGATE_READ:
+        case OPEN_DELEGATE_READ_ATTRS_DELEG:
                 open_read_delegation4 read;
         case OPEN_DELEGATE_WRITE:
+        case OPEN_DELEGATE_WRITE_ATTRS_DELEG:
                 open_write_delegation4 write;
         case OPEN_DELEGATE_NONE_EXT: /* new to v4.1 */
                 open_none_delegation4 od_whynone;
@@ -3949,3 +3953,102 @@ program NFS4_CALLBACK {
                         CB_COMPOUND(CB_COMPOUND4args) = 1;
         } = 1;
 } = 0x40000000;
+
+/*
+ * The following content was extracted from draft-ietf-nfsv4-delstid
+ */
+
+typedef bool            fattr4_offline;
+
+
+const FATTR4_OFFLINE            = 83;
+
+
+struct open_arguments4 {
+  bitmap4  oa_share_access;
+  bitmap4  oa_share_deny;
+  bitmap4  oa_share_access_want;
+  bitmap4  oa_open_claim;
+  bitmap4  oa_create_mode;
+};
+
+
+enum open_args_share_access4 {
+   OPEN_ARGS_SHARE_ACCESS_READ  = 1,
+   OPEN_ARGS_SHARE_ACCESS_WRITE = 2,
+   OPEN_ARGS_SHARE_ACCESS_BOTH  = 3
+};
+
+
+enum open_args_share_deny4 {
+   OPEN_ARGS_SHARE_DENY_NONE  = 0,
+   OPEN_ARGS_SHARE_DENY_READ  = 1,
+   OPEN_ARGS_SHARE_DENY_WRITE = 2,
+   OPEN_ARGS_SHARE_DENY_BOTH  = 3
+};
+
+
+enum open_args_share_access_want4 {
+   OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG           = 3,
+   OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG            = 4,
+   OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL              = 5,
+   OPEN_ARGS_SHARE_ACCESS_WANT_SIGNAL_DELEG_WHEN_RESRC_AVAIL
+                                                   = 17,
+   OPEN_ARGS_SHARE_ACCESS_WANT_PUSH_DELEG_WHEN_UNCONTENDED
+                                                   = 18,
+   OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS    = 20,
+   OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION = 21
+};
+
+
+enum open_args_open_claim4 {
+   OPEN_ARGS_OPEN_CLAIM_NULL          = 0,
+   OPEN_ARGS_OPEN_CLAIM_PREVIOUS      = 1,
+   OPEN_ARGS_OPEN_CLAIM_DELEGATE_CUR  = 2,
+   OPEN_ARGS_OPEN_CLAIM_DELEGATE_PREV = 3,
+   OPEN_ARGS_OPEN_CLAIM_FH            = 4,
+   OPEN_ARGS_OPEN_CLAIM_DELEG_CUR_FH  = 5,
+   OPEN_ARGS_OPEN_CLAIM_DELEG_PREV_FH = 6
+};
+
+
+enum open_args_createmode4 {
+   OPEN_ARGS_CREATEMODE_UNCHECKED4     = 0,
+   OPEN_ARGS_CREATE_MODE_GUARDED       = 1,
+   OPEN_ARGS_CREATEMODE_EXCLUSIVE4     = 2,
+   OPEN_ARGS_CREATE_MODE_EXCLUSIVE4_1  = 3
+};
+
+
+typedef open_arguments4 fattr4_open_arguments;
+
+
+%/*
+% * Determine what OPEN supports.
+% */
+const FATTR4_OPEN_ARGUMENTS     = 86;
+
+
+const OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION = 0x200000;
+
+
+const OPEN4_RESULT_NO_OPEN_STATEID = 0x00000010;
+
+
+/*
+ * attributes for the delegation times being
+ * cached and served by the "client"
+ */
+typedef nfstime4        fattr4_time_deleg_access;
+typedef nfstime4        fattr4_time_deleg_modify;
+
+
+%/*
+% * New RECOMMENDED Attribute for
+% * delegation caching of times
+% */
+const FATTR4_TIME_DELEG_ACCESS  = 84;
+const FATTR4_TIME_DELEG_MODIFY  = 85;
+
+
+const OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS = 0x100000;

-- 
2.47.0

[PATCH pynfs v2 7/7] st_deleg: test delegated timestamps in CB_GETATTR

[Jeff Layton]

First, query the server to get the SUPPORTED_ATTRS and OPEN_ARGUMENTS,
and determine whether delegated timestamps are supported. If they are,
request them, and test whether they are as expected.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 nfs4.1/server41tests/st_delegation.py | 42 +++++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/nfs4.1/server41tests/st_delegation.py b/nfs4.1/server41tests/st_delegation.py
index 2aa73ba7acd0bd857a4fd5206b8857f980176d73..fc374e693cb4b9a9adaaf5ff15a64a02573113b0 100644
--- a/nfs4.1/server41tests/st_delegation.py
+++ b/nfs4.1/server41tests/st_delegation.py
@@ -301,22 +301,44 @@ def _testCbGetattr(t, env, change=0, size=0):
     sess1 = env.c1.new_client_session(b"%s_1" % env.testname(t))
     sess1.client.cb_post_hook(OP_CB_GETATTR, getattr_post_hook)
 
-    fh, deleg = __create_file_with_deleg(sess1, env.testname(t),
-                                                OPEN4_SHARE_ACCESS_READ  |
-                                                OPEN4_SHARE_ACCESS_WRITE |
-                                                OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG)
+    res = sess1.compound([op.putrootfh(),
+                          op.getattr(nfs4lib.list2bitmap([FATTR4_SUPPORTED_ATTRS,
+                                                          FATTR4_OPEN_ARGUMENTS]))])
+    check(res)
+    caps = res.resarray[-1].obj_attributes
+
+    openmask = (OPEN4_SHARE_ACCESS_READ  |
+                OPEN4_SHARE_ACCESS_WRITE |
+                OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG)
+
+    if caps[FATTR4_SUPPORTED_ATTRS] & FATTR4_OPEN_ARGUMENTS:
+        if caps[FATTR4_OPEN_ARGUMENTS].oa_share_access_want & OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS:
+            openmask |= 1<<OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS
+
+    fh, deleg = __create_file_with_deleg(sess1, env.testname(t), openmask)
     print("__create_file_with_deleg: ", fh, deleg)
-    attrs1 = do_getattrdict(sess1, fh, [FATTR4_CHANGE, FATTR4_SIZE])
-    cbattrs = dict(attrs1)
+    attrs1 = do_getattrdict(sess1, fh, [FATTR4_CHANGE, FATTR4_SIZE,
+                                        FATTR4_TIME_ACCESS, FATTR4_TIME_MODIFY])
+
+    cbattrs[FATTR4_CHANGE] = attrs1[FATTR4_CHANGE]
+    cbattrs[FATTR4_SIZE] = attrs1[FATTR4_SIZE]
 
     if change != 0:
         cbattrs[FATTR4_CHANGE] += 1
         if size > 0:
             cbattrs[FATTR4_SIZE] = size
 
+    if openmask & 1<<OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS:
+        cbattrs[FATTR4_TIME_DELEG_ACCESS] = attrs1[FATTR4_TIME_ACCESS]
+        cbattrs[FATTR4_TIME_DELEG_MODIFY] = attrs1[FATTR4_TIME_MODIFY]
+        if change != 0:
+            cbattrs[FATTR4_TIME_DELEG_ACCESS].seconds += 1
+            cbattrs[FATTR4_TIME_DELEG_MODIFY].seconds += 1
+
     # create a new client session and do a GETATTR
     sess2 = env.c1.new_client_session(b"%s_2" % env.testname(t))
-    slot = sess2.compound_async([op.putfh(fh), op.getattr(1<<FATTR4_CHANGE | 1<<FATTR4_SIZE)])
+    slot = sess2.compound_async([op.putfh(fh), op.getattr(1<<FATTR4_CHANGE | 1<<FATTR4_SIZE |
+                                                          1<<FATTR4_TIME_ACCESS | 1<<FATTR4_TIME_MODIFY)])
 
     # wait for the CB_GETATTR
     completed = cb.wait(2)
@@ -343,6 +365,9 @@ def testCbGetattrNoChange(t, env):
         fail("Bad size: %u != %u" % (attrs1[FATTR4_SIZE], attrs2[FATTR4_SIZE]))
     if attrs1[FATTR4_CHANGE] != attrs2[FATTR4_CHANGE]:
         fail("Bad change attribute: %u != %u" % (attrs1[FATTR4_CHANGE], attrs2[FATTR4_CHANGE]))
+    if FATTR4_TIME_DELEG_MODIFY in attrs2:
+        if attrs1[FATTR4_TIME_MODIFY] != attrs2[FATTR4_TIME_DELEG_MODIFY]:
+            fail("Bad modify time: ", attrs1[FATTR4_TIME_MODIFY], " != ", attrs2[FATTR4_TIME_DELEG_MODIFY])
 
 def testCbGetattrWithChange(t, env):
     """Test CB_GETATTR with simulated changes to file
@@ -359,3 +384,6 @@ def testCbGetattrWithChange(t, env):
         fail("Bad size: %u != 5" % attrs2[FATTR4_SIZE])
     if attrs1[FATTR4_CHANGE] == attrs2[FATTR4_CHANGE]:
         fail("Bad change attribute: %u == %u" % (attrs1[FATTR4_CHANGE], attrs2[FATTR4_CHANGE]))
+    if FATTR4_TIME_DELEG_MODIFY in attrs2:
+        if attrs1[FATTR4_TIME_MODIFY] == attrs2[FATTR4_TIME_DELEG_MODIFY]:
+            fail("Bad modify time: ", attrs1[FATTR4_TIME_MODIFY], " == ", attrs2[FATTR4_TIME_DELEG_MODIFY])

-- 
2.47.0

Re: [PATCH pynfs v2 0/7] pynfs: add CB_GETATTR tests and tests for delegated timestamps

[Calum Mackay]

[-- Attachment #1.1: Type: text/plain, Size: 1930 bytes --]

thanks very much Jeff, and for the reminder. I'll look at these this week.

cheers,
c.

On 14/10/2024 9:50 pm, Jeff Layton wrote:
> I sent these a month or so ago, but Calum was on PTO. Sending again,
> with some additions.
> 
> This patchset adds a couple of CB_GETATTR tests, and then updates them
> to also test delegated mtime support. There is also a patch to make
> the nfsv4.1 tests default to minorversion 2
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> Changes in v2:
> - check timestamps in WRT18, and pass_warn if they don't change
> - have v4.1 tests default to minorversion 2
> - have DELEG2 open the file r/w
> - add support for the "delstid" draft symbols
> - test delegated timestamps in new CB_GETATTR tests
> - Link to v1: https://lore.kernel.org/r/20240905-cb_getattr-v1-0-0af05c68234f@kernel.org
> 
> ---
> Jeff Layton (7):
>        WRT18: have it also check the ctime between writes
>        DELEG2: fix write delegation test to open the file RW
>        pynfs: update maintainer info
>        nfs4.1: add two CB_GETATTR tests
>        nfs4.1: default to minorversion 2
>        nfs4.1: add support for the "delstid" draft
>        st_deleg: test delegated timestamps in CB_GETATTR
> 
>   CONTRIBUTING                          |   6 +-
>   nfs4.0/servertests/st_write.py        |  28 ++++++---
>   nfs4.1/nfs4client.py                  |   8 ++-
>   nfs4.1/nfs4lib.py                     |   3 +
>   nfs4.1/server41tests/environment.py   |   3 +
>   nfs4.1/server41tests/st_delegation.py | 102 ++++++++++++++++++++++++++++++-
>   nfs4.1/testserver.py                  |   2 +-
>   nfs4.1/xdrdef/nfs4.x                  | 111 ++++++++++++++++++++++++++++++++--
>   8 files changed, 242 insertions(+), 21 deletions(-)
> ---
> base-commit: c75f65983498a3254e3970da86eb6954415cac01
> change-id: 20240905-cb_getattr-8db184a5b4bf
> 
> Best regards,



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH pynfs v2 0/7] pynfs: add CB_GETATTR tests and tests for delegated timestamps

[Calum Mackay]

[-- Attachment #1.1: Type: text/plain, Size: 1924 bytes --]

On 14/10/2024 9:50 pm, Jeff Layton wrote:
> I sent these a month or so ago, but Calum was on PTO. Sending again,
> with some additions.
> 
> This patchset adds a couple of CB_GETATTR tests, and then updates them
> to also test delegated mtime support. There is also a patch to make
> the nfsv4.1 tests default to minorversion 2

Applied; apologies for the delay.

Thanks very much, Jeff.

cheers,
calum.

> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> Changes in v2:
> - check timestamps in WRT18, and pass_warn if they don't change
> - have v4.1 tests default to minorversion 2
> - have DELEG2 open the file r/w
> - add support for the "delstid" draft symbols
> - test delegated timestamps in new CB_GETATTR tests
> - Link to v1: https://lore.kernel.org/r/20240905-cb_getattr-v1-0-0af05c68234f@kernel.org
> 
> ---
> Jeff Layton (7):
>        WRT18: have it also check the ctime between writes
>        DELEG2: fix write delegation test to open the file RW
>        pynfs: update maintainer info
>        nfs4.1: add two CB_GETATTR tests
>        nfs4.1: default to minorversion 2
>        nfs4.1: add support for the "delstid" draft
>        st_deleg: test delegated timestamps in CB_GETATTR
> 
>   CONTRIBUTING                          |   6 +-
>   nfs4.0/servertests/st_write.py        |  28 ++++++---
>   nfs4.1/nfs4client.py                  |   8 ++-
>   nfs4.1/nfs4lib.py                     |   3 +
>   nfs4.1/server41tests/environment.py   |   3 +
>   nfs4.1/server41tests/st_delegation.py | 102 ++++++++++++++++++++++++++++++-
>   nfs4.1/testserver.py                  |   2 +-
>   nfs4.1/xdrdef/nfs4.x                  | 111 ++++++++++++++++++++++++++++++++--
>   8 files changed, 242 insertions(+), 21 deletions(-)
> ---
> base-commit: c75f65983498a3254e3970da86eb6954415cac01
> change-id: 20240905-cb_getattr-8db184a5b4bf
> 
> Best regards,




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]