diff mbox series

[v2] generic/126: run it inside its own subdirectory

Message ID 20250219-generic126-v2-1-e7537f6c9607@kernel.org (mailing list archive)
State New
Headers show
Series [v2] generic/126: run it inside its own subdirectory | expand

Commit Message

Jeff Layton Feb. 19, 2025, 5:57 p.m. UTC 
I had been seeing some failures in generic/126 when running on NFS under
kdevops.

The problem turned out to be that kdevops makes the root of the NFS
export mode 01777 by default (i.e. with the sticky bit set). This causes
some of these permission tests to fail because the open() is called with
the O_CREAT bit set.

Make the outcome of this test not depend on the permissions on
$TEST_DIR. Create a new directory to do these tests in, and clean it up
when done.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v2:
- add commands to _cleanup() to clean up test directory (Thanks Darrick!)
- Link to v1: https://lore.kernel.org/r/20250219-generic126-v1-1-018e4e60c811@kernel.org
---
 tests/generic/126 | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)


---
base-commit: 8467552f09e1672a02712653b532a84bd46ea10e
change-id: 20250219-generic126-f81b3022c4b7

Best regards,

Comments

Darrick J. Wong Feb. 19, 2025, 6:06 p.m. UTC | #1 
On Wed, Feb 19, 2025 at 12:57:55PM -0500, Jeff Layton wrote:
> I had been seeing some failures in generic/126 when running on NFS under
> kdevops.
> 
> The problem turned out to be that kdevops makes the root of the NFS
> export mode 01777 by default (i.e. with the sticky bit set). This causes
> some of these permission tests to fail because the open() is called with
> the O_CREAT bit set.
> 
> Make the outcome of this test not depend on the permissions on
> $TEST_DIR. Create a new directory to do these tests in, and clean it up
> when done.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Looks good to me, thanks for the _cleanup fix!

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
> Changes in v2:
> - add commands to _cleanup() to clean up test directory (Thanks Darrick!)
> - Link to v1: https://lore.kernel.org/r/20250219-generic126-v1-1-018e4e60c811@kernel.org
> ---
>  tests/generic/126 | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/generic/126 b/tests/generic/126
> index 142d2bb97db706bafd2152856ef0993c5c273441..1f17492ed2a395ed3798c71243f3098558a157dc 100755
> --- a/tests/generic/126
> +++ b/tests/generic/126
> @@ -13,6 +13,7 @@ _begin_fstest perms auto quick
>  _cleanup()
>  {
>      cd /
> +    test -n "$testdir" && rm -rf "$testdir"
>  }
>  
>  # Import common functions.
> @@ -23,7 +24,13 @@ _require_chown
>  
>  QA_FS_PERMS=$here/src/fs_perms
>  
> -cd $TEST_DIR
> +testdir="$TEST_DIR/generic-126.$$"
> +
> +mkdir $testdir
> +chown 0:0 $testdir
> +chmod 0755 $testdir
> +cd $testdir
> +
>  cp $here/src/testx ./testx.file
>  
>  # file_perm owner_uid owner_gid tester_uid tester_gid perm_to_test expected_result pass=1
> @@ -46,7 +53,8 @@ $QA_FS_PERMS 200 99 99 200 99 w 1
>  $QA_FS_PERMS 040 99 99 99 500 r 1
>  $QA_FS_PERMS 400 99 99 200 99 r 1
>  
> -rm -f ./testx.file
> +cd /
> +rm -rf $testdir
>  
>  status=0
>  exit
> 
> ---
> base-commit: 8467552f09e1672a02712653b532a84bd46ea10e
> change-id: 20250219-generic126-f81b3022c4b7
> 
> Best regards,
> -- 
> Jeff Layton <jlayton@kernel.org>
> 
>
Zorro Lang Feb. 20, 2025, 7:12 a.m. UTC | #2 
On Wed, Feb 19, 2025 at 12:57:55PM -0500, Jeff Layton wrote:
> I had been seeing some failures in generic/126 when running on NFS under
> kdevops.
> 
> The problem turned out to be that kdevops makes the root of the NFS
> export mode 01777 by default (i.e. with the sticky bit set). This causes
> some of these permission tests to fail because the open() is called with
> the O_CREAT bit set.
> 
> Make the outcome of this test not depend on the permissions on
> $TEST_DIR. Create a new directory to do these tests in, and clean it up
> when done.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---



> Changes in v2:
> - add commands to _cleanup() to clean up test directory (Thanks Darrick!)
> - Link to v1: https://lore.kernel.org/r/20250219-generic126-v1-1-018e4e60c811@kernel.org
> ---

Hi Jeff,

Thanks for fixing nfs test issue again :)

>  tests/generic/126 | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/generic/126 b/tests/generic/126
> index 142d2bb97db706bafd2152856ef0993c5c273441..1f17492ed2a395ed3798c71243f3098558a157dc 100755
> --- a/tests/generic/126
> +++ b/tests/generic/126
> @@ -13,6 +13,7 @@ _begin_fstest perms auto quick
>  _cleanup()
>  {
>      cd /
> +    test -n "$testdir" && rm -rf "$testdir"
>  }
>  
>  # Import common functions.
> @@ -23,7 +24,13 @@ _require_chown
>  
>  QA_FS_PERMS=$here/src/fs_perms
>  
> -cd $TEST_DIR
> +testdir="$TEST_DIR/generic-126.$$"
> +
> +mkdir $testdir
> +chown 0:0 $testdir
> +chmod 0755 $testdir
> +cd $testdir
> +
>  cp $here/src/testx ./testx.file
>  
>  # file_perm owner_uid owner_gid tester_uid tester_gid perm_to_test expected_result pass=1
> @@ -46,7 +53,8 @@ $QA_FS_PERMS 200 99 99 200 99 w 1
>  $QA_FS_PERMS 040 99 99 99 500 r 1
>  $QA_FS_PERMS 400 99 99 200 99 r 1
>  
> -rm -f ./testx.file
> +cd /
> +rm -rf $testdir

I think these two steps are useless, due to you let _cleanup to do it. Others
looks good to me. If you agree, I'll help to change that when I merge this
patch :)

Reviewed-by: Zorro Lang <zlang@redhat.com>

Thanks,
Zorro

>  
>  status=0
>  exit
> 
> ---
> base-commit: 8467552f09e1672a02712653b532a84bd46ea10e
> change-id: 20250219-generic126-f81b3022c4b7
> 
> Best regards,
> -- 
> Jeff Layton <jlayton@kernel.org>
> 
>
Zorro Lang Feb. 20, 2025, 7:30 a.m. UTC | #3 
On Wed, Feb 19, 2025 at 12:57:55PM -0500, Jeff Layton wrote:
> I had been seeing some failures in generic/126 when running on NFS under
> kdevops.
> 
> The problem turned out to be that kdevops makes the root of the NFS
> export mode 01777 by default (i.e. with the sticky bit set). This causes
> some of these permission tests to fail because the open() is called with
> the O_CREAT bit set.
> 
> Make the outcome of this test not depend on the permissions on
> $TEST_DIR. Create a new directory to do these tests in, and clean it up
> when done.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> Changes in v2:
> - add commands to _cleanup() to clean up test directory (Thanks Darrick!)
> - Link to v1: https://lore.kernel.org/r/20250219-generic126-v1-1-018e4e60c811@kernel.org
> ---
>  tests/generic/126 | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/generic/126 b/tests/generic/126
> index 142d2bb97db706bafd2152856ef0993c5c273441..1f17492ed2a395ed3798c71243f3098558a157dc 100755
> --- a/tests/generic/126
> +++ b/tests/generic/126
> @@ -13,6 +13,7 @@ _begin_fstest perms auto quick
>  _cleanup()
>  {
>      cd /
> +    test -n "$testdir" && rm -rf "$testdir"
>  }
>  
>  # Import common functions.
> @@ -23,7 +24,13 @@ _require_chown
>  
>  QA_FS_PERMS=$here/src/fs_perms
>  
> -cd $TEST_DIR
> +testdir="$TEST_DIR/generic-126.$$"
> +
> +mkdir $testdir
> +chown 0:0 $testdir
> +chmod 0755 $testdir

Sorry, more review points :)

We have a _require_chmod helper, due to not all filesystems supports it.
If the chmod is need, you need to add _require_chmod.

And as Dave suggested, can you explain why these steps are necessary?
If this test fails without these steps, please add some comments for that.

Thanks,
Zorro

> +cd $testdir
> +
>  cp $here/src/testx ./testx.file
>  
>  # file_perm owner_uid owner_gid tester_uid tester_gid perm_to_test expected_result pass=1
> @@ -46,7 +53,8 @@ $QA_FS_PERMS 200 99 99 200 99 w 1
>  $QA_FS_PERMS 040 99 99 99 500 r 1
>  $QA_FS_PERMS 400 99 99 200 99 r 1
>  
> -rm -f ./testx.file
> +cd /
> +rm -rf $testdir
>  
>  status=0
>  exit
> 
> ---
> base-commit: 8467552f09e1672a02712653b532a84bd46ea10e
> change-id: 20250219-generic126-f81b3022c4b7
> 
> Best regards,
> -- 
> Jeff Layton <jlayton@kernel.org>
> 
>
diff mbox series

Patch

diff --git a/tests/generic/126 b/tests/generic/126
index 142d2bb97db706bafd2152856ef0993c5c273441..1f17492ed2a395ed3798c71243f3098558a157dc 100755
--- a/tests/generic/126
+++ b/tests/generic/126
@@ -13,6 +13,7 @@  _begin_fstest perms auto quick
 _cleanup()
 {
     cd /
+    test -n "$testdir" && rm -rf "$testdir"
 }
 
 # Import common functions.
@@ -23,7 +24,13 @@  _require_chown
 
 QA_FS_PERMS=$here/src/fs_perms
 
-cd $TEST_DIR
+testdir="$TEST_DIR/generic-126.$$"
+
+mkdir $testdir
+chown 0:0 $testdir
+chmod 0755 $testdir
+cd $testdir
+
 cp $here/src/testx ./testx.file
 
 # file_perm owner_uid owner_gid tester_uid tester_gid perm_to_test expected_result pass=1
@@ -46,7 +53,8 @@  $QA_FS_PERMS 200 99 99 200 99 w 1
 $QA_FS_PERMS 040 99 99 99 500 r 1
 $QA_FS_PERMS 400 99 99 200 99 r 1
 
-rm -f ./testx.file
+cd /
+rm -rf $testdir
 
 status=0
 exit