Re: [PATCH] pynfs: courtesy: send RECLAIM_COMPLETE before session2 opening the file

[Calum Mackay <calum.mackay@oracle.com>]

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

On 10/06/2021 2:01 am, suy.fnst@fujitsu.com wrote:
> The test fails on v5.13-rc5 and old kernels. Because the second session
> doesn't send RECLAIM_COMPLETE before attempting to do a non-reclaim
> open. So the server returns NFS4ERR_GRACE instead of NFS4_OK.
> 
>      # ./testserver.py ${server_IP}:/nfsroot --rundeps COUR2
>      INFO   :rpc.poll:got connection from ('127.0.0.1', 39206), assigned to
>      fd=5
>      INFO   :rpc.thread:Called connect(('193.168.140.239', 2049))
>      INFO   :rpc.poll:Adding 6 generated by another thread
>      INFO   :test.env:Created client to 193.168.140.239, 2049
>      INFO   :test.env:Called do_readdir()
>      INFO   :test.env:do_readdir() = [entry4(cookie=512,
>      name=b'COUR2_1623055313', attrs={})]
>      fileb'COUR2_1623119443'created by sess1
>      INFO   :test.env:Sleeping for 22 seconds: twice the lease period
>      INFO   :test.env:Woke up
>      session created
>      **************************************************
>      COUR2    st_courtesy.testLockSleepLock                            :
>      FAILURE
>             OP_OPEN should return NFS4_OK, instead got
>                       NFS4ERR_GRACE
>      **************************************************
>      Command line asked for 1 of 255 tests
>        Of those: 0 Skipped, 1 Failed, 0 Warned, 0 Passed
> 
> RFC5661, page 567:
> "Whenever a client establishes a new client ID and before it does the
> first non-reclaim operation that obtains a lock, it MUST send a
> RECLAIM_COMPLETE with rca_one_fs set to FALSE, even if there are no
> locks to reclaim. If non-reclaim locking operations are done before
> the RECLAIM_COMPLETE, an NFS4ERR_GRACE error will be returned."
> 
> Send RECLAIM_COMPLETE before the file open to let the test pass.
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>   nfs4.1/server41tests/st_courtesy.py | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/nfs4.1/server41tests/st_courtesy.py b/nfs4.1/server41tests/st_courtesy.py
> index dd911a37772d..3478a9d93dbf 100644
> --- a/nfs4.1/server41tests/st_courtesy.py
> +++ b/nfs4.1/server41tests/st_courtesy.py
> @@ -74,6 +74,9 @@ def testLockSleepLock(t, env):
>       c2 = env.c1.new_client(b"%s_2" % env.testname(t))
>       sess2 = c2.create_session()
>   
> +    res = sess2.compound([op.reclaim_complete(FALSE)])
> +    check(res)
> +
>       res = open_file(sess2, env.testname(t), access=OPEN4_SHARE_ACCESS_WRITE)
>       check(res)
>   
> 

I'd still like to check whether this is the right place to fix this.

Initially, I was confused as to why the first client "c1" doesn't face 
the same issue. A network trace shows that a RECLAIM_COMPLETE is indeed 
sent for c1, despite not appearing explicitly in testLockSleepLock(). 
Whereas one isn't sent for c2, hence the problem.

This is probably because c1 is initialised with:

   61     sess1 = env.c1.new_client_session(env.testname(t))


and c2 with:

   74     c2 = env.c1.new_client(b"%s_2" % env.testname(t))
   75     sess2 = c2.create_session()


The c1 case results in a RECLAIM_COMPLETE, but the c2 case does not.

I'm not yet sure whether that ought to be done in 
new_client()/create_session(). If so, then there would be no need to add 
it explicitly here.


[I suspect this was missed in my testing, since the Solaris server I 
used may be less strict about requiring the RECLAIM_COMPLETE]


I'll look into this more and report asap.

thanks again,
calum.

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