2022-08-12 14:12:46

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH] DELEG5: Create test file with mode o666

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.

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.

Signed-off-by: Chuck Lever <[email protected]>
---
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,



2022-08-12 19:23:40

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] DELEG5: Create test file with mode o666



> On Aug 12, 2022, at 10:08 AM, Chuck Lever <[email protected]> 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 <[email protected]>
> ---
> 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



2022-08-14 15:16:42

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH] DELEG5: Create test file with mode o666

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