Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 14 Aug 2022 10:56:18 -0400
To:     Chuck Lever III <chuck.lever@oracle.com>
Cc:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] DELEG5: Create test file with mode o666
Message-ID: <20220814145618.GA5578@fieldses.org>
References: <166031330113.3080139.1273532571655274363.stgit@morisot.1015granger.net>
 <F2DE58E9-E804-4F5E-9A9D-1EBEEAC9ECE5@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <F2DE58E9-E804-4F5E-9A9D-1EBEEAC9ECE5@oracle.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
From:   bfields@fieldses.org (J. Bruce Fields)
Precedence: bulk

Thanks, updated the changelog and applied.--b.

On Fri, Aug 12, 2022 at 07:12:42PM +0000, Chuck Lever III wrote:
> 
> 
> > On Aug 12, 2022, at 10:08 AM, Chuck Lever <chuck.lever@oracle.com> wrote:
> > 
> > DELEG5   st_delegation.testManyReaddeleg                          : FAILURE
> >           Open which causes recall should return NFS4_OK or
> >           NFS4ERR_DELAY, instead got NFS4ERR_ACCESS
> > ******* CB_Recall (id=14)********
> > **************************************************
> > Command line asked for 1 of 678 tests
> > Of those: 0 Skipped, 1 Failed, 0 Warned, 0 Passed
> > 
> > WARNING: could not clean testdir due to:
> > Making sure b'DELEG5-1' is writable: operation OP_SETATTR should return NFS4_OK, instead got NFS4ERR_DELAY
> > 
> > DELEG5 attempts an OPEN with ACCESS_WRITE to force the recall of
> > a bunch of delegations. The OPEN that requests ACCESS_WRITE fails,
> > however, because the test file was created as 0,0 with mode o644.
> 
> This paragraph could be more clear:
> 
> DELEG5 creates the test file as UID 0 with mode rw-r--r--
> 
> The later OPEN for Write (to trigger delegation recall) is sent as UID 1.
> This OPEN fails because mode rw-r--r-- does not permit UID 1 to open a file
> owned by UID 0 for write.
> 
> 
> > A perhaps more preferable long-term fix would be to follow the
> > advice of the nearby comment and convert DELEG5 to use _get_deleg(),
> > but I'm not expert enough to tackle that yet.
> 
> _get_deleg() creates its test file as UID0 with mode rw-rw-rw-,
> thus side-stepping the subsequent permission conflict. My proposed
> fix simply copies that behavior to DELEG5.
> 
> 
> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > ---
> > nfs4.0/servertests/st_delegation.py |    3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/nfs4.0/servertests/st_delegation.py b/nfs4.0/servertests/st_delegation.py
> > index 9c98ec0e0fb3..ba49cf9f09db 100644
> > --- a/nfs4.0/servertests/st_delegation.py
> > +++ b/nfs4.0/servertests/st_delegation.py
> > @@ -244,7 +244,8 @@ def testManyReaddeleg(t, env, funct=_recall, response=NFS4_OK):
> >     c.init_connection(b'pynfs%i_%s' % (os.getpid(), t.word()), cb_ident=0)
> >     cbids = []
> >     fh, stateid = c.create_confirm(t.word(), access=OPEN4_SHARE_ACCESS_READ,
> > -                                   deny=OPEN4_SHARE_DENY_NONE)
> > +                                   deny=OPEN4_SHARE_DENY_NONE,
> > +                                   attrs={FATTR4_MODE: 0o666})
> >     for i in range(count):
> >         c.init_connection(b'pynfs%i_%s_%i' % (os.getpid(), t.word(), i), cb_ident=0)
> >         fh, stateid = c.open_confirm(t.word(), access=OPEN4_SHARE_ACCESS_READ,
> > 
> > 
> 
> --
> Chuck Lever
> 
>