[PATCH] pynfs: Fix bitmask check and handle NFS4ERR_DELAY in CB_GETATTR

[PATCH] pynfs: Fix bitmask check and handle NFS4ERR_DELAY in CB_GETATTR

[Suhas Athani]

From: Suhas Athani <Suhas.Athani@ibm.com>

This patch fixes logic errors in the _testCbGetattr helper
function (used by DELEG24 and DELEG25).

Changes:
- Fix Open Arguments Mask: The test previously used
OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS directly
as a mask. It is now correctly shifted (1 << Constant) to check
the capability bit properly.
- Handle NFS4ERR_DELAY: Added a retry loop for the GETATTR
operation. The test now retries instead of failing immediately.
- Fix Callback Verification: Updated the completion check to
verify cb.is_set() at the end of the test. Previously, the test
tracked completion via a variable captured before the retry loop,
which could lead to false failures if the callback arrived during
a retry.

Signed-off-by: Suhas Athani <Suhas.Athani@ibm.com>
---
 nfs4.1/server41tests/st_delegation.py | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/nfs4.1/server41tests/st_delegation.py b/nfs4.1/server41tests/st_delegation.py
index f27e852..ef8fc2a 100644
--- a/nfs4.1/server41tests/st_delegation.py
+++ b/nfs4.1/server41tests/st_delegation.py
@@ -1,6 +1,7 @@
 from .st_create_session import create_session
 from .st_open import open_claim4
 from xdrdef.nfs4_const import *
+import time
 
 from .environment import check, fail, create_file, open_file, close_file, do_getattrdict
 from xdrdef.nfs4_type import *
@@ -315,7 +316,7 @@ def _testCbGetattr(t, env, change=0, size=0):
         fail("FATTR4_OPEN_ARGUMENTS not supported")
 
     if caps[FATTR4_SUPPORTED_ATTRS] & FATTR4_OPEN_ARGUMENTS:
-        if caps[FATTR4_OPEN_ARGUMENTS].oa_share_access_want & OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS:
+        if caps[FATTR4_OPEN_ARGUMENTS].oa_share_access_want & (1<<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)
@@ -344,12 +345,26 @@ def _testCbGetattr(t, env, change=0, size=0):
                                                           1<<FATTR4_TIME_ACCESS | 1<<FATTR4_TIME_MODIFY)])
 
     # wait for the CB_GETATTR
-    completed = cb.wait(2)
+    cb.wait(2)
     res = sess2.listen(slot)
+
+    # Handle NFS4ERR_DELAY - retry until we get NFS4_OK
+    retry_count = 0
+    max_retries = 5
+    while res.status == NFS4ERR_DELAY and retry_count < max_retries:
+        time.sleep(0.1)
+        res = sess2.compound([op.putfh(fh), op.getattr(1<<FATTR4_CHANGE | 1<<FATTR4_SIZE |
+            1<<FATTR4_TIME_ACCESS | 1<<FATTR4_TIME_MODIFY)])
+        retry_count += 1
+
+    if res.status == NFS4ERR_DELAY:
+        fail(f"GETATTR still returning DELAY after {max_retries} retries")
+
     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:
+    # Only expect NFS4_OK now since we handle DELAY
+    check(res, [NFS4_OK])
+    if not cb.is_set():
         fail("CB_GETATTR not received")
     return attrs1, attrs2
 
-- 
2.43.5

Re: [PATCH] pynfs: Fix bitmask check and handle NFS4ERR_DELAY in CB_GETATTR

[Jeff Layton]

On Wed, 2025-12-17 at 15:39 +0530, Suhas Athani wrote:
> From: Suhas Athani <Suhas.Athani@ibm.com>
> 
> This patch fixes logic errors in the _testCbGetattr helper
> function (used by DELEG24 and DELEG25).
> 
> Changes:
> - Fix Open Arguments Mask: The test previously used
> OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS directly
> as a mask. It is now correctly shifted (1 << Constant) to check
> the capability bit properly.
> - Handle NFS4ERR_DELAY: Added a retry loop for the GETATTR
> operation. The test now retries instead of failing immediately.
> - Fix Callback Verification: Updated the completion check to
> verify cb.is_set() at the end of the test. Previously, the test
> tracked completion via a variable captured before the retry loop,
> which could lead to false failures if the callback arrived during
> a retry.
> 

This all looks fine, but it's generally preferred to have one patch per
issue. I'd suggest splitting this into 3 patches, since you're fixing 3
problems.

> Signed-off-by: Suhas Athani <Suhas.Athani@ibm.com>
> ---
>  nfs4.1/server41tests/st_delegation.py | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/nfs4.1/server41tests/st_delegation.py b/nfs4.1/server41tests/st_delegation.py
> index f27e852..ef8fc2a 100644
> --- a/nfs4.1/server41tests/st_delegation.py
> +++ b/nfs4.1/server41tests/st_delegation.py
> @@ -1,6 +1,7 @@
>  from .st_create_session import create_session
>  from .st_open import open_claim4
>  from xdrdef.nfs4_const import *
> +import time
>  
>  from .environment import check, fail, create_file, open_file, close_file, do_getattrdict
>  from xdrdef.nfs4_type import *
> @@ -315,7 +316,7 @@ def _testCbGetattr(t, env, change=0, size=0):
>          fail("FATTR4_OPEN_ARGUMENTS not supported")
>  
>      if caps[FATTR4_SUPPORTED_ATTRS] & FATTR4_OPEN_ARGUMENTS:
> -        if caps[FATTR4_OPEN_ARGUMENTS].oa_share_access_want & OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS:
> +        if caps[FATTR4_OPEN_ARGUMENTS].oa_share_access_want & (1<<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)
> @@ -344,12 +345,26 @@ def _testCbGetattr(t, env, change=0, size=0):
>                                                            1<<FATTR4_TIME_ACCESS | 1<<FATTR4_TIME_MODIFY)])
>  
>      # wait for the CB_GETATTR
> -    completed = cb.wait(2)
> +    cb.wait(2)
>      res = sess2.listen(slot)
> +
> +    # Handle NFS4ERR_DELAY - retry until we get NFS4_OK
> +    retry_count = 0
> +    max_retries = 5
> +    while res.status == NFS4ERR_DELAY and retry_count < max_retries:
> +        time.sleep(0.1)
> +        res = sess2.compound([op.putfh(fh), op.getattr(1<<FATTR4_CHANGE | 1<<FATTR4_SIZE |
> +            1<<FATTR4_TIME_ACCESS | 1<<FATTR4_TIME_MODIFY)])
> +        retry_count += 1
> +
> +    if res.status == NFS4ERR_DELAY:
> +        fail(f"GETATTR still returning DELAY after {max_retries} retries")
> +
>      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:
> +    # Only expect NFS4_OK now since we handle DELAY
> +    check(res, [NFS4_OK])
> +    if not cb.is_set():
>          fail("CB_GETATTR not received")
>      return attrs1, attrs2
>  


-- 
Jeff Layton <jlayton@kernel.org>