Re: [pynfs PATCH v2 5/5] LOCK24: fix the lock_seqid in second lock request

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

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

Now that I have some repo space (thank you Trond), I am putting things 
together…

On 13/03/2023 6:51 pm, Frank Filz wrote:
> Looks good to me, tested against Ganesha and the updated patch passes.

Frank, may I add your Tested-by:, for 5/5 please?

cheers,
calum.


> 
> Frank
> 
>> -----Original Message-----
>> From: Jeff Layton [mailto:jlayton@kernel.org]
>> Sent: Monday, March 13, 2023 4:24 AM
>> To: calum.mackay@oracle.com
>> Cc: bfields@fieldses.org; ffilzlnx@mindspring.com;
> linux-nfs@vger.kernel.org;
>> Frank Filz <ffilz@redhat.com>
>> Subject: [pynfs PATCH v2 5/5] LOCK24: fix the lock_seqid in second lock
> request
>>
>> This test currently fails against Linux nfsd, but I think it's the test
> that's wrong. It
>> basically does:
>>
>> open for read
>> read lock
>> unlock
>> open upgrade
>> write lock
>>
>> The write lock above is sent with a lock_seqid of 0, which is wrong.
>> RFC7530/16.10.5 says:
>>
>>     o  In the case in which the state has been created and the [new
>>        lockowner] boolean is true, the server rejects the request with the
>>        error NFS4ERR_BAD_SEQID.  The only exception is where there is a
>>        retransmission of a previous request in which the boolean was
>>        true.  In this case, the lock_seqid will match the original
>>        request, and the response will reflect the final case, below.
>>
>> Since the above is not a retransmission, knfsd is correct to reject this
> call. This
>> patch fixes the open_sequence object to track the lock seqid and set it
> correctly
>> in the LOCK request.
>>
>> With this, LOCK24 passes against knfsd.
>>
>> Cc: Frank Filz <ffilz@redhat.com>
>> Fixes: 4299316fb357 (Add LOCK24 test case to test open uprgade/downgrade
>> scenario)
>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>> ---
>>   nfs4.0/servertests/st_lock.py | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/nfs4.0/servertests/st_lock.py b/nfs4.0/servertests/st_lock.py
> index
>> 468672403ffe..9d650ab017b9 100644
>> --- a/nfs4.0/servertests/st_lock.py
>> +++ b/nfs4.0/servertests/st_lock.py
>> @@ -886,6 +886,7 @@ class open_sequence:
>>           self.client = client
>>           self.owner = owner
>>           self.lockowner = lockowner
>> +        self.lockseqid = 0
>>       def open(self, access):
>>           self.fh, self.stateid = self.client.create_confirm(self.owner,
>>   						access=access,
>> @@ -900,14 +901,17 @@ class open_sequence:
>>           self.client.close_file(self.owner, self.fh, self.stateid)
>>       def lock(self, type):
>>           res = self.client.lock_file(self.owner, self.fh, self.stateid,
>> -                    type=type, lockowner=self.lockowner)
>> +                                    type=type, lockowner=self.lockowner,
>> +                                    lockseqid=self.lockseqid)
>>           check(res)
>>           if res.status == NFS4_OK:
>>               self.lockstateid = res.lockid
>> +            self.lockseqid += 1
>>       def unlock(self):
>>           res = self.client.unlock_file(1, self.fh, self.lockstateid)
>>           if res.status == NFS4_OK:
>>               self.lockstateid = res.lockid
>> +            self.lockseqid += 1
>>
>>   def testOpenUpgradeLock(t, env):
>>       """Try open, lock, open, downgrade, close
>> --
>> 2.39.2
> 


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