linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] generic: remove incorrect _require_idmapped_mounts checks
@ 2025-05-26 17:54 Amir Goldstein
  2025-05-30  5:31 ` Christian Brauner
  0 siblings, 1 reply; 2+ messages in thread
From: Amir Goldstein @ 2025-05-26 17:54 UTC (permalink / raw)
  To: Zorro Lang
  Cc: Christian Brauner, linux-fsdevel, fstests, Yang Xu,
	Anthony Iliopoulos, David Disseldorp

commit f5661920 ("generic: add missed _require_idmapped_mounts check")
wrongly adds _require_idmapped_mounts to tests that do not require
idmapped mounts support.

The added _require_idmapped_mounts in test generic/633 goes against
commit d8dee122 ("idmapped-mounts: always run generic vfs tests")
that intentionally removed this requirement from the generic tests.

The added _require_idmapped_mounts in tests generic/69{6,7} causes
those tests not to run with overlayfs, which does not support idmapped
mounts. However, those tests are regression tests to kernel commit
1639a49ccdce ("fs: move S_ISGID stripping into the vfs_*() helpers")
which is documented as also solving a correction issue with overlayfs,
so removing this test converage is very much undesired.

Remove the incorrectly added _require_idmapped_mounts checks.
Also fix the log in _require_idmapped_mounts to say that
"idmapped mounts not support by $FSTYP", which is what the helper
checks instead of "vfstests not support by $FSTYP" which is incorrect.

Cc: Yang Xu <xuyang2018.jy@fujitsu.com>
Cc: Anthony Iliopoulos <ailiop@suse.com>
Cc: David Disseldorp <ddiss@suse.de>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Christian,

Please confirm that I am not missing anything.

should we leave those tests or remove them from the idmapped test group?

Thanks,
Amir.

 common/rc         | 2 +-
 tests/generic/633 | 1 -
 tests/generic/696 | 1 -
 tests/generic/697 | 1 -
 4 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/common/rc b/common/rc
index bffd576a..96d65d1c 100644
--- a/common/rc
+++ b/common/rc
@@ -2639,7 +2639,7 @@ _require_idmapped_mounts()
 		--fstype "$FSTYP"
 
 	if [ $? -ne 0 ]; then
-		_notrun "vfstest not support by $FSTYP"
+		_notrun "idmapped mounts not support by $FSTYP"
 	fi
 }
 
diff --git a/tests/generic/633 b/tests/generic/633
index f58dbbf5..b683c427 100755
--- a/tests/generic/633
+++ b/tests/generic/633
@@ -12,7 +12,6 @@ _begin_fstest auto quick atime attr cap idmapped io_uring mount perms rw unlink
 # Import common functions.
 . ./common/filter
 
-_require_idmapped_mounts
 _require_test
 
 echo "Silence is golden"
diff --git a/tests/generic/696 b/tests/generic/696
index d2e86c96..48b3aea0 100755
--- a/tests/generic/696
+++ b/tests/generic/696
@@ -17,7 +17,6 @@ _begin_fstest auto quick cap idmapped mount perms rw unlink
 # Import common functions.
 . ./common/filter
 
-_require_idmapped_mounts
 _require_test
 _require_scratch
 _fixed_by_kernel_commit ac6800e279a2 \
diff --git a/tests/generic/697 b/tests/generic/697
index 1ce673f7..66444a95 100755
--- a/tests/generic/697
+++ b/tests/generic/697
@@ -17,7 +17,6 @@ _begin_fstest auto quick cap acl idmapped mount perms rw unlink
 . ./common/filter
 . ./common/attr
 
-_require_idmapped_mounts
 _require_test
 _require_acls
 _fixed_by_kernel_commit 1639a49ccdce \
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] generic: remove incorrect _require_idmapped_mounts checks
  2025-05-26 17:54 [PATCH] generic: remove incorrect _require_idmapped_mounts checks Amir Goldstein
@ 2025-05-30  5:31 ` Christian Brauner
  0 siblings, 0 replies; 2+ messages in thread
From: Christian Brauner @ 2025-05-30  5:31 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Zorro Lang, linux-fsdevel, fstests, Yang Xu, Anthony Iliopoulos,
	David Disseldorp

On Mon, May 26, 2025 at 07:54:37PM +0200, Amir Goldstein wrote:
> commit f5661920 ("generic: add missed _require_idmapped_mounts check")
> wrongly adds _require_idmapped_mounts to tests that do not require
> idmapped mounts support.
> 
> The added _require_idmapped_mounts in test generic/633 goes against
> commit d8dee122 ("idmapped-mounts: always run generic vfs tests")
> that intentionally removed this requirement from the generic tests.
> 
> The added _require_idmapped_mounts in tests generic/69{6,7} causes
> those tests not to run with overlayfs, which does not support idmapped
> mounts. However, those tests are regression tests to kernel commit
> 1639a49ccdce ("fs: move S_ISGID stripping into the vfs_*() helpers")
> which is documented as also solving a correction issue with overlayfs,
> so removing this test converage is very much undesired.
> 
> Remove the incorrectly added _require_idmapped_mounts checks.
> Also fix the log in _require_idmapped_mounts to say that
> "idmapped mounts not support by $FSTYP", which is what the helper
> checks instead of "vfstests not support by $FSTYP" which is incorrect.
> 
> Cc: Yang Xu <xuyang2018.jy@fujitsu.com>
> Cc: Anthony Iliopoulos <ailiop@suse.com>
> Cc: David Disseldorp <ddiss@suse.de>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---

Seems correct to me:

Reviewed-by: Christian Brauner <brauner@kernel.org>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-05-30  5:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-26 17:54 [PATCH] generic: remove incorrect _require_idmapped_mounts checks Amir Goldstein
2025-05-30  5:31 ` Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).