[PATCH v1 0/7] add test requirements

[PATCH v1 0/7] add test requirements

[Yuezhang Mo]

These patches add test requirements to the following tests so that
they do not run on filesystems that do not support these requirements,
rather than causing the tests to fail.

Yuezhang Mo (7):
  generic/696: add _require_chmod
  generic/656: add _require_chown
  generic/754: add _require_symlinks
  generic/755: add _require_hardlinks
  generic/730: add _require_scratch_shutdown
  generic/645: add _require_chown
  generic/633: add test requirements

 tests/generic/633 | 6 ++++++
 tests/generic/645 | 1 +
 tests/generic/656 | 1 +
 tests/generic/696 | 1 +
 tests/generic/730 | 1 +
 tests/generic/754 | 1 +
 tests/generic/755 | 1 +
 7 files changed, 12 insertions(+)

-- 
2.25.1

[PATCH v1 1/7] generic/696: add _require_chmod

[Yuezhang Mo]

This test uses chmod to set the S_ISGID bit, which requires chmod
support, so add _require_chmod to check for chmod support.

Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
Reviewed-by: Friendy Su <friendy.su@sony.com>
---
 tests/generic/696 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/generic/696 b/tests/generic/696
index d2e86c96..8731cad2 100755
--- a/tests/generic/696
+++ b/tests/generic/696
@@ -20,6 +20,7 @@ _begin_fstest auto quick cap idmapped mount perms rw unlink
 _require_idmapped_mounts
 _require_test
 _require_scratch
+_require_chmod
 _fixed_by_kernel_commit ac6800e279a2 \
 	"fs: Add missing umask strip in vfs_tmpfile" \
 1639a49ccdce "fs: move S_ISGID stripping into the vfs_*() helpers"
-- 
2.25.1

[PATCH v1 2/7] generic/656: add _require_chown

[Yuezhang Mo]

This test uses chown to set GID and UID, which requires chown
support, so add _require_chown to check for chown support.

Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
Reviewed-by: Friendy Su <friendy.su@sony.com>
---
 tests/generic/656 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/generic/656 b/tests/generic/656
index cd49c4f6..2ff1c517 100755
--- a/tests/generic/656
+++ b/tests/generic/656
@@ -17,6 +17,7 @@ _begin_fstest auto attr cap idmapped mount perms
 
 _require_idmapped_mounts
 _require_test
+_require_chown
 _require_user fsgqa
 _require_user fsgqa2
 _require_group fsgqa
-- 
2.25.1

[PATCH v1 3/7] generic/754: add _require_symlinks

[Yuezhang Mo]

This test tests whether xattrs can be added to a symbolic link,
which requires symlink support, so add _require_symlinks to check
for symlink support.

Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
Reviewed-by: Friendy Su <friendy.su@sony.com>
---
 tests/generic/754 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/generic/754 b/tests/generic/754
index 7afda609..c1016641 100755
--- a/tests/generic/754
+++ b/tests/generic/754
@@ -10,6 +10,7 @@
 #
 . ./common/preamble
 _begin_fstest auto
+_require_symlinks
 
 _require_scratch
 
-- 
2.25.1

[PATCH v1 4/7] generic/755: add _require_hardlinks

[Yuezhang Mo]

This test is used to test whether the ctime of the original file
changes after deleting its hard link, which requires hardlink
support, so add _require_hardlinks to check for hardlink support.

Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
Reviewed-by: Friendy Su <friendy.su@sony.com>
---
 tests/generic/755 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/generic/755 b/tests/generic/755
index a0c5abd4..b2fd3e58 100755
--- a/tests/generic/755
+++ b/tests/generic/755
@@ -10,6 +10,7 @@
 . ./common/preamble
 _begin_fstest auto quick
 
+_require_hardlinks
 _require_test
 [ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit 3bc2ac2f8f0b \
 	"btrfs: update target inode's ctime on unlink"
-- 
2.25.1

[PATCH v1 5/7] generic/730: add _require_scratch_shutdown

[Yuezhang Mo]

Before testing for shutdown, check whether the file system
supports it with _require_scratch_shutdown.

Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
Reviewed-by: Friendy Su <friendy.su@sony.com>
---
 tests/generic/730 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/generic/730 b/tests/generic/730
index 6251980e..1dce547f 100755
--- a/tests/generic/730
+++ b/tests/generic/730
@@ -25,6 +25,7 @@ _cleanup()
 _require_test
 _require_block_device $TEST_DEV
 _require_scsi_debug
+_require_scratch_shutdown
 
 size=$(_small_fs_size_mb 256)
 SCSI_DEBUG_DEV=`_get_scsi_debug_dev 512 512 0 $size`
-- 
2.25.1

[PATCH v1 6/7] generic/645: add _require_chown

[Yuezhang Mo]

This test uses chown to set GID and UID, which requires chown
support, so add _require_chown to check for chown support.

Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
Reviewed-by: Friendy Su <friendy.su@sony.com>
---
 tests/generic/645 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/generic/645 b/tests/generic/645
index 068a46a9..d6eb75e6 100755
--- a/tests/generic/645
+++ b/tests/generic/645
@@ -15,6 +15,7 @@ _begin_fstest auto quick idmapped mount
 
 _require_idmapped_mounts
 _require_test
+_require_chown
 _wants_kernel_commit dacfd001eaf2 \
 	"fs/mnt_idmapping.c: Return -EINVAL when no map is written"
 
-- 
2.25.1

[PATCH v1 7/7] generic/633: add test requirements

[Yuezhang Mo]

Since the following test requirements are required by the following
sub-tests, this commit adds them to this test to avoid failures on
filesystems that do not support them.

requirement | sub-tests
------------+-------------------------------------------------------
hardlink    | hardlink_crossing_mounts tcore_fsids_mapped
            | tcore_hardlink_crossing_idmapped_mounts
            | tcore_hardlink_from_idmapped_mount
            | tcore_hardlink_from_idmapped_mount_in_userns
------------|-------------------------------------------------------
symlink     | protected_symlinks symlink_regular_mounts
            | tcore_create_in_userns tcore_symlink_idmapped_mounts
            | tcore_symlink_idmapped_mounts_in_userns
------------|-------------------------------------------------------
chown       | hardlink_crossing_mounts rename_crossing_mounts
            | setid_binaries sticky_bit_unlink
            | symlink_regular_mounts tcore_acls
            | tcore_hardlink_crossing_idmapped_mounts
            | tcore_hardlink_from_idmapped_mount
            | tcore_protected_symlinks_idmapped_mounts
            | tcore_rename_crossing_idmapped_mounts
            | tcore_rename_from_idmapped_mount
            | tcore_setattr_truncate_idmapped_in_userns
            | tcore_setid_binaries_idmapped_mounts
            | tcore_setid_binaries_idmapped_mounts_in_userns
            | tcore_setid_binaries_idmapped_mounts_in_userns_separate_userns
            | tcore_sticky_bit_unlink_idmapped_mounts
            | tcore_sticky_bit_unlink_idmapped_mounts_in_userns
            | tcore_sticky_bit_rename_idmapped_mounts
            | tcore_sticky_bit_rename_idmapped_mounts_in_userns
------------|-------------------------------------------------------
chmod       | protected_symlinks
            | tcore_protected_symlinks_idmapped_mounts_in_userns
------------|-------------------------------------------------------
io_uring    | io_uring io_uring_userns tcore_io_uring_idmapped_userns
            | tcore_io_uring_idmapped_unmapped
            | tcore_io_uring_idmapped_unmapped_userns
------------|-------------------------------------------------------
mknod       | tcore_expected_uid_gid_idmapped_mounts

Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
Reviewed-by: Friendy Su <friendy.su@sony.com>
---
 tests/generic/633 | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/generic/633 b/tests/generic/633
index f58dbbf5..e19a2043 100755
--- a/tests/generic/633
+++ b/tests/generic/633
@@ -13,6 +13,12 @@ _begin_fstest auto quick atime attr cap idmapped io_uring mount perms rw unlink
 . ./common/filter
 
 _require_idmapped_mounts
+_require_hardlinks
+_require_symlinks
+_require_chown
+_require_chmod
+_require_io_uring
+_require_mknod
 _require_test
 
 echo "Silence is golden"
-- 
2.25.1

Re: [PATCH v1 5/7] generic/730: add _require_scratch_shutdown

[Amir Goldstein]

On Wed, May 28, 2025 at 11:15 AM Yuezhang Mo <Yuezhang.Mo@sony.com> wrote:
>
> Before testing for shutdown, check whether the file system
> supports it with _require_scratch_shutdown.

Where exactly is this test performing shutdown?

>
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> Reviewed-by: Friendy Su <friendy.su@sony.com>
> ---
>  tests/generic/730 | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tests/generic/730 b/tests/generic/730
> index 6251980e..1dce547f 100755
> --- a/tests/generic/730
> +++ b/tests/generic/730
> @@ -25,6 +25,7 @@ _cleanup()
>  _require_test
>  _require_block_device $TEST_DEV
>  _require_scsi_debug
> +_require_scratch_shutdown
>
>  size=$(_small_fs_size_mb 256)
>  SCSI_DEBUG_DEV=`_get_scsi_debug_dev 512 512 0 $size`
> --
> 2.25.1
>
>

Re: [PATCH v1 7/7] generic/633: add test requirements

[Amir Goldstein]

On Wed, May 28, 2025 at 11:15 AM Yuezhang Mo <Yuezhang.Mo@sony.com> wrote:
>
> Since the following test requirements are required by the following
> sub-tests, this commit adds them to this test to avoid failures on
> filesystems that do not support them.
>
> requirement | sub-tests
> ------------+-------------------------------------------------------
> hardlink    | hardlink_crossing_mounts tcore_fsids_mapped
>             | tcore_hardlink_crossing_idmapped_mounts
>             | tcore_hardlink_from_idmapped_mount
>             | tcore_hardlink_from_idmapped_mount_in_userns
> ------------|-------------------------------------------------------
> symlink     | protected_symlinks symlink_regular_mounts
>             | tcore_create_in_userns tcore_symlink_idmapped_mounts
>             | tcore_symlink_idmapped_mounts_in_userns
> ------------|-------------------------------------------------------
> chown       | hardlink_crossing_mounts rename_crossing_mounts
>             | setid_binaries sticky_bit_unlink
>             | symlink_regular_mounts tcore_acls
>             | tcore_hardlink_crossing_idmapped_mounts
>             | tcore_hardlink_from_idmapped_mount
>             | tcore_protected_symlinks_idmapped_mounts
>             | tcore_rename_crossing_idmapped_mounts
>             | tcore_rename_from_idmapped_mount
>             | tcore_setattr_truncate_idmapped_in_userns
>             | tcore_setid_binaries_idmapped_mounts
>             | tcore_setid_binaries_idmapped_mounts_in_userns
>             | tcore_setid_binaries_idmapped_mounts_in_userns_separate_userns
>             | tcore_sticky_bit_unlink_idmapped_mounts
>             | tcore_sticky_bit_unlink_idmapped_mounts_in_userns
>             | tcore_sticky_bit_rename_idmapped_mounts
>             | tcore_sticky_bit_rename_idmapped_mounts_in_userns
> ------------|-------------------------------------------------------
> chmod       | protected_symlinks
>             | tcore_protected_symlinks_idmapped_mounts_in_userns
> ------------|-------------------------------------------------------
> io_uring    | io_uring io_uring_userns tcore_io_uring_idmapped_userns
>             | tcore_io_uring_idmapped_unmapped
>             | tcore_io_uring_idmapped_unmapped_userns
> ------------|-------------------------------------------------------
> mknod       | tcore_expected_uid_gid_idmapped_mounts
>
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> Reviewed-by: Friendy Su <friendy.su@sony.com>
> ---
>  tests/generic/633 | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tests/generic/633 b/tests/generic/633
> index f58dbbf5..e19a2043 100755
> --- a/tests/generic/633
> +++ b/tests/generic/633
> @@ -13,6 +13,12 @@ _begin_fstest auto quick atime attr cap idmapped io_uring mount perms rw unlink
>  . ./common/filter
>
>  _require_idmapped_mounts

Please note that the requirement above is wrong:

https://lore.kernel.org/fstests/20250526175437.1528310-1-amir73il@gmail.com/

> +_require_hardlinks
> +_require_symlinks
> +_require_chown
> +_require_chmod
> +_require_io_uring
> +_require_mknod

I wonder if it makes more sense to have a collective check for all (or
most) of the above,
e.g. _require_vfstest, which is different than _require_idmapped_mounts,
so that we do not have to add all of these for tests running
other vfstests subtests in the future.

Thanks,
Amir.

Re: [PATCH v1 0/7] add test requirements

[Christoph Hellwig]

On Wed, May 28, 2025 at 05:15:23PM +0800, Yuezhang Mo wrote:
> These patches add test requirements to the following tests so that
> they do not run on filesystems that do not support these requirements,
> rather than causing the tests to fail.

What file system do you want to support?  How are they going to work
without wiring them up in common/rc?

Re: [PATCH v1 5/7] generic/730: add _require_scratch_shutdown

[Yuezhang.Mo]

On Wed, May 28, 2025 at 06:47 PM Amir Goldstein <amir73il@gmail.com> wrote:
> On Wed, May 28, 2025 at 11:15 AM Yuezhang Mo <Yuezhang.Mo@sony.com> wrote:
> >
> > Before testing for shutdown, check whether the file system
> > supports it with _require_scratch_shutdown.
> 
> Where exactly is this test performing shutdown?

The following line performs the shutdown.

 echo 1 > /sys/block/$(_short_dev $SCSI_DEBUG_DEV)/device/delete

Re: [PATCH v1 0/7] add test requirements

[Yuezhang.Mo]

> On Wed, May 28, 2025 at 05:15:23PM +0800, Yuezhang Mo wrote:
> > These patches add test requirements to the following tests so that
> > they do not run on filesystems that do not support these requirements,
> > rather than causing the tests to fail.
>
> What file system do you want to support?  How are they going to work
> without wiring them up in common/rc?

For filesystems that do not support the features(symlink, hardlink, access 
permissions, etc), such as exfat, these tests will always fail and there is no 
need to run them.

Re: [PATCH v1 7/7] generic/633: add test requirements

[Yuezhang.Mo]

> >  _require_idmapped_mounts
>
> Please note that the requirement above is wrong:
> 
> https://lore.kernel.org/fstests/20250526175437.1528310-1-amir73il@gmail.com/

Thanks for the info, if so this patch will need to be rebased.

Re: [PATCH v1 0/7] add test requirements

[Christoph Hellwig]

On Thu, May 29, 2025 at 05:43:48AM +0000, Yuezhang.Mo@sony.com wrote:
> For filesystems that do not support the features(symlink, hardlink, access 
> permissions, etc), such as exfat, these tests will always fail and there is no 
> need to run them.

So this is for exfat?  What do the results look like with this patchset?

Re: [PATCH v1 0/7] add test requirements

[Yuezhang.Mo]

> On Thu, May 29, 2025 at 05:43:48AM +0000, Yuezhang.Mo@sony.com wrote:
> > For filesystems that do not support the features(symlink, hardlink, access
> > permissions, etc), such as exfat, these tests will always fail and there is no
> > need to run them.
> 
> So this is for exfat?  What do the results look like with this patchset?

Yes, this patch set also for other file systems like exfat which do not support the
above features.

With this patch set, these test cases will be skipped as "Not Run".

Re: [PATCH v1 0/7] add test requirements

[Christoph Hellwig]

On Tue, Jun 03, 2025 at 02:35:04AM +0000, Yuezhang.Mo@sony.com wrote:
> Yes, this patch set also for other file systems like exfat which do not support the
> above features.

So is this for exfat or "other file systems".  How do the results from
an auto run with exfat look with these patches?

Re: [PATCH v1 0/7] add test requirements

[Yuezhang.Mo]

> On Tue, Jun 03, 2025 at 02:35:04AM +0000, Yuezhang.Mo@sony.com wrote:
> > Yes, this patch set also for other file systems like exfat which do not support the
> > above features.
> 
> So is this for exfat or "other file systems".  How do the results from
> an auto run with exfat look with these patches?

In my environment, except for the following test cases, all other cases pass.

generic/622: It seems that some adjustments need to be made for exfat
generic/689:  _require_acls should be added, I'll append a commit for it to this patchset
generic/740:  exfatprogs's mkfs.exfat doesn't support foreign fs detection, but other
                      mkfs.exfat might support it, adding "_exclude_fs exfat" doesn't seem good.

Re: [PATCH v1 5/7] generic/730: add _require_scratch_shutdown

[Anand Jain]

On 5/28/2025 5:15 PM, Yuezhang Mo wrote:
> Before testing for shutdown, check whether the file system
> supports it with _require_scratch_shutdown.
> 
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> Reviewed-by: Friendy Su <friendy.su@sony.com>
> ---
>   tests/generic/730 | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/tests/generic/730 b/tests/generic/730
> index 6251980e..1dce547f 100755
> --- a/tests/generic/730
> +++ b/tests/generic/730

Zorro,

While here, you could also add this test case to the shutdown group.
Nevertheless,

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thx.


> @@ -25,6 +25,7 @@ _cleanup()
>   _require_test
>   _require_block_device $TEST_DEV
>   _require_scsi_debug
> +_require_scratch_shutdown
>   
>   size=$(_small_fs_size_mb 256)
>   SCSI_DEBUG_DEV=`_get_scsi_debug_dev 512 512 0 $size`

Re: [PATCH v1 7/7] generic/633: add test requirements

[Zorro Lang]

On Wed, May 28, 2025 at 12:56:55PM +0200, Amir Goldstein wrote:
> On Wed, May 28, 2025 at 11:15 AM Yuezhang Mo <Yuezhang.Mo@sony.com> wrote:
> >
> > Since the following test requirements are required by the following
> > sub-tests, this commit adds them to this test to avoid failures on
> > filesystems that do not support them.
> >
> > requirement | sub-tests
> > ------------+-------------------------------------------------------
> > hardlink    | hardlink_crossing_mounts tcore_fsids_mapped
> >             | tcore_hardlink_crossing_idmapped_mounts
> >             | tcore_hardlink_from_idmapped_mount
> >             | tcore_hardlink_from_idmapped_mount_in_userns
> > ------------|-------------------------------------------------------
> > symlink     | protected_symlinks symlink_regular_mounts
> >             | tcore_create_in_userns tcore_symlink_idmapped_mounts
> >             | tcore_symlink_idmapped_mounts_in_userns
> > ------------|-------------------------------------------------------
> > chown       | hardlink_crossing_mounts rename_crossing_mounts
> >             | setid_binaries sticky_bit_unlink
> >             | symlink_regular_mounts tcore_acls
> >             | tcore_hardlink_crossing_idmapped_mounts
> >             | tcore_hardlink_from_idmapped_mount
> >             | tcore_protected_symlinks_idmapped_mounts
> >             | tcore_rename_crossing_idmapped_mounts
> >             | tcore_rename_from_idmapped_mount
> >             | tcore_setattr_truncate_idmapped_in_userns
> >             | tcore_setid_binaries_idmapped_mounts
> >             | tcore_setid_binaries_idmapped_mounts_in_userns
> >             | tcore_setid_binaries_idmapped_mounts_in_userns_separate_userns
> >             | tcore_sticky_bit_unlink_idmapped_mounts
> >             | tcore_sticky_bit_unlink_idmapped_mounts_in_userns
> >             | tcore_sticky_bit_rename_idmapped_mounts
> >             | tcore_sticky_bit_rename_idmapped_mounts_in_userns
> > ------------|-------------------------------------------------------
> > chmod       | protected_symlinks
> >             | tcore_protected_symlinks_idmapped_mounts_in_userns
> > ------------|-------------------------------------------------------
> > io_uring    | io_uring io_uring_userns tcore_io_uring_idmapped_userns
> >             | tcore_io_uring_idmapped_unmapped
> >             | tcore_io_uring_idmapped_unmapped_userns
> > ------------|-------------------------------------------------------
> > mknod       | tcore_expected_uid_gid_idmapped_mounts
> >
> > Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> > Reviewed-by: Friendy Su <friendy.su@sony.com>
> > ---
> >  tests/generic/633 | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/tests/generic/633 b/tests/generic/633
> > index f58dbbf5..e19a2043 100755
> > --- a/tests/generic/633
> > +++ b/tests/generic/633
> > @@ -13,6 +13,12 @@ _begin_fstest auto quick atime attr cap idmapped io_uring mount perms rw unlink
> >  . ./common/filter
> >
> >  _require_idmapped_mounts
> 
> Please note that the requirement above is wrong:
> 
> https://lore.kernel.org/fstests/20250526175437.1528310-1-amir73il@gmail.com/
> 
> > +_require_hardlinks
> > +_require_symlinks
> > +_require_chown
> > +_require_chmod
> > +_require_io_uring
> > +_require_mknod
> 
> I wonder if it makes more sense to have a collective check for all (or
> most) of the above,
> e.g. _require_vfstest, which is different than _require_idmapped_mounts,
> so that we do not have to add all of these for tests running
> other vfstests subtests in the future.

Yeah, and some _require_ looks optional, e.g. _require_io_uring, why is it a
necessary requirement, we even run xfstests with io_uring. If so, we don't do
vfstest if io_uring isn't supported?

Although we can add as many as possible _require_* checks to a test case, but
more _require_* take more time to run. So I think we'd better to add "missed"
_require_* for specific test failure on someone fs, not only "I reviewed the
code, so I feel all of these are needed"

Thanks,
Zorro

> 
> Thanks,
> Amir.
> 

Re: [PATCH v1 6/7] generic/645: add _require_chown

[Zorro Lang]

On Wed, May 28, 2025 at 05:15:29PM +0800, Yuezhang Mo wrote:
> This test uses chown to set GID and UID, which requires chown
> support, so add _require_chown to check for chown support.
> 
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> Reviewed-by: Friendy Su <friendy.su@sony.com>
> ---
>  tests/generic/645 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/generic/645 b/tests/generic/645
> index 068a46a9..d6eb75e6 100755
> --- a/tests/generic/645
> +++ b/tests/generic/645
> @@ -15,6 +15,7 @@ _begin_fstest auto quick idmapped mount
>  
>  _require_idmapped_mounts
>  _require_test
> +_require_chown

Yeah, I think this makes sense to me, due to I always hit the error as:
  +idmapped-mounts.c: 6599: nested_userns - Operation not permitted - failure: fchownat dir1/file1_1
  +vfstest.c: 2418: run_test - Operation not permitted - failure: test that nested user namespaces behave correctly when attached to idmapped mounts

when tests on exfat.

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

>  _wants_kernel_commit dacfd001eaf2 \
>  	"fs/mnt_idmapping.c: Return -EINVAL when no map is written"
>  
> -- 
> 2.25.1
> 
> 

Re: [PATCH v1 5/7] generic/730: add _require_scratch_shutdown

[Zorro Lang]

On Wed, May 28, 2025 at 05:15:28PM +0800, Yuezhang Mo wrote:
> Before testing for shutdown, check whether the file system
> supports it with _require_scratch_shutdown.
> 
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> Reviewed-by: Friendy Su <friendy.su@sony.com>
> ---
>  tests/generic/730 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/generic/730 b/tests/generic/730
> index 6251980e..1dce547f 100755
> --- a/tests/generic/730
> +++ b/tests/generic/730
> @@ -25,6 +25,7 @@ _cleanup()
>  _require_test
>  _require_block_device $TEST_DEV
>  _require_scsi_debug
> +_require_scratch_shutdown

Hmm... although this test case really does "shutdown", but this _require_scratch_shutdown
looks weird, due to the g/730 doesn't has _require_scratch, that means there might be not
SCRATCH_DEV. So...(see below)

>  
>  size=$(_small_fs_size_mb 256)
>  SCSI_DEBUG_DEV=`_get_scsi_debug_dev 512 512 0 $size`

... how about

diff --git a/tests/generic/730 b/tests/generic/730
index 6251980e..6b5d3196 100755
--- a/tests/generic/730
+++ b/tests/generic/730
@@ -29,6 +29,7 @@ _require_scsi_debug
 size=$(_small_fs_size_mb 256)
 SCSI_DEBUG_DEV=`_get_scsi_debug_dev 512 512 0 $size`
 test -b "$SCSI_DEBUG_DEV" || _notrun "Failed to initialize scsi debug device"
+SCRATCH_DEV=$SCSI_DEBUG_DEV _require_scratch_shutdown
 echo "SCSI debug device $SCSI_DEBUG_DEV" >>$seqres.full

Thanks,
Zorro

> -- 
> 2.25.1
> 
> 

Re: [PATCH v1 1/7] generic/696: add _require_chmod

[Zorro Lang]

On Wed, May 28, 2025 at 05:15:24PM +0800, Yuezhang Mo wrote:
> This test uses chmod to set the S_ISGID bit, which requires chmod
> support, so add _require_chmod to check for chmod support.
> 
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> Reviewed-by: Friendy Su <friendy.su@sony.com>
> ---
>  tests/generic/696 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/generic/696 b/tests/generic/696
> index d2e86c96..8731cad2 100755
> --- a/tests/generic/696
> +++ b/tests/generic/696
> @@ -20,6 +20,7 @@ _begin_fstest auto quick cap idmapped mount perms rw unlink
>  _require_idmapped_mounts
>  _require_test
>  _require_scratch
> +_require_chmod

OK, this make sense to me, due to I hit below error too:

    +vfstest.c: 1781: setgid_create_umask - Success - failure: is_setgid
    +vfstest.c: 2418: run_test - Success - failure: create operations in directories with setgid bit set under umask

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

>  _fixed_by_kernel_commit ac6800e279a2 \
>  	"fs: Add missing umask strip in vfs_tmpfile" \
>  1639a49ccdce "fs: move S_ISGID stripping into the vfs_*() helpers"
> -- 
> 2.25.1
> 
> 

Re: [PATCH v1 2/7] generic/656: add _require_chown

[Zorro Lang]

On Wed, May 28, 2025 at 05:15:25PM +0800, Yuezhang Mo wrote:
> This test uses chown to set GID and UID, which requires chown
> support, so add _require_chown to check for chown support.
> 
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> Reviewed-by: Friendy Su <friendy.su@sony.com>
> ---
>  tests/generic/656 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/generic/656 b/tests/generic/656
> index cd49c4f6..2ff1c517 100755
> --- a/tests/generic/656
> +++ b/tests/generic/656
> @@ -17,6 +17,7 @@ _begin_fstest auto attr cap idmapped mount perms
>  
>  _require_idmapped_mounts
>  _require_test
> +_require_chown

Makes sense, I got below error too:
    +idmapped-mounts.c: 7277: setattr_fix_968219708108 - Operation not permitted - failure: chown_r
    +vfstest.c: 2418: run_test - Operation not permitted - failure: test that setattr works correctly

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

>  _require_user fsgqa
>  _require_user fsgqa2
>  _require_group fsgqa
> -- 
> 2.25.1
> 
> 

Re: [PATCH v1 3/7] generic/754: add _require_symlinks

[Zorro Lang]

On Wed, May 28, 2025 at 05:15:26PM +0800, Yuezhang Mo wrote:
> This test tests whether xattrs can be added to a symbolic link,
> which requires symlink support, so add _require_symlinks to check
> for symlink support.
> 
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> Reviewed-by: Friendy Su <friendy.su@sony.com>
> ---
>  tests/generic/754 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/generic/754 b/tests/generic/754
> index 7afda609..c1016641 100755
> --- a/tests/generic/754
> +++ b/tests/generic/754
> @@ -10,6 +10,7 @@
>  #
>  . ./common/preamble
>  _begin_fstest auto
> +_require_symlinks

Make sense to fix this error on FAT:

    +/mnt/scratch/symlink.32: target is corrupt
    +/mnt/scratch/symlink.64: target is corrupt
    +/mnt/scratch/symlink.96: target is corrupt
    +/mnt/scratch/symlink.128: target is corrupt
    +/mnt/scratch/symlink.160: target is corrupt
    +/mnt/scratch/symlink.192: target is corrupt
    ...

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

>  
>  _require_scratch
>  
> -- 
> 2.25.1
> 
> 

Re: [PATCH v1 4/7] generic/755: add _require_hardlinks

[Zorro Lang]

On Wed, May 28, 2025 at 05:15:27PM +0800, Yuezhang Mo wrote:
> This test is used to test whether the ctime of the original file
> changes after deleting its hard link, which requires hardlink
> support, so add _require_hardlinks to check for hardlink support.
> 
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> Reviewed-by: Friendy Su <friendy.su@sony.com>
> ---
>  tests/generic/755 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/generic/755 b/tests/generic/755
> index a0c5abd4..b2fd3e58 100755
> --- a/tests/generic/755
> +++ b/tests/generic/755
> @@ -10,6 +10,7 @@
>  . ./common/preamble
>  _begin_fstest auto quick
>  
> +_require_hardlinks

OK, it helps to avoid this failure:

    +ln: failed to create hard link '/mnt/test/unlink-ctime2.132335' => '/mnt/test/unlink-ctime1.132335': Operation not permitted
    +unlink: cannot unlink '/mnt/test/unlink-ctime2.132335': No such file or directory
    +Target's ctime did not change after unlink!

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

>  _require_test
>  [ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit 3bc2ac2f8f0b \
>  	"btrfs: update target inode's ctime on unlink"
> -- 
> 2.25.1
> 
> 

Re: [PATCH v1 0/7] add test requirements

[Zorro Lang]

On Wed, May 28, 2025 at 05:15:23PM +0800, Yuezhang Mo wrote:
> These patches add test requirements to the following tests so that
> they do not run on filesystems that do not support these requirements,
> rather than causing the tests to fail.
> 

I've reviewed this patchset, now most of patches are Acked, except ...

> Yuezhang Mo (7):
>   generic/696: add _require_chmod
>   generic/656: add _require_chown
>   generic/754: add _require_symlinks
>   generic/755: add _require_hardlinks
>   generic/730: add _require_scratch_shutdown

This need change, but as btrfs list hoped to have it too, so I can help to
do this change when I merge it if you agree with my review point.

>   generic/645: add _require_chown
>   generic/633: add test requirements

This need more update and review, so I'll merge this patchset without
this patch. You can change and resend this patch singly later.

Thanks,
Zorro

> 
>  tests/generic/633 | 6 ++++++
>  tests/generic/645 | 1 +
>  tests/generic/656 | 1 +
>  tests/generic/696 | 1 +
>  tests/generic/730 | 1 +
>  tests/generic/754 | 1 +
>  tests/generic/755 | 1 +
>  7 files changed, 12 insertions(+)
> 
> -- 
> 2.25.1
> 
> 

Re: [PATCH v1 7/7] generic/633: add test requirements

[Yuezhang.Mo]

> On Wed, May 28, 2025 at 12:56:55PM +0200, Amir Goldstein wrote:
> > On Wed, May 28, 2025 at 11:15 AM Yuezhang Mo <Yuezhang.Mo@sony.com> wrote:
> > >
> > > Since the following test requirements are required by the following
> > > sub-tests, this commit adds them to this test to avoid failures on
> > > filesystems that do not support them.
> > >
> > > requirement | sub-tests
> > > ------------+-------------------------------------------------------
> > > hardlink    | hardlink_crossing_mounts tcore_fsids_mapped
> > >             | tcore_hardlink_crossing_idmapped_mounts
> > >             | tcore_hardlink_from_idmapped_mount
> > >             | tcore_hardlink_from_idmapped_mount_in_userns
> > > ------------|-------------------------------------------------------
> > > symlink     | protected_symlinks symlink_regular_mounts
> > >             | tcore_create_in_userns tcore_symlink_idmapped_mounts
> > >             | tcore_symlink_idmapped_mounts_in_userns
> > > ------------|-------------------------------------------------------
> > > chown       | hardlink_crossing_mounts rename_crossing_mounts
> > >             | setid_binaries sticky_bit_unlink
> > >             | symlink_regular_mounts tcore_acls
> > >             | tcore_hardlink_crossing_idmapped_mounts
> > >             | tcore_hardlink_from_idmapped_mount
> > >             | tcore_protected_symlinks_idmapped_mounts
> > >             | tcore_rename_crossing_idmapped_mounts
> > >             | tcore_rename_from_idmapped_mount
> > >             | tcore_setattr_truncate_idmapped_in_userns
> > >             | tcore_setid_binaries_idmapped_mounts
> > >             | tcore_setid_binaries_idmapped_mounts_in_userns
> > >             | tcore_setid_binaries_idmapped_mounts_in_userns_separate_userns
> > >             | tcore_sticky_bit_unlink_idmapped_mounts
> > >             | tcore_sticky_bit_unlink_idmapped_mounts_in_userns
> > >             | tcore_sticky_bit_rename_idmapped_mounts
> > >             | tcore_sticky_bit_rename_idmapped_mounts_in_userns
> > > ------------|-------------------------------------------------------
> > > chmod       | protected_symlinks
> > >             | tcore_protected_symlinks_idmapped_mounts_in_userns
> > > ------------|-------------------------------------------------------
> > > io_uring    | io_uring io_uring_userns tcore_io_uring_idmapped_userns
> > >             | tcore_io_uring_idmapped_unmapped
> > >             | tcore_io_uring_idmapped_unmapped_userns
> > > ------------|-------------------------------------------------------
> > > mknod       | tcore_expected_uid_gid_idmapped_mounts
> > >
> > > Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> > > Reviewed-by: Friendy Su <friendy.su@sony.com>
> > > ---
> > >  tests/generic/633 | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/tests/generic/633 b/tests/generic/633
> > > index f58dbbf5..e19a2043 100755
> > > --- a/tests/generic/633
> > > +++ b/tests/generic/633
> > > @@ -13,6 +13,12 @@ _begin_fstest auto quick atime attr cap idmapped io_uring mount perms rw unlink
> > >  . ./common/filter
> > >
> > >  _require_idmapped_mounts
> >
> > Please note that the requirement above is wrong:
> >
> > https://lore.kernel.org/fstests/20250526175437.1528310-1-amir73il@gmail.com/
> >
> > > +_require_hardlinks
> > > +_require_symlinks
> > > +_require_chown
> > > +_require_chmod
> > > +_require_io_uring
> > > +_require_mknod
> >
> > I wonder if it makes more sense to have a collective check for all (or
> > most) of the above,
> > e.g. _require_vfstest, which is different than _require_idmapped_mounts,
> > so that we do not have to add all of these for tests running
> > other vfstests subtests in the future.
> 
> Yeah, and some _require_ looks optional, e.g. _require_io_uring, why is it a
> necessary requirement, we even run xfstests with io_uring. If so, we don't do
> vfstest if io_uring isn't supported?

OK, I will removed '_require_io_uring'.

> 
> Although we can add as many as possible _require_* checks to a test case, but
> more _require_* take more time to run. So I think we'd better to add "missed"
> _require_* for specific test failure on someone fs, not only "I reviewed the
> code, so I feel all of these are needed"

Do you mean we should check these requirements only for some specific fs?
Like this.

if [ "${FSTYP}" == "exfat" ]; then
      _require_hardlinks
      _require_symlinks
      _require_chown
      _require_chmod
     _require_mknod
fi

I think this is not good, firstly because it is hard to maintain(other fs may also
need these checks), and secondly because checking these requirements does
not take much time (in my environment it takes less than a second).

Re: [PATCH v1 5/7] generic/730: add _require_scratch_shutdown

[Yuezhang.Mo]

> >  _require_block_device $TEST_DEV
> >  _require_scsi_debug
> > +_require_scratch_shutdown
> 
> Hmm... although this test case really does "shutdown", but this _require_scratch_shutdown
> looks weird, due to the g/730 doesn't has _require_scratch, that means there might be not
> SCRATCH_DEV. So...(see below)
> 
> >
> >  size=$(_small_fs_size_mb 256)
> >  SCSI_DEBUG_DEV=`_get_scsi_debug_dev 512 512 0 $size`
> 
> ... how about
> 
> diff --git a/tests/generic/730 b/tests/generic/730
> index 6251980e..6b5d3196 100755
> --- a/tests/generic/730
> +++ b/tests/generic/730
> @@ -29,6 +29,7 @@ _require_scsi_debug
>  size=$(_small_fs_size_mb 256)
>  SCSI_DEBUG_DEV=`_get_scsi_debug_dev 512 512 0 $size`
>  test -b "$SCSI_DEBUG_DEV" || _notrun "Failed to initialize scsi debug device"
> +SCRATCH_DEV=$SCSI_DEBUG_DEV _require_scratch_shutdown
>  echo "SCSI debug device $SCSI_DEBUG_DEV" >>$seqres.full

I agree, I will update this patch.

[PATCH v2 5/7] generic/730: add _require_scratch_shutdown

[Yuezhang Mo]

Before testing for shutdown, check whether the file system
supports it with _require_scratch_shutdown.

Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
Reviewed-by: Friendy Su <friendy.su@sony.com>
---
 tests/generic/730 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/generic/730 b/tests/generic/730
index 6251980e..6b5d3196 100755
--- a/tests/generic/730
+++ b/tests/generic/730
@@ -29,6 +29,7 @@ _require_scsi_debug
 size=$(_small_fs_size_mb 256)
 SCSI_DEBUG_DEV=`_get_scsi_debug_dev 512 512 0 $size`
 test -b "$SCSI_DEBUG_DEV" || _notrun "Failed to initialize scsi debug device"
+SCRATCH_DEV=$SCSI_DEBUG_DEV _require_scratch_shutdown
 echo "SCSI debug device $SCSI_DEBUG_DEV" >>$seqres.full
 
 run_check _mkfs_dev $SCSI_DEBUG_DEV
-- 
2.25.1

Re: [PATCH v1 7/7] generic/633: add test requirements

[Zorro Lang]

On Fri, Jun 06, 2025 at 02:31:52AM +0000, Yuezhang.Mo@sony.com wrote:
> > On Wed, May 28, 2025 at 12:56:55PM +0200, Amir Goldstein wrote:
> > > On Wed, May 28, 2025 at 11:15 AM Yuezhang Mo <Yuezhang.Mo@sony.com> wrote:
> > > >
> > > > Since the following test requirements are required by the following
> > > > sub-tests, this commit adds them to this test to avoid failures on
> > > > filesystems that do not support them.
> > > >
> > > > requirement | sub-tests
> > > > ------------+-------------------------------------------------------
> > > > hardlink    | hardlink_crossing_mounts tcore_fsids_mapped
> > > >             | tcore_hardlink_crossing_idmapped_mounts
> > > >             | tcore_hardlink_from_idmapped_mount
> > > >             | tcore_hardlink_from_idmapped_mount_in_userns
> > > > ------------|-------------------------------------------------------
> > > > symlink     | protected_symlinks symlink_regular_mounts
> > > >             | tcore_create_in_userns tcore_symlink_idmapped_mounts
> > > >             | tcore_symlink_idmapped_mounts_in_userns
> > > > ------------|-------------------------------------------------------
> > > > chown       | hardlink_crossing_mounts rename_crossing_mounts
> > > >             | setid_binaries sticky_bit_unlink
> > > >             | symlink_regular_mounts tcore_acls
> > > >             | tcore_hardlink_crossing_idmapped_mounts
> > > >             | tcore_hardlink_from_idmapped_mount
> > > >             | tcore_protected_symlinks_idmapped_mounts
> > > >             | tcore_rename_crossing_idmapped_mounts
> > > >             | tcore_rename_from_idmapped_mount
> > > >             | tcore_setattr_truncate_idmapped_in_userns
> > > >             | tcore_setid_binaries_idmapped_mounts
> > > >             | tcore_setid_binaries_idmapped_mounts_in_userns
> > > >             | tcore_setid_binaries_idmapped_mounts_in_userns_separate_userns
> > > >             | tcore_sticky_bit_unlink_idmapped_mounts
> > > >             | tcore_sticky_bit_unlink_idmapped_mounts_in_userns
> > > >             | tcore_sticky_bit_rename_idmapped_mounts
> > > >             | tcore_sticky_bit_rename_idmapped_mounts_in_userns
> > > > ------------|-------------------------------------------------------
> > > > chmod       | protected_symlinks
> > > >             | tcore_protected_symlinks_idmapped_mounts_in_userns
> > > > ------------|-------------------------------------------------------
> > > > io_uring    | io_uring io_uring_userns tcore_io_uring_idmapped_userns
> > > >             | tcore_io_uring_idmapped_unmapped
> > > >             | tcore_io_uring_idmapped_unmapped_userns
> > > > ------------|-------------------------------------------------------
> > > > mknod       | tcore_expected_uid_gid_idmapped_mounts
> > > >
> > > > Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> > > > Reviewed-by: Friendy Su <friendy.su@sony.com>
> > > > ---
> > > >  tests/generic/633 | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/tests/generic/633 b/tests/generic/633
> > > > index f58dbbf5..e19a2043 100755
> > > > --- a/tests/generic/633
> > > > +++ b/tests/generic/633
> > > > @@ -13,6 +13,12 @@ _begin_fstest auto quick atime attr cap idmapped io_uring mount perms rw unlink
> > > >  . ./common/filter
> > > >
> > > >  _require_idmapped_mounts
> > >
> > > Please note that the requirement above is wrong:
> > >
> > > https://lore.kernel.org/fstests/20250526175437.1528310-1-amir73il@gmail.com/
> > >
> > > > +_require_hardlinks
> > > > +_require_symlinks
> > > > +_require_chown
> > > > +_require_chmod
> > > > +_require_io_uring
> > > > +_require_mknod
> > >
> > > I wonder if it makes more sense to have a collective check for all (or
> > > most) of the above,
> > > e.g. _require_vfstest, which is different than _require_idmapped_mounts,
> > > so that we do not have to add all of these for tests running
> > > other vfstests subtests in the future.
> > 
> > Yeah, and some _require_ looks optional, e.g. _require_io_uring, why is it a
> > necessary requirement, we even run xfstests with io_uring. If so, we don't do
> > vfstest if io_uring isn't supported?
> 
> OK, I will removed '_require_io_uring'.
> 
> > 
> > Although we can add as many as possible _require_* checks to a test case, but
> > more _require_* take more time to run. So I think we'd better to add "missed"
> > _require_* for specific test failure on someone fs, not only "I reviewed the
> > code, so I feel all of these are needed"
> 
> Do you mean we should check these requirements only for some specific fs?
> Like this.
> 
> if [ "${FSTYP}" == "exfat" ]; then
>       _require_hardlinks
>       _require_symlinks
>       _require_chown
>       _require_chmod
>      _require_mknod
> fi

No, of course not :)

I mean if one case *can* have 100 _require_ conditions in theory, we don't need to
write all 100 _require_ conditions.

As you've removed _require_io_uring, I hope you can make sure if there're more
"optional" _require_ conditions. On the other words, if there're some tests
might be run or might not (likes io_uring).

Thanks,
Zorro

> 
> I think this is not good, firstly because it is hard to maintain(other fs may also
> need these checks), and secondly because checking these requirements does
> not take much time (in my environment it takes less than a second).
> 
> 
> 

Re: [PATCH v1 7/7] generic/633: add test requirements

[Yuezhang.Mo]

> > > Although we can add as many as possible _require_* checks to a test case, but
> > > more _require_* take more time to run. So I think we'd better to add "missed"
> > > _require_* for specific test failure on someone fs, not only "I reviewed the
> > > code, so I feel all of these are needed"
> >
> > Do you mean we should check these requirements only for some specific fs?
> > Like this.
> >
> > if [ "${FSTYP}" == "exfat" ]; then
> >       _require_hardlinks
> >       _require_symlinks
> >       _require_chown
> >       _require_chmod
> >      _require_mknod
> > fi
> 
> No, of course not :)
> 
> I mean if one case *can* have 100 _require_ conditions in theory, we don't need to
> write all 100 _require_ conditions.

The optional _require_ condition is different on different file systems.
For exfat, this test fails with the following error, adding _require_chown
is enough, the others are optional.

+vfstest.c: 110: hardlink_crossing_mounts - Operation not permitted - failure: chown_r
+vfstest.c: 2418: run_test - Operation not permitted - failure: cross mount hardlink

I will update this patch to only add _require_chown. If someone in the future
finds that one filesystem supports chown but not others, he can create a new
patch for it.