2020-12-17 00:37:22

by Thomas Haynes

Subject: [pynfs 03/10] Close the file for SEQ10b

Signed-off-by: Tom Haynes <[email protected]>
---
nfs4.1/server41tests/st_sequence.py | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/nfs4.1/server41tests/st_sequence.py b/nfs4.1/server41tests/st_sequence.py
index 9be1096..969deb8 100644
--- a/nfs4.1/server41tests/st_sequence.py
+++ b/nfs4.1/server41tests/st_sequence.py
@@ -225,6 +225,7 @@ def testReplayCache007(t, env):
check(res)
fh = res.resarray[-1].object
stateid = res.resarray[-2].stateid
+
ops = env.home + [op.savefh(),\
op.rename(b"%s_1" % env.testname(t), b"%s_2" % env.testname(t))]
res1 = sess1.compound(ops, cache_this=False)
@@ -233,6 +234,10 @@ def testReplayCache007(t, env):
check(res2, [NFS4_OK, NFS4ERR_RETRY_UNCACHED_REP])
close_file(sess1, fh, stateid=stateid)

+ # Cleanup
+ res = sess1.compound([op.putfh(fh), op.close(0, stateid)])
+ check(res)
+
def testOpNotInSession(t, env):
"""Operations other than SEQUENCE, BIND_CONN_TO_SESSION, EXCHANGE_ID,
CREATE_SESSION, and DESTROY_SESSION, MUST NOT appear as the
--
2.26.2

2020-12-18 16:45:10

by J. Bruce Fields

Subject: Re: [pynfs 03/10] Close the file for SEQ10b

On Wed, Dec 16, 2020 at 04:35:09PM -0800, Tom Haynes wrote:
> Signed-off-by: Tom Haynes <[email protected]>
> ---
> nfs4.1/server41tests/st_sequence.py | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/nfs4.1/server41tests/st_sequence.py b/nfs4.1/server41tests/st_sequence.py
> index 9be1096..969deb8 100644
> --- a/nfs4.1/server41tests/st_sequence.py
> +++ b/nfs4.1/server41tests/st_sequence.py
> @@ -225,6 +225,7 @@ def testReplayCache007(t, env):
> check(res)
> fh = res.resarray[-1].object
> stateid = res.resarray[-2].stateid
> +
> ops = env.home + [op.savefh(),\
> op.rename(b"%s_1" % env.testname(t), b"%s_2" % env.testname(t))]
> res1 = sess1.compound(ops, cache_this=False)
> @@ -233,6 +234,10 @@ def testReplayCache007(t, env):
> check(res2, [NFS4_OK, NFS4ERR_RETRY_UNCACHED_REP])
> close_file(sess1, fh, stateid=stateid)
>
> + # Cleanup
> + res = sess1.compound([op.putfh(fh), op.close(0, stateid)])
> + check(res)
> +

This is giving me:

SEQ10b st_sequence.testReplayCache007 : FAILURE
OP_CLOSE should return NFS4_OK, instead got
NFS4ERR_BAD_STATEID

probably because the file was already closed just above. I'm not sure
whta was intended here. Reverting for now.

--b.

> def testOpNotInSession(t, env):
> """Operations other than SEQUENCE, BIND_CONN_TO_SESSION, EXCHANGE_ID,
> CREATE_SESSION, and DESTROY_SESSION, MUST NOT appear as the
> --
> 2.26.2

2020-12-21 02:58:44

by J. Bruce Fields

Subject: Re: [pynfs 03/10] Close the file for SEQ10b

On Sun, Dec 20, 2020 at 06:42:55PM -0800, Thomas Haynes wrote:
>
>
> > On Dec 18, 2020, at 8:43 AM, J. Bruce Fields <[email protected]> wrote:
> >
> > On Wed, Dec 16, 2020 at 04:35:09PM -0800, Tom Haynes wrote:
> >>
> >> close_file(sess1, fh, stateid=stateid)
> >>
> >> + # Cleanup
> >> + res = sess1.compound([op.putfh(fh), op.close(0, stateid)])
> >> + check(res)
> >> +
> >
> > This is giving me:
> >
> > SEQ10b st_sequence.testReplayCache007 : FAILURE
> > OP_CLOSE should return NFS4_OK, instead got
> > NFS4ERR_BAD_STATEID
> >
> > probably because the file was already closed just above. I'm not sure
> > whta was intended here. Reverting for now.
> >
> > —b
>
> Ahh, that close is not there in the branch we have internally. And since git took the change, I thought it was still good.
>
> Backing out is cool.

Oh, I see what happened, thanks for following up.

--b.

2020-12-21 05:08:26

by Thomas Haynes

Subject: Re: [pynfs 03/10] Close the file for SEQ10b

> On Dec 18, 2020, at 8:43 AM, J. Bruce Fields <[email protected]> wrote:
>
> On Wed, Dec 16, 2020 at 04:35:09PM -0800, Tom Haynes wrote:
>>
>> close_file(sess1, fh, stateid=stateid)
>>
>> + # Cleanup
>> + res = sess1.compound([op.putfh(fh), op.close(0, stateid)])
>> + check(res)
>> +
>
> This is giving me:
>
> SEQ10b st_sequence.testReplayCache007 : FAILURE
> OP_CLOSE should return NFS4_OK, instead got
> NFS4ERR_BAD_STATEID
>
> probably because the file was already closed just above. I'm not sure
> whta was intended here. Reverting for now.
>
> —b

Ahh, that close is not there in the branch we have internally. And since git took the change, I thought it was still good.

Backing out is cool.