[PATCH 0/6] btrfs-progs: tests: fix no acl support

[PATCH 0/6] btrfs-progs: tests: fix no acl support

[Anand Jain]

Btrfs ACL support is a compiling time optional feature, instead of
failing the test cases, let them say notrun. This set fixes it.

Patch 1/6 is preparatory adds helper to check for btrfs acl support.
Patches 2-6/6 are actual fixes.

Thanks.

Anand Jain (6):
  btrfs-progs: tests: add helper check_prereq_btrfsacl
  btrfs-progs: tests: misc/057-btrfstune-free-space-tree check for btrfs
    acl support
  btrfs-progs: tests: convert/001-ext2-basic check for btrfs acl support
  btrfs-progs: tests: convert/003-ext4-basic check for btrfs acl support
  btrfs-progs: tests: convert/005-delete-all-rollback check for btrfs
    acl support
  btrfs-progs: tests: convert/006-large-hole-extent check for btrfs acl
    support

 tests/common                                        | 13 +++++++++++++
 tests/convert-tests/001-ext2-basic/test.sh          |  1 +
 tests/convert-tests/003-ext4-basic/test.sh          |  1 +
 tests/convert-tests/005-delete-all-rollback/test.sh |  1 +
 tests/convert-tests/006-large-hole-extent/test.sh   |  1 +
 .../057-btrfstune-free-space-tree/test.sh           |  1 +
 6 files changed, 18 insertions(+)

-- 
2.31.1

[PATCH 1/6] btrfs-progs: tests: add helper check_prereq_btrfsacl

[Anand Jain]

Some test cases are failing because acl is not compiled on the system.
Instead, they should be marked as 'not_run'.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/common | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/common b/tests/common
index d985ef72a4f1..0c75d5d76c44 100644
--- a/tests/common
+++ b/tests/common
@@ -575,6 +575,19 @@ setup_root_helper()
 	SUDO_HELPER=root_helper
 }
 
+# Check if btrfs is compiled with CONFIG_BTRFS_FS_POSIX_ACL, requires TEST_DEV.
+check_prereq_btrfsacl()
+{
+	run_check_mkfs_test_dev
+	run_check_mount_test_dev
+
+	if grep "$TEST_MNT" /proc/self/mounts | grep -q noacl; then
+		run_check_umount_test_dev
+		_not_run "acl not compiled"
+	fi
+	run_check_umount_test_dev
+}
+
 prepare_test_dev()
 {
 	# num[K/M/G/T...]
-- 
2.31.1

[PATCH 2/6] btrfs-progs: tests: misc/057-btrfstune-free-space-tree check for btrfs acl support

[Anand Jain]

Fix failure due to no acl support in btrfs.

  $ make test
    ::
    [TEST/misc]   057-btrfstune-free-space-tree
    failed: setfacl -m u:root:x /Volumes/ws/btrfs-progs/tests/mnt/acls/acls.1
    test failed for case 057-btrfstune-free-space-tree
    make: *** [Makefile:493: test-misc] Error 1

Instead, use check_prereq_btrfsacl() to call _not_run().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/misc-tests/057-btrfstune-free-space-tree/test.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/misc-tests/057-btrfstune-free-space-tree/test.sh b/tests/misc-tests/057-btrfstune-free-space-tree/test.sh
index 93ff4307fca9..d91fe2588c93 100755
--- a/tests/misc-tests/057-btrfstune-free-space-tree/test.sh
+++ b/tests/misc-tests/057-btrfstune-free-space-tree/test.sh
@@ -10,6 +10,7 @@ check_prereq btrfs
 
 setup_root_helper
 prepare_test_dev
+check_prereq_btrfsacl
 
 run_check_mkfs_test_dev -O ^free-space-tree
 run_check_mount_test_dev
-- 
2.31.1

[PATCH 3/6] btrfs-progs: tests: convert/001-ext2-basic check for btrfs acl support

[Anand Jain]

Fix failure due to no acl support in btrfs.

  $ make test
  ::
    [TEST/conv]   001-ext2-basic
    [TEST/conv]     ext2 4k nodesize, btrfs defaults
  failed: setfacl -m u:root:x /Volumes/ws/btrfs-progs/tests/mnt/acls/acls.1
  test failed for case 001-ext2-basic
  make: *** [Makefile:477: test-convert] Error 1

Instead, use check_prereq_btrfsacl() to call _not_run().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/convert-tests/001-ext2-basic/test.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/convert-tests/001-ext2-basic/test.sh b/tests/convert-tests/001-ext2-basic/test.sh
index 4e8aaecb2f55..4666a83a24b3 100755
--- a/tests/convert-tests/001-ext2-basic/test.sh
+++ b/tests/convert-tests/001-ext2-basic/test.sh
@@ -8,6 +8,7 @@ check_global_prereq mke2fs
 
 setup_root_helper
 prepare_test_dev
+check_prereq_btrfsacl
 
 # Iterate over defaults and options that are not tied to hardware capabilities
 # or number of devices
-- 
2.31.1

[PATCH 4/6] btrfs-progs: tests: convert/003-ext4-basic check for btrfs acl support

[Anand Jain]

Fix failure due to no acl support in btrfs.

  $ make test
  ::
  [TEST/conv]   003-ext4-basic
  [TEST/conv]     ext4 4k nodesize, btrfs defaults
  failed: /Volumes/ws/btrfs-progs/btrfs inspect-internal dump-super -Ffa /Volumes/ws/btrfs-progs/tests/test.img
  test failed for case 003-ext4-basic
  make: *** [Makefile:477: test-convert] Error 1

Instead, use check_prereq_btrfsacl() to call _not_run().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/convert-tests/003-ext4-basic/test.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/convert-tests/003-ext4-basic/test.sh b/tests/convert-tests/003-ext4-basic/test.sh
index 8a0b2e333554..911d03006be6 100755
--- a/tests/convert-tests/003-ext4-basic/test.sh
+++ b/tests/convert-tests/003-ext4-basic/test.sh
@@ -8,6 +8,7 @@ check_global_prereq mke2fs
 
 setup_root_helper
 prepare_test_dev
+check_prereq_btrfsacl
 
 # Iterate over defaults and options that are not tied to hardware capabilities
 # or number of devices
-- 
2.31.1

[PATCH 5/6] btrfs-progs: tests: convert/005-delete-all-rollback check for btrfs acl support

[Anand Jain]

Fix failure due to no acl support in btrfs.

  $ make test
  ::
  [TEST/conv]   005-delete-all-rollback
  [TEST/conv]     ext4 4k nodesize, btrfs defaults
  failed: /Volumes/ws/btrfs-progs/btrfs inspect-internal dump-super -Ffa /Volumes/ws/btrfs-progs/tests/test.img
  test failed for case 005-delete-all-rollback
  make: *** [Makefile:477: test-convert] Error 1

Instead, use check_prereq_btrfsacl() to call _not_run().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/convert-tests/005-delete-all-rollback/test.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/convert-tests/005-delete-all-rollback/test.sh b/tests/convert-tests/005-delete-all-rollback/test.sh
index 0b668a6fc1de..02b53bac58a6 100755
--- a/tests/convert-tests/005-delete-all-rollback/test.sh
+++ b/tests/convert-tests/005-delete-all-rollback/test.sh
@@ -10,6 +10,7 @@ check_global_prereq mke2fs
 
 setup_root_helper
 prepare_test_dev
+check_prereq_btrfsacl
 
 # simple wrapper for a convert test
 # $1: btrfs features, argument to -O
-- 
2.31.1

[PATCH 6/6] btrfs-progs: tests: convert/006-large-hole-extent check for btrfs acl support

[Anand Jain]

Fix failure due to no acl support in btrfs.

  $ make test
    [TEST/conv]   006-large-hole-extent
    [TEST/conv]     large hole extent test, btrfs defaults
    failed: /Volumes/ws/btrfs-progs/btrfs inspect-internal dump-super -Ffa /Volumes/ws/btrfs-progs/tests/test.img
    test failed for case 006-large-hole-extent
    make: *** [Makefile:477: test-convert] Error 1

Instead, use check_prereq_btrfsacl() to call _not_run().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/convert-tests/006-large-hole-extent/test.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/convert-tests/006-large-hole-extent/test.sh b/tests/convert-tests/006-large-hole-extent/test.sh
index 872b4a6f6175..d7007b705862 100755
--- a/tests/convert-tests/006-large-hole-extent/test.sh
+++ b/tests/convert-tests/006-large-hole-extent/test.sh
@@ -13,6 +13,7 @@ check_global_prereq mke2fs
 
 setup_root_helper
 prepare_test_dev
+check_prereq_btrfsacl
 
 default_mke2fs="mke2fs -t ext4 -b 4096"
 convert_test_preamble '' 'large hole extent test' 16k "$default_mke2fs"
-- 
2.31.1

Re: [PATCH 1/6] btrfs-progs: tests: add helper check_prereq_btrfsacl

[David Sterba]

On Tue, Jun 20, 2023 at 04:49:57PM +0800, Anand Jain wrote:
> Some test cases are failing because acl is not compiled on the system.
> Instead, they should be marked as 'not_run'.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  tests/common | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/tests/common b/tests/common
> index d985ef72a4f1..0c75d5d76c44 100644
> --- a/tests/common
> +++ b/tests/common
> @@ -575,6 +575,19 @@ setup_root_helper()
>  	SUDO_HELPER=root_helper
>  }
>  
> +# Check if btrfs is compiled with CONFIG_BTRFS_FS_POSIX_ACL, requires TEST_DEV.
> +check_prereq_btrfsacl()

The _prereq_ helpers are supposed to check utilities or modules, the
kernel features are named like check_kernel_support_FEATURE.

> +{
> +	run_check_mkfs_test_dev
> +	run_check_mount_test_dev
> +
> +	if grep "$TEST_MNT" /proc/self/mounts | grep -q noacl; then

So this relies on the fact that if ACLs are not compiled in a mounted
filesystem will add noacl to the mount options. This works though
requires a mount. Another option is to grep /proc/config.gz, or
eventually rely on the sysfs export.

That it needs mount can be problematic on kernels with unsupported
features built by default, like eventually block-group-tree. This would
be rare but also would fail the testsuite. The CI environments do not
always have most up-to-date kernels while part of the test suite can
still run.

We can do the mkfs/mount check with some caution, though I don't like it
much.

Re: [PATCH 2/6] btrfs-progs: tests: misc/057-btrfstune-free-space-tree check for btrfs acl support

[David Sterba]

On Tue, Jun 20, 2023 at 04:49:58PM +0800, Anand Jain wrote:
> Fix failure due to no acl support in btrfs.
> 
>   $ make test
>     ::
>     [TEST/misc]   057-btrfstune-free-space-tree
>     failed: setfacl -m u:root:x /Volumes/ws/btrfs-progs/tests/mnt/acls/acls.1
>     test failed for case 057-btrfstune-free-space-tree
>     make: *** [Makefile:493: test-misc] Error 1
> 
> Instead, use check_prereq_btrfsacl() to call _not_run().
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  tests/misc-tests/057-btrfstune-free-space-tree/test.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/misc-tests/057-btrfstune-free-space-tree/test.sh b/tests/misc-tests/057-btrfstune-free-space-tree/test.sh
> index 93ff4307fca9..d91fe2588c93 100755
> --- a/tests/misc-tests/057-btrfstune-free-space-tree/test.sh
> +++ b/tests/misc-tests/057-btrfstune-free-space-tree/test.sh
> @@ -10,6 +10,7 @@ check_prereq btrfs
>  
>  setup_root_helper
>  prepare_test_dev
> +check_prereq_btrfsacl

You can do all the test case changes in one patch, the error messages
are basically the same.

Re: [PATCH 1/6] btrfs-progs: tests: add helper check_prereq_btrfsacl

[Anand Jain]

>> +# Check if btrfs is compiled with CONFIG_BTRFS_FS_POSIX_ACL, requires TEST_DEV.
>> +check_prereq_btrfsacl()
> 
> The _prereq_ helpers are supposed to check utilities or modules, the
> kernel features are named like check_kernel_support_FEATURE.

Ah. That's a clean thumb rule. I will fix it.

> 
>> +{
>> +	run_check_mkfs_test_dev
>> +	run_check_mount_test_dev
>> +
>> +	if grep "$TEST_MNT" /proc/self/mounts | grep -q noacl; then
> 
> So this relies on the fact that if ACLs are not compiled in a mounted
> filesystem will add noacl to the mount options. This works though
> requires a mount.

Yep.

>Another option is to grep /proc/config.gz, or

/proc/config.gz is optional file as well, so we can check ACL
support first in /proc/config.gz and then fallback to the other
methods. Will add it.

> eventually rely on the sysfs export.

Yes.

> That it needs mount can be problematic on kernels with unsupported
> features built by default, like eventually block-group-tree. This would
> be rare but also would fail the testsuite. The CI environments do not
> always have most up-to-date kernels while part of the test suite can
> still run.
> 
> We can do the mkfs/mount check with some caution, though I don't like it
> much.

There isn't much choice; using the mkfs/mount to confirm ACL support
should be the last method IMO, and hopefully, in the long run, the
sysfs method will make other methods redundant.



Thanks for the comments.
Anand

Re: [PATCH 2/6] btrfs-progs: tests: misc/057-btrfstune-free-space-tree check for btrfs acl support

[Anand Jain]

> 
> You can do all the test case changes in one patch, the error messages
> are basically the same.

Yeah, will merge them into one patch.

Thanks, Anand