> -----Original Message----- > From: Calum Mackay [mailto:calum.mackay@xxxxxxxxxx] > Sent: Thursday, April 13, 2023 11:35 AM > To: Frank Filz <ffilzlnx@xxxxxxxxxxxxxx>; 'Jeff Layton' <jlayton@xxxxxxxxxx> > Cc: Calum Mackay <calum.mackay@xxxxxxxxxx>; bfields@xxxxxxxxxxxx; linux- > nfs@xxxxxxxxxxxxxxx; 'Frank Filz' <ffilz@xxxxxxxxxx> > Subject: Re: [pynfs PATCH v2 5/5] LOCK24: fix the lock_seqid in second lock > request > > 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? Yes, definitely. Frank Tested-by: Frank Filz <ffilzlnx@xxxxxxxxxxxxxx> > > cheers, > calum. > > > > > > Frank > > > >> -----Original Message----- > >> From: Jeff Layton [mailto:jlayton@xxxxxxxxxx] > >> Sent: Monday, March 13, 2023 4:24 AM > >> To: calum.mackay@xxxxxxxxxx > >> Cc: bfields@xxxxxxxxxxxx; ffilzlnx@xxxxxxxxxxxxxx; > > linux-nfs@xxxxxxxxxxxxxxx; > >> Frank Filz <ffilz@xxxxxxxxxx> > >> 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@xxxxxxxxxx> > >> Fixes: 4299316fb357 (Add LOCK24 test case to test open > >> uprgade/downgrade > >> scenario) > >> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > >> --- > >> 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 > >
