2023-03-13 11:24:09

by Jeffrey Layton

[permalink] [raw]
Subject: [pynfs PATCH v2 0/5] An assortment of pynfs patches

v2:
- instead of changing the meaning of the "all" flag, add a new
"everything" flag.
- add patch to fix LOCK24

I sent some of these a couple of weeks ago, but didn't Cc Calum (the new
maintainer) and a couple of them needed some changes. This set should
address the concerns about changing the meaning of "all" and adds a
new patch that fixes LOCK24 on knfsd.

Jeff Layton (5):
nfs4.0: add a retry loop on NFS4ERR_DELAY to compound function
examples: add a new example localhost_helper.sh script
nfs4.0/testserver.py: don't return an error when tests fail
testserver.py: add a new (special) "everything" flag
LOCK24: fix the lock_seqid in second lock request

examples/localhost_helper.sh | 29 +++++++++++++++++++++++++++++
nfs4.0/nfs4lib.py | 21 +++++++++++++++------
nfs4.0/servertests/st_lock.py | 6 +++++-
nfs4.0/testserver.py | 2 --
nfs4.1/testmod.py | 2 ++
5 files changed, 51 insertions(+), 9 deletions(-)
create mode 100755 examples/localhost_helper.sh

--
2.39.2



2023-03-13 11:24:10

by Jeffrey Layton

[permalink] [raw]
Subject: [pynfs PATCH v2 3/5] nfs4.0/testserver.py: don't return an error when tests fail

This script was originally changed in eb3ba0b60055 ("Have testserver.py
have non-zero exit code if any tests fail"), but the same change wasn't
made to the 4.1 testserver.py script.

There also wasn't much explanation for it, and it makes it difficult to
tell whether the test harness itself failed, or whether there was a
failure in a requested test.

Stop the 4.0 testserver.py from exiting with an error code when a test
fails, so that a successful return means only that the test harness
itself worked, not that every requested test passed.

Cc: Frank Filz <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
nfs4.0/testserver.py | 2 --
1 file changed, 2 deletions(-)

diff --git a/nfs4.0/testserver.py b/nfs4.0/testserver.py
index f2c41568e5c7..4f4286daa657 100755
--- a/nfs4.0/testserver.py
+++ b/nfs4.0/testserver.py
@@ -387,8 +387,6 @@ def main():

if nfail < 0:
sys.exit(3)
- if nfail > 0:
- sys.exit(2)

if __name__ == "__main__":
main()
--
2.39.2


2023-03-13 11:24:09

by Jeffrey Layton

[permalink] [raw]
Subject: [pynfs PATCH v2 2/5] examples: add a new example localhost_helper.sh script

It's possible (and pretty common) to run pynfs against the server
running on the same host. Add a new serverhelper script for that
purpose.

Signed-off-by: Jeff Layton <[email protected]>
---
examples/localhost_helper.sh | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
create mode 100755 examples/localhost_helper.sh

diff --git a/examples/localhost_helper.sh b/examples/localhost_helper.sh
new file mode 100755
index 000000000000..6db123311e7a
--- /dev/null
+++ b/examples/localhost_helper.sh
@@ -0,0 +1,29 @@
+#!/bin/bash
+#
+# serverhelper script for running tests against knfsd running on localhost.
+# Note that this requires that the running user can use sudo to restart nfsd
+# without a password.
+#
+
+# server argument is ignored here
+server=$1
+command=$2
+shift; shift
+
+case $command in
+reboot )
+ sudo systemctl restart nfs-server.service
+ ;;
+unlink )
+ rm $1
+ ;;
+rename )
+ mv $1 $2
+ ;;
+link )
+ ln $1 $2
+ ;;
+chmod )
+ chmod $1 $2
+ ;;
+esac
--
2.39.2


2023-03-13 11:24:12

by Jeffrey Layton

[permalink] [raw]
Subject: [pynfs PATCH v2 4/5] testserver.py: add a new (special) "everything" flag

The READMEs for v4.0 and v4.1 are inconsistent here. For v4.0, the "all"
flag is supposed to run all of the "standard" tests. For v4.1 "all" is
documented to run all of the tests, but it actually doesn't since not
every tests has "all" in its FLAGS: field.

I really want to be able to run _all_ the tests sometimes. Add a special
case new "everything" flag that is automatically added to every test.

Signed-off-by: Jeff Layton <[email protected]>
---
nfs4.1/testmod.py | 2 ++
1 file changed, 2 insertions(+)

diff --git a/nfs4.1/testmod.py b/nfs4.1/testmod.py
index 11e759d673fd..b3174006a0a3 100644
--- a/nfs4.1/testmod.py
+++ b/nfs4.1/testmod.py
@@ -386,6 +386,8 @@ def createtests(testdir):
for t in tests:
## if not t.flags_list:
## raise RuntimeError("%s has no flags" % t.fullname)
+ if "everything" not in t.flags_list:
+ t.flags_list.append("everything")
for f in t.flags_list:
if f not in flag_dict:
flag_dict[f] = bit
--
2.39.2


2023-03-13 11:24:12

by Jeffrey Layton

[permalink] [raw]
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 <[email protected]>
Fixes: 4299316fb357 (Add LOCK24 test case to test open uprgade/downgrade scenario)
Signed-off-by: Jeff Layton <[email protected]>
---
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


2023-03-13 16:40:12

by Calum Mackay

[permalink] [raw]
Subject: Re: [pynfs PATCH v2 0/5] An assortment of pynfs patches

On 13/03/2023 11:23 am, Jeff Layton wrote:
> v2:
> - instead of changing the meaning of the "all" flag, add a new
> "everything" flag.
> - add patch to fix LOCK24
>
> I sent some of these a couple of weeks ago, but didn't Cc Calum (the new
> maintainer) and a couple of them needed some changes. This set should
> address the concerns about changing the meaning of "all" and adds a
> new patch that fixes LOCK24 on knfsd.

Thanks very much, Jeff.

I don't have my repo set-up yet, but hopefully soon, and will get to
these (and others) asap.

cheers,
calum.

>
> Jeff Layton (5):
> nfs4.0: add a retry loop on NFS4ERR_DELAY to compound function
> examples: add a new example localhost_helper.sh script
> nfs4.0/testserver.py: don't return an error when tests fail
> testserver.py: add a new (special) "everything" flag
> LOCK24: fix the lock_seqid in second lock request
>
> examples/localhost_helper.sh | 29 +++++++++++++++++++++++++++++
> nfs4.0/nfs4lib.py | 21 +++++++++++++++------
> nfs4.0/servertests/st_lock.py | 6 +++++-
> nfs4.0/testserver.py | 2 --
> nfs4.1/testmod.py | 2 ++
> 5 files changed, 51 insertions(+), 9 deletions(-)
> create mode 100755 examples/localhost_helper.sh
>



Attachments:
OpenPGP_signature (840.00 B)
OpenPGP digital signature

2023-03-13 18:52:46

by Frank Filz

[permalink] [raw]
Subject: RE: [pynfs PATCH v2 5/5] LOCK24: fix the lock_seqid in second lock request

Looks good to me, tested against Ganesha and the updated patch passes.

Frank

> -----Original Message-----
> From: Jeff Layton [mailto:[email protected]]
> Sent: Monday, March 13, 2023 4:24 AM
> To: [email protected]
> Cc: [email protected]; [email protected];
[email protected];
> Frank Filz <[email protected]>
> 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 <[email protected]>
> Fixes: 4299316fb357 (Add LOCK24 test case to test open uprgade/downgrade
> scenario)
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> 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


2023-03-13 21:24:08

by Jeffrey Layton

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

Thanks for testing it, Frank.

FWIW, if the unmodified test still passes on ganesha then that's
probably an indicator that it's not doing adequate vetting of the lock
seqid with v4.0.

Cheers,
Jeff

On Mon, 2023-03-13 at 11:51 -0700, Frank Filz wrote:
> Looks good to me, tested against Ganesha and the updated patch passes.
>
> Frank
>
> > -----Original Message-----
> > From: Jeff Layton [mailto:[email protected]]
> > Sent: Monday, March 13, 2023 4:24 AM
> > To: [email protected]
> > Cc: [email protected]; [email protected];
> [email protected];
> > Frank Filz <[email protected]>
> > 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 <[email protected]>
> > Fixes: 4299316fb357 (Add LOCK24 test case to test open uprgade/downgrade
> > scenario)
> > Signed-off-by: Jeff Layton <[email protected]>
> > ---
> > 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
>

--
Jeff Layton <[email protected]>

2023-03-28 13:29:23

by Petr Vorel

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

Hi Jeff,

thanks for fixing a long standing issue.

Tested-by: Petr Vorel <[email protected]>
(on openSUSE and SLES)

Kind regards,
Petr

2023-04-13 18:41:17

by Calum Mackay

[permalink] [raw]
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?

cheers,
calum.


>
> Frank
>
>> -----Original Message-----
>> From: Jeff Layton [mailto:[email protected]]
>> Sent: Monday, March 13, 2023 4:24 AM
>> To: [email protected]
>> Cc: [email protected]; [email protected];
> [email protected];
>> Frank Filz <[email protected]>
>> 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 <[email protected]>
>> Fixes: 4299316fb357 (Add LOCK24 test case to test open uprgade/downgrade
>> scenario)
>> Signed-off-by: Jeff Layton <[email protected]>
>> ---
>> 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
>


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2023-04-14 14:51:57

by Frank Filz

[permalink] [raw]
Subject: RE: [pynfs PATCH v2 5/5] LOCK24: fix the lock_seqid in second lock request



> -----Original Message-----
> From: Calum Mackay [mailto:[email protected]]
> Sent: Thursday, April 13, 2023 11:35 AM
> To: Frank Filz <[email protected]>; 'Jeff Layton' <[email protected]>
> Cc: Calum Mackay <[email protected]>; [email protected]; linux-
> [email protected]; 'Frank Filz' <[email protected]>
> 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 <[email protected]>
>
> cheers,
> calum.
>
>
> >
> > Frank
> >
> >> -----Original Message-----
> >> From: Jeff Layton [mailto:[email protected]]
> >> Sent: Monday, March 13, 2023 4:24 AM
> >> To: [email protected]
> >> Cc: [email protected]; [email protected];
> > [email protected];
> >> Frank Filz <[email protected]>
> >> 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 <[email protected]>
> >> Fixes: 4299316fb357 (Add LOCK24 test case to test open
> >> uprgade/downgrade
> >> scenario)
> >> Signed-off-by: Jeff Layton <[email protected]>
> >> ---
> >> 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
> >


2023-04-14 17:26:47

by Calum Mackay

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

On 14/04/2023 3:41 pm, Frank Filz wrote:
>> -----Original Message-----
>> From: Calum Mackay [mailto:[email protected]]
>> Sent: Thursday, April 13, 2023 11:35 AM
>> To: Frank Filz <[email protected]>; 'Jeff Layton' <[email protected]>
>> Cc: Calum Mackay <[email protected]>; [email protected]; linux-
>> [email protected]; 'Frank Filz' <[email protected]>
>> 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 <[email protected]>

thanks very much, Frank.

cheers,
calum.

>>
>> cheers,
>> calum.
>>
>>
>>>
>>> Frank
>>>
>>>> -----Original Message-----
>>>> From: Jeff Layton [mailto:[email protected]]
>>>> Sent: Monday, March 13, 2023 4:24 AM
>>>> To: [email protected]
>>>> Cc: [email protected]; [email protected];
>>> [email protected];
>>>> Frank Filz <[email protected]>
>>>> 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 <[email protected]>
>>>> Fixes: 4299316fb357 (Add LOCK24 test case to test open
>>>> uprgade/downgrade
>>>> scenario)
>>>> Signed-off-by: Jeff Layton <[email protected]>
>>>> ---
>>>> 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
>>>
>
>


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature