Re: [PATCH pynfs RFC] pynfs: add v4.1+ st_delegation and st_xattr tests to "all" group

[Calum Mackay <calum.mackay@xxxxxxxxxx>]

On 04/03/2025 8:37 pm, Calum Mackay wrote:
> On 20/02/2025 4:54 pm, Jeff Layton wrote:
> > These tests all pass against a fully up-to-date Linux knfsd, and I think
> > should pass against ganesha as well. Add the "all" flag to these tests.
> >
> > Cc: Frank Filz <ffilzlnx@xxxxxxxxxxxxxx>
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > Should we add the "all" tag to these tests?

hi Jeff,

Chuck reports that some of these tests break on older NFS server 
kernels, so perhaps aren't suited to "all", after all.

Might it be better to only add to "all" to those tests which do work on 
both current and older kernels, for some value of "older"?

I could revert this for now, unless you can split them accordingly 
shortly, but for the moment I'll leave it in.


Anyone having difficulty with the latest pynfs and older kernels can, 
for the moment, use the "pynfs-0.1" tag.

thanks!

cheers,
c.


> > It might also be good to
> > think about tagging out a release before this change, so that we can
> > tell people to use a specific version if they're hitting problems.
>
> Thanks very much, Jeff.
>
> Applied, and tagged "pynfs-0.2", as there's no other pending changes.
>
>
> The previous commit was given the tag "pynfs-0.1"; before that we've not 
> had tags in the current repo.
>
> cheers,
> c.
>
>
>
> > ---
> >   nfs4.1/server41tests/st_delegation.py | 24 ++++++++++++------------
> >   nfs4.1/server41tests/st_xattr.py      | 22 +++++++++++-----------
> >   2 files changed, 23 insertions(+), 23 deletions(-)
> >
> > diff --git a/nfs4.1/server41tests/st_delegation.py b/nfs4.1/ 
> > server41tests/st_delegation.py
> > index 
> > fc374e693cb4b9a9adaaf5ff15a64a02573113b0..fa9b4515dba25c6dd0bf11b409b6eacf5e783cbd 100644
> > --- a/nfs4.1/server41tests/st_delegation.py
> > +++ b/nfs4.1/server41tests/st_delegation.py
> > @@ -67,7 +67,7 @@ def _testDeleg(t, env, openaccess, want, 
> > breakaccess, sec = None, sec2 = None):
> >   def testReadDeleg(t, env):
> >       """Test read delegation handout and return
> > -    FLAGS: open deleg
> > +    FLAGS: open deleg all
> >       CODE: DELEG1
> >       """
> >       _testDeleg(t, env, OPEN4_SHARE_ACCESS_READ,
> > @@ -76,7 +76,7 @@ def testReadDeleg(t, env):
> >   def testWriteDeleg(t, env):
> >       """Test write delegation handout and return
> > -    FLAGS: writedelegations deleg
> > +    FLAGS: writedelegations deleg all
> >       CODE: DELEG2
> >       """
> >       _testDeleg(t, env, OPEN4_SHARE_ACCESS_READ| 
> > OPEN4_SHARE_ACCESS_WRITE,
> > @@ -85,7 +85,7 @@ def testWriteDeleg(t, env):
> >   def testAnyDeleg(t, env):
> >       """Test any delegation handout and return
> > -    FLAGS: open deleg
> > +    FLAGS: open deleg all
> >       CODE: DELEG3
> >       """
> >       _testDeleg(t, env, OPEN4_SHARE_ACCESS_READ,
> > @@ -94,7 +94,7 @@ def testAnyDeleg(t, env):
> >   def testNoDeleg(t, env):
> >       """Test no delegation handout
> > -    FLAGS: open deleg
> > +    FLAGS: open deleg all
> >       CODE: DELEG4
> >       """
> >       sess1 = env.c1.new_client_session(b"%s_1" % env.testname(t))
> > @@ -115,7 +115,7 @@ def testNoDeleg(t, env):
> >   def testCBSecParms(t, env):
> >       """Test auth_sys callbacks
> > -    FLAGS: create_session open deleg
> > +    FLAGS: create_session open deleg all
> >       CODE: DELEG5
> >       """
> >       uid = 17
> > @@ -131,7 +131,7 @@ def testCBSecParms(t, env):
> >   def testCBSecParmsNull(t, env):
> >       """Test auth_null callbacks
> > -    FLAGS: create_session open deleg
> > +    FLAGS: create_session open deleg all
> >       CODE: DELEG6
> >       """
> >       recall = _testDeleg(t, env, OPEN4_SHARE_ACCESS_READ,
> > @@ -144,7 +144,7 @@ def testCBSecParmsNull(t, env):
> >   def testCBSecParmsChange(t, env):
> >       """Test changing of auth_sys callbacks with backchannel_ctl
> > -    FLAGS: create_session open deleg backchannel_ctl
> > +    FLAGS: create_session open deleg backchannel_ctl all
> >       CODE: DELEG7
> >       """
> >       uid1 = 17
> > @@ -165,7 +165,7 @@ def testDelegRevocation(t, env):
> >       """Allow a delegation to be revoked, check that TEST_STATEID and
> >          FREE_STATEID have the required effect.
> > -    FLAGS: deleg
> > +    FLAGS: deleg all
> >       CODE: DELEG8
> >       """
> > @@ -220,7 +220,7 @@ def testDelegRevocation(t, env):
> >   def testWriteOpenvsReadDeleg(t, env):
> >       """Ensure that a write open prevents granting a read delegation
> > -    FLAGS: deleg
> > +    FLAGS: deleg all
> >       CODE: DELEG9
> >       """
> > @@ -249,7 +249,7 @@ def testServerSelfConflict3(t, env):
> >       That should succeed.  Then do a write open from a different client,
> >       and verify that it breaks the delegation.
> > -    FLAGS: deleg
> > +    FLAGS: deleg all
> >       CODE: DELEG23
> >       """
> > @@ -357,7 +357,7 @@ def testCbGetattrNoChange(t, env):
> >       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
> > +    FLAGS: deleg all
> >       CODE: DELEG24
> >       """
> >       attrs1, attrs2 = _testCbGetattr(t, env)
> > @@ -376,7 +376,7 @@ def testCbGetattrWithChange(t, env):
> >       attrs before sending them back to the server. Test that the 
> > second client
> >       sees different attrs than the original one.
> > -    FLAGS: deleg
> > +    FLAGS: deleg all
> >       CODE: DELEG25
> >       """
> >       attrs1, attrs2 = _testCbGetattr(t, env, change=1, size=5)
> > diff --git a/nfs4.1/server41tests/st_xattr.py b/nfs4.1/server41tests/ 
> > st_xattr.py
> > index 
> > b3eb8a87465b9fd76121e846f9927bfc0867ffc8..f67df9517bdbac0ebd88c0c9f94244a96d5d2d3e 100644
> > --- a/nfs4.1/server41tests/st_xattr.py
> > +++ b/nfs4.1/server41tests/st_xattr.py
> > @@ -15,7 +15,7 @@ current_stateid = stateid4(1, b'\0' * 12)
> >   def testGetXattrAttribute(t, env):
> >       """Server with xattr support MUST support.
> > -    FLAGS: xattr
> > +    FLAGS: xattr all
> >       CODE: XATT1
> >       VERS: 2-
> >       """
> > @@ -37,7 +37,7 @@ def testGetXattrAttribute(t, env):
> >   def testGetMissingAttr(t, env):
> >       """Server MUST return NFS4ERR_NOXATTR if value is missing.
> > -    FLAGS: xattr
> > +    FLAGS: xattr all
> >       CODE: XATT2
> >       VERS: 2-
> >       """
> > @@ -53,7 +53,7 @@ def testGetMissingAttr(t, env):
> >   def testCreateNewAttr(t, env):
> >       """Server MUST return NFS4_ON on create.
> > -    FLAGS: xattr
> > +    FLAGS: xattr all
> >       CODE: XATT3
> >       VERS: 2-
> >       """
> > @@ -76,7 +76,7 @@ def testCreateNewAttr(t, env):
> >   def testCreateNewIfMissingAttr(t, env):
> >       """Server MUST update existing attribute with SETXATTR4_EITHER.
> > -    FLAGS: xattr
> > +    FLAGS: xattr all
> >       CODE: XATT4
> >       VERS: 2-
> >       """
> > @@ -99,7 +99,7 @@ def testCreateNewIfMissingAttr(t, env):
> >   def testUpdateOfMissingAttr(t, env):
> >       """Server MUST return NFS4ERR_NOXATTR on update of missing 
> > attribute.
> > -    FLAGS: xattr
> > +    FLAGS: xattr all
> >       CODE: XATT5
> >       VERS: 2-
> >       """
> > @@ -117,7 +117,7 @@ def testUpdateOfMissingAttr(t, env):
> >   def testExclusiveCreateAttr(t, env):
> >       """Server MUST return NFS4ERR_EXIST on create of existing 
> > attribute.
> > -    FLAGS: xattr
> > +    FLAGS: xattr all
> >       CODE: XATT6
> >       VERS: 2-
> >       """
> > @@ -138,7 +138,7 @@ def testExclusiveCreateAttr(t, env):
> >   def testUpdateExistingAttr(t, env):
> >       """Server MUST return NFS4_ON on update of existing attribute.
> > -    FLAGS: xattr
> > +    FLAGS: xattr all
> >       CODE: XATT7
> >       VERS: 2-
> >       """
> > @@ -165,7 +165,7 @@ def testUpdateExistingAttr(t, env):
> >   def testRemoveNonExistingAttr(t, env):
> >       """Server MUST return NFS4ERR_NOXATTR on remove of non existing 
> > attribute.
> > -    FLAGS: xattr
> > +    FLAGS: xattr all
> >       CODE: XATT8
> >       VERS: 2-
> >       """
> > @@ -183,7 +183,7 @@ def testRemoveNonExistingAttr(t, env):
> >   def testRemoveExistingAttr(t, env):
> >       """Server MUST return NFS4_ON on remove of existing attribute.
> > -    FLAGS: xattr
> > +    FLAGS: xattr all
> >       CODE: XATT9
> >       VERS: 2-
> >       """
> > @@ -204,7 +204,7 @@ def testRemoveExistingAttr(t, env):
> >   def testListNoAttrs(t, env):
> >       """Server MUST return NFS4_ON an empty list if no attributes 
> > defined.
> > -    FLAGS: xattr
> > +    FLAGS: xattr all
> >       CODE: XATT10
> >       VERS: 2-
> >       """
> > @@ -227,7 +227,7 @@ def testListNoAttrs(t, env):
> >   def testListAttrs(t, env):
> >       """Server MUST return NFS4_ON and list of defined attributes.
> > -    FLAGS: xattr
> > +    FLAGS: xattr all
> >       CODE: XATT11
> >       VERS: 2-
> >       """
> >
> > ---
> > base-commit: 81a4693305abb42ffd16e77a4808a1a607693476
> > change-id: 20250220-fixes-4bb1039117da
> >
> > Best regards,