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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 15/06/2021 3:47 pm, J. Bruce Fields wrote:
On Mon, Jun 14, 2021 at 09:50:34PM +0100, Calum Mackay wrote:
On 10/06/2021 2:01 am, suy.fnst@xxxxxxxxxxx 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@xxxxxxxxxxxxxx>
---
  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.

There's definitely cases where clients want to be able to create a new
session without sending a new RECLAIM_COMPLETE.

thanks Bruce; yes, I wondered.


Any reason we can't replace those two lines by a single
new_client_session()?

I wasn't quite sure on the semantics of those calls.

We want what appears to the server to be a new client c2, not a new session from an existing client c1. I wasn't sure whether new_client_session() would give us that?

Put alternatively, what is the difference between new_client_session and new_client/new_session? Is it just a different granularity, and to give the option e.g. of not using RECLAIM_COMPLETE?


I'll have a further look…

thanks very much,
calum.



I'd do either that or just add the explicit
RECLAIM_COMPLETE.

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

That's a server bug:

	https://datatracker.ietf.org/doc/html/rfc5661#page-173

	... NFS4ERR_GRACE must always be returned to clients attempting
	a non-reclaim lock request before doing their own global
	RECLAIM_COMPLETE.

I've complained about it before.  I had some idea it'd been fixed, maybe
not.

--b.

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux