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 <[email protected]>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
> > 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 <[email protected]>
> > ---
> > 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 <[email protected]> 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 <[email protected]>
> > > 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 <[email protected]>
> > > ---
> > > 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
> > >
>