| Message ID | CANkgwes_79iE9MGvzGu_tLjNVZprTjTMNzohADRU6pw4Z3xC_w@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pynfs minor: fixed Environment._maketree to use proper stateid during file write | expand |
On Fri, Jan 21, 2022 at 03:06:57PM +0200, Volodymyr Khomenko wrote: > > From 63c0711f9cd8f8c0aaff7d0116a42b5001bddcd2 Mon Sep 17 00:00:00 2001 > From: Volodymyr Khomenko <Khomenko.Volodymyr@gmail.com> > Date: Fri, 21 Jan 2022 14:52:28 +0200 > Subject: [PATCH] Minor: fixed Environment._maketree (used by init) to use > proper stateid during file write > > _maketree is a part of generic init sequence for server41tests so the code should be generic. > Using zero stateid (when "other" and "seqid" are both zero, the stateid is treated > as a special anonymous stateid) is a special use-case of anonymous access > so it must not be used during generic initialization. OK, applying, but I'm a little wary. If a server isn't accepting the zero stateid here then I think that's a server bug. --b. > > Signed-off-by: Volodymyr Khomenko <Khomenko.Volodymyr@gmail.com> > --- > nfs4.1/server41tests/environment.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/nfs4.1/server41tests/environment.py b/nfs4.1/server41tests/environment.py > index 14b0902..0b7c976 100644 > --- a/nfs4.1/server41tests/environment.py > +++ b/nfs4.1/server41tests/environment.py > @@ -198,7 +198,7 @@ class Environment(testmod.Environment): > log.warning("could not create /%s" % b'/'.join(path)) > # Make file-object in /tree > fh, stateid = create_confirm(sess, b'maketree', tree + [b'file']) > - res = write_file(sess, fh, self.filedata) > + res = write_file(sess, fh, self.filedata, stateid=stateid) > check(res, msg="Writing data to /%s/file" % b'/'.join(tree)) > res = close_file(sess, fh, stateid) > check(res) > -- > 2.25.1 >
> On Fri, Jan 21, 2022 at 03:06:57PM +0200, Volodymyr Khomenko wrote: > > > > > From 63c0711f9cd8f8c0aaff7d0116a42b5001bddcd2 Mon Sep 17 00:00:00 > 2001 > > From: Volodymyr Khomenko <Khomenko.Volodymyr@gmail.com> > > Date: Fri, 21 Jan 2022 14:52:28 +0200 > > Subject: [PATCH] Minor: fixed Environment._maketree (used by init) to > > use proper stateid during file write > > > > _maketree is a part of generic init sequence for server41tests so the code > should be generic. > > Using zero stateid (when "other" and "seqid" are both zero, the > > stateid is treated as a special anonymous stateid) is a special > > use-case of anonymous access so it must not be used during generic > initialization. > > OK, applying, but I'm a little wary. If a server isn't accepting the zero stateid > here then I think that's a server bug. Yea, that makes me nervous about a server bug also. Maybe we should have explicit special stateid tests. It's always tricky because initialization of the tree requires a bunch of stuff to work before it's explicitly tested... Frank > > Signed-off-by: Volodymyr Khomenko <Khomenko.Volodymyr@gmail.com> > > --- > > nfs4.1/server41tests/environment.py | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/nfs4.1/server41tests/environment.py > > b/nfs4.1/server41tests/environment.py > > index 14b0902..0b7c976 100644 > > --- a/nfs4.1/server41tests/environment.py > > +++ b/nfs4.1/server41tests/environment.py > > @@ -198,7 +198,7 @@ class Environment(testmod.Environment): > > log.warning("could not create /%s" % b'/'.join(path)) > > # Make file-object in /tree > > fh, stateid = create_confirm(sess, b'maketree', tree + [b'file']) > > - res = write_file(sess, fh, self.filedata) > > + res = write_file(sess, fh, self.filedata, stateid=stateid) > > check(res, msg="Writing data to /%s/file" % b'/'.join(tree)) > > res = close_file(sess, fh, stateid) > > check(res) > > -- > > 2.25.1 > >
Yes, I agree with you, accepting zero stateid is very important for the server. However, our NFS4 server implementation is still in the early stages of development so we intentionally have left this feature outside for now. So our intention is to run pynfs tests for other features we have already implemented. And yes, I would propose to make a dedicated test for a special stateid feature (if it's not done yet). From the protocol side, all-zeros and all-ones are special values for stateid. Thanks! Volodymyr. On Wed, Jan 26, 2022 at 5:29 PM Frank Filz <ffilzlnx@mindspring.com> wrote: > > > On Fri, Jan 21, 2022 at 03:06:57PM +0200, Volodymyr Khomenko wrote: > > > > > > > > From 63c0711f9cd8f8c0aaff7d0116a42b5001bddcd2 Mon Sep 17 00:00:00 > > 2001 > > > From: Volodymyr Khomenko <Khomenko.Volodymyr@gmail.com> > > > Date: Fri, 21 Jan 2022 14:52:28 +0200 > > > Subject: [PATCH] Minor: fixed Environment._maketree (used by init) to > > > use proper stateid during file write > > > > > > _maketree is a part of generic init sequence for server41tests so the > code > > should be generic. > > > Using zero stateid (when "other" and "seqid" are both zero, the > > > stateid is treated as a special anonymous stateid) is a special > > > use-case of anonymous access so it must not be used during generic > > initialization. > > > > OK, applying, but I'm a little wary. If a server isn't accepting the zero > stateid > > here then I think that's a server bug. > > Yea, that makes me nervous about a server bug also. Maybe we should have > explicit special stateid tests. > > It's always tricky because initialization of the tree requires a bunch of > stuff to work before it's explicitly tested... > > Frank > > > > Signed-off-by: Volodymyr Khomenko <Khomenko.Volodymyr@gmail.com> > > > --- > > > nfs4.1/server41tests/environment.py | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/nfs4.1/server41tests/environment.py > > > b/nfs4.1/server41tests/environment.py > > > index 14b0902..0b7c976 100644 > > > --- a/nfs4.1/server41tests/environment.py > > > +++ b/nfs4.1/server41tests/environment.py > > > @@ -198,7 +198,7 @@ class Environment(testmod.Environment): > > > log.warning("could not create /%s" % b'/'.join(path)) > > > # Make file-object in /tree > > > fh, stateid = create_confirm(sess, b'maketree', tree + > [b'file']) > > > - res = write_file(sess, fh, self.filedata) > > > + res = write_file(sess, fh, self.filedata, stateid=stateid) > > > check(res, msg="Writing data to /%s/file" % b'/'.join(tree)) > > > res = close_file(sess, fh, stateid) > > > check(res) > > > -- > > > 2.25.1 > > > >
From 63c0711f9cd8f8c0aaff7d0116a42b5001bddcd2 Mon Sep 17 00:00:00 2001 From: Volodymyr Khomenko <Khomenko.Volodymyr@gmail.com> Date: Fri, 21 Jan 2022 14:52:28 +0200 Subject: [PATCH] Minor: fixed Environment._maketree (used by init) to use proper stateid during file write _maketree is a part of generic init sequence for server41tests so the code should be generic. Using zero stateid (when "other" and "seqid" are both zero, the stateid is treated as a special anonymous stateid) is a special use-case of anonymous access so it must not be used during generic initialization. Signed-off-by: Volodymyr Khomenko <Khomenko.Volodymyr@gmail.com> --- nfs4.1/server41tests/environment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nfs4.1/server41tests/environment.py b/nfs4.1/server41tests/environment.py index 14b0902..0b7c976 100644 --- a/nfs4.1/server41tests/environment.py +++ b/nfs4.1/server41tests/environment.py @@ -198,7 +198,7 @@ class Environment(testmod.Environment): log.warning("could not create /%s" % b'/'.join(path)) # Make file-object in /tree fh, stateid = create_confirm(sess, b'maketree', tree + [b'file']) - res = write_file(sess, fh, self.filedata) + res = write_file(sess, fh, self.filedata, stateid=stateid) check(res, msg="Writing data to /%s/file" % b'/'.join(tree)) res = close_file(sess, fh, stateid) check(res) -- 2.25.1