diff mbox series

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

Message ID 6ace53b5d32b7814cf4770bcb5bb1d2f4287d490.1687242517.git.anand.jain@oracle.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: tests: fix no acl support | expand

Commit Message

Anand Jain June 20, 2023, 8:49 a.m. UTC
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(+)

Comments

David Sterba June 21, 2023, 2:56 p.m. UTC | #1
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.
Anand Jain June 22, 2023, 8:11 a.m. UTC | #2
>> +# 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
diff mbox series

Patch

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...]