Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CANkgwes_79iE9MGvzGu_tLjNVZprTjTMNzohADRU6pw4Z3xC_w@mail.gmail.com>
 <20220126141053.GA29832@fieldses.org> <0bc601d812c9$8b52b750$a1f825f0$@mindspring.com>
In-Reply-To: <0bc601d812c9$8b52b750$a1f825f0$@mindspring.com>
From:   Volodymyr Khomenko <volodymyr@vastdata.com>
Date:   Thu, 27 Jan 2022 10:10:03 +0200
Message-ID: <CANkgwev3YoQcQwnJ6vb0sXt8J0OWzQN+n-9=92azJ4dYEPO3oQ@mail.gmail.com>
Subject: Re: [PATCH] pynfs minor: fixed Environment._maketree to use proper
 stateid during file write
To:     Frank Filz <ffilzlnx@mindspring.com>
Cc:     "J. Bruce Fields" <bfields@fieldses.org>, linux-nfs@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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
> > >
>