[v4] syscalls/statx04: use stx_attributes_mask before test

stx_attributes_mask shows what's supported in stx_attributes.
Set supp_{append,compr,immutable,nodump} attributes only on filesystems
which actually support it.

Also merge duplicate code and document the kernel commits that ext2/ext4/xfs/btrfs
supports statx syscall correctly.

Reviewed-by: Li Wang <liwang@redhat.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
v3->v4:
1. add kernel commits into top description
2. docparse formatting

v2->v3:
1.add kernel version check for stx_attributes_mask
2. use test_flag(int) instead of test_flagged and test_unflagged

 testcases/kernel/syscalls/statx/statx04.c | 197 ++++++++++++++--------
 1 file changed, 123 insertions(+), 74 deletions(-)

Hello Yang,

Yang Xu <xuyang2018.jy@fujitsu.com> writes:

> stx_attributes_mask shows what's supported in stx_attributes.
> Set supp_{append,compr,immutable,nodump} attributes only on filesystems
> which actually support it.
>
> Also merge duplicate code and document the kernel commits that ext2/ext4/xfs/btrfs
> supports statx syscall correctly.
>
> Reviewed-by: Li Wang <liwang@redhat.com>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
> v3->v4:
> 1. add kernel commits into top description
> 2. docparse formatting

Note that I also suggested NOT checking the kernel version. Because
these patches are likely to get backported.

Please see comments below.

>
> v2->v3:
> 1.add kernel version check for stx_attributes_mask
> 2. use test_flag(int) instead of test_flagged and test_unflagged
>
>  testcases/kernel/syscalls/statx/statx04.c | 197 ++++++++++++++--------
>  1 file changed, 123 insertions(+), 74 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/statx/statx04.c b/testcases/kernel/syscalls/statx/statx04.c
> index f8350ed2d..3c221b461 100644
> --- a/testcases/kernel/syscalls/statx/statx04.c
> +++ b/testcases/kernel/syscalls/statx/statx04.c
> @@ -4,21 +4,46 @@
>   * Email: code@zilogic.com
>   */
>  
> -/*
> - * Test statx
> +/*\
> + * [Description]
>   *
>   * This code tests if the attributes field of statx received expected value.
>   * File set with following flags by using SAFE_IOCTL:
> - * 1) STATX_ATTR_COMPRESSED - The file is compressed by the filesystem.
> - * 2) STATX_ATTR_IMMUTABLE - The file cannot be modified.
> - * 3) STATX_ATTR_APPEND - The file can only be opened in append mode for
> - *                        writing.
> - * 4) STATX_ATTR_NODUMP - File is not a candidate for backup when a backup
> + *
> + * - STATX_ATTR_COMPRESSED - The file is compressed by the filesystem.
> + * - STATX_ATTR_IMMUTABLE - The file cannot be modified.
> + * - STATX_ATTR_APPEND - The file can only be opened in append mode for writing.
> + * - STATX_ATTR_NODUMP - File is not a candidate for backup when a backup
>   *                        program such as dump(8) is run.
>   *
>   * Two directories are tested.
> - * First directory has all flags set.
> - * Second directory has no flags set.
> + * First directory has all flags set. Second directory has no flags set.
> + *
> + * ext2, ext4, btrfs and xfs support statx syscall since the following commit
> + *
> + *  commit 93bc420ed41df63a18ae794101f7cbf45226a6ef
> + *  Author: yangerkun <yangerkun@huawei.com>
> + *  Date:   Mon Feb 18 09:07:02 2019 +0800
> + *
> + *  ext2: support statx syscall
> + *
> + *  commit 99652ea56a4186bc5bf8a3721c5353f41b35ebcb
> + *  Author: David Howells <dhowells@redhat.com>
> + *  Date:   Fri Mar 31 18:31:56 2017 +0100
> + *
> + *  ext4: Add statx support
> + *
> + *  commit 04a87e3472828f769a93655d7c64a27573bdbc2c
> + *  Author: Yonghong Song <yhs@fb.com>
> + *  Date:   Fri May 12 15:07:43 2017 -0700
> + *
> + *  Btrfs: add statx support
> + *
> + *  commit 1b9598c8fb9965fff901c4caa21fed9644c34df3
> + *  Author: Luis R. Rodriguez <mcgrof@kernel.org>
> + *  Date:   Fri Mar 1 08:14:57 2019 -0800
> + *
> + *  xfs: fix reporting supported extra file attributes for statx()

This is good. However I also meant that you can add the commit tag to
tst_test. E.g. from the cve-2014-0196 testcase

static struct tst_test test = {
	.setup = setup,
	.cleanup = cleanup,
	.test_all = run,
	.tags = (const struct tst_tag[]) {
		{"linux-git", "4291086b1f08"},
		{"CVE", "2014-0196"},
		{}
	}
};


>   *
>   * Minimum kernel version required is 4.11.
>   */
> @@ -34,85 +59,67 @@
>  #define TESTDIR_UNFLAGGED MOUNT_POINT"/test_dir2"
>  
>  static int fd, clear_flags;
> +static int supp_compr = 1, supp_append = 1, supp_immutable = 1, supp_nodump = 1;
>  
> -static void test_flagged(void)
> -{
> -	struct statx buf;
> -
> -	TEST(statx(AT_FDCWD, TESTDIR_FLAGGED, 0, 0, &buf));
> -	if (TST_RET == 0)
> -		tst_res(TPASS,
> -			"sys_statx(AT_FDCWD, %s, 0, 0, &buf)", TESTDIR_FLAGGED);
> -	else
> -		tst_brk(TFAIL | TTERRNO,
> -			"sys_statx(AT_FDCWD, %s, 0, 0, &buf)", TESTDIR_FLAGGED);
> -
> -	if (buf.stx_attributes & STATX_ATTR_COMPRESSED)
> -		tst_res(TPASS, "STATX_ATTR_COMPRESSED flag is set");
> -	else
> -		tst_res(TFAIL, "STATX_ATTR_COMPRESSED flag is not set");
> -
> -	if (buf.stx_attributes & STATX_ATTR_APPEND)
> -		tst_res(TPASS, "STATX_ATTR_APPEND flag is set");
> -	else
> -		tst_res(TFAIL, "STATX_ATTR_APPEND flag is not set");
> -
> -	if (buf.stx_attributes & STATX_ATTR_IMMUTABLE)
> -		tst_res(TPASS, "STATX_ATTR_IMMUTABLE flag is set");
> -	else
> -		tst_res(TFAIL, "STATX_ATTR_IMMUTABLE flag is not set");
> -
> -	if (buf.stx_attributes & STATX_ATTR_NODUMP)
> -		tst_res(TPASS, "STATX_ATTR_NODUMP flag is set");
> -	else
> -		tst_res(TFAIL, "STATX_ATTR_NODUMP flag is not set");
> -}
> -
> -static void test_unflagged(void)
> +static void test_flag(int flag)
>  {
>  	struct statx buf;
>  
> -	TEST(statx(AT_FDCWD, TESTDIR_UNFLAGGED, 0, 0, &buf));
> +	TEST(statx(AT_FDCWD, flag ? TESTDIR_FLAGGED : TESTDIR_UNFLAGGED, 0, 0, &buf));
>  	if (TST_RET == 0)
>  		tst_res(TPASS,
>  			"sys_statx(AT_FDCWD, %s, 0, 0, &buf)",
> -			TESTDIR_UNFLAGGED);
> +			flag ? TESTDIR_FLAGGED : TESTDIR_UNFLAGGED);
>  	else
>  		tst_brk(TFAIL | TTERRNO,
>  			"sys_statx(AT_FDCWD, %s, 0, 0, &buf)",
> -			TESTDIR_UNFLAGGED);
> -
> -	if ((buf.stx_attributes & STATX_ATTR_COMPRESSED) == 0)
> -		tst_res(TPASS, "STATX_ATTR_COMPRESSED flag is not set");
> -	else
> -		tst_res(TFAIL, "STATX_ATTR_COMPRESSED flag is set");
> -
> -	if ((buf.stx_attributes & STATX_ATTR_APPEND) == 0)
> -		tst_res(TPASS, "STATX_ATTR_APPEND flag is not set");
> -	else
> -		tst_res(TFAIL, "STATX_ATTR_APPEND flag is set");
> -
> -	if ((buf.stx_attributes & STATX_ATTR_IMMUTABLE) == 0)
> -		tst_res(TPASS, "STATX_ATTR_IMMUTABLE flag is not set");
> -	else
> -		tst_res(TFAIL, "STATX_ATTR_IMMUTABLE flag is set");
> -
> -	if ((buf.stx_attributes & STATX_ATTR_NODUMP) == 0)
> -		tst_res(TPASS, "STATX_ATTR_NODUMP flag is not set");
> -	else
> -		tst_res(TFAIL, "STATX_ATTR_NODUMP flag is set");
> +			flag ? TESTDIR_FLAGGED : TESTDIR_UNFLAGGED);
> +
> +	if (supp_compr) {
> +		if (buf.stx_attributes & STATX_ATTR_COMPRESSED)
> +			tst_res(flag ? TPASS : TFAIL,
> +				"STATX_ATTR_COMPRESSED flag is set");
> +		else
> +			tst_res(flag ? TFAIL : TPASS,
> +				"STATX_ATTR_COMPRESSED flag is not set");
> +	}
> +	if (supp_append) {
> +		if (buf.stx_attributes & STATX_ATTR_APPEND)
> +			tst_res(flag ? TPASS : TFAIL,
> +				"STATX_ATTR_APPEND flag is set");
> +		else
> +			tst_res(flag ? TFAIL : TPASS,
> +				"STATX_ATTR_APPEND flag is not set");
> +	}
> +	if (supp_immutable) {
> +		if (buf.stx_attributes & STATX_ATTR_IMMUTABLE)
> +			tst_res(flag ? TPASS : TFAIL,
> +				"STATX_ATTR_IMMUTABLE flag is set");
> +		else
> +			tst_res(flag ? TFAIL : TPASS,
> +				"STATX_ATTR_IMMUTABLE flag is not set");
> +	}
> +	if (supp_nodump) {
> +		if (buf.stx_attributes & STATX_ATTR_NODUMP)
> +			tst_res(flag ? TPASS : TFAIL,
> +				"STATX_ATTR_NODUMP flag is set");
> +		else
> +			tst_res(flag ? TFAIL : TPASS,
> +				"STATX_ATTR_NODUMP flag is not set");
> +	}
>  }
>  
>  struct test_cases {
> -	void (*tfunc)(void);
> +	void (*tfunc)(int);
> +	int set_flag;
>  } tcases[] = {
> -	{&test_flagged},
> -	{&test_unflagged},
> +	{&test_flag, 1},
> +	{&test_flag, 0},
>  };
>  
>  static void run(unsigned int i)
>  {
> -	tcases[i].tfunc();
> +	tcases[i].tfunc(tcases[i].set_flag);
>  }
>  
>  static void caid_flags_setup(void)
> @@ -135,12 +142,17 @@ static void caid_flags_setup(void)
>  		tst_brk(TBROK | TERRNO, "ioctl(%i, FS_IOC_GETFLAGS, ...)", fd);
>  	}
>  
> -	attr |= FS_COMPR_FL | FS_APPEND_FL | FS_IMMUTABLE_FL | FS_NODUMP_FL;
> +	if (supp_compr)
> +		attr |= FS_COMPR_FL;
> +	if (supp_append)
> +		attr |= FS_APPEND_FL;
> +	if (supp_immutable)
> +		attr |= FS_IMMUTABLE_FL;
> +	if (supp_nodump)
> +		attr |= FS_NODUMP_FL;
>  
>  	ret = ioctl(fd, FS_IOC_SETFLAGS, &attr);
>  	if (ret < 0) {
> -		if (errno == EOPNOTSUPP)
> -			tst_brk(TCONF, "Flags not supported");
>  		tst_brk(TBROK | TERRNO, "ioctl(%i, FS_IOC_SETFLAGS, %i)", fd, attr);
>  	}
>  
> @@ -149,11 +161,48 @@ static void caid_flags_setup(void)
>  
>  static void setup(void)
>  {
> +	struct statx buf;
> +
>  	SAFE_MKDIR(TESTDIR_FLAGGED, 0777);
>  	SAFE_MKDIR(TESTDIR_UNFLAGGED, 0777);

So we create the dirs here. I assume they are created with all blank
flags otherwise the existing test would be broken.

>  
> +	/* Don't check ext4 because ext4 supports statx since 4.11. */
>  	if (!strcmp(tst_device->fs_type, "btrfs") && tst_kvercmp(4, 13, 0) < 0)
> -		tst_brk(TCONF, "Btrfs statx() supported since 4.13");
> +		tst_brk(TCONF,
> +			"Btrfs statx() stx_attributes_mask supported since 4.13, see test description!");
> +
> +	if (!strcmp(tst_device->fs_type, "xfs") && tst_kvercmp(5, 1, 0) < 0)
> +		tst_brk(TCONF,
> +			"xfs statx() stx_attributes_mask supported since 5.1, see test description!");
> +
> +	if (!strcmp(tst_device->fs_type, "ext2") && tst_kvercmp(5, 1, 0) < 0)
> +		tst_brk(TCONF,
> +			"ext2 statx() stx_attributes_mask supported since 5.1, see test description!");
> +
> +	TEST(statx(AT_FDCWD, TESTDIR_FLAGGED, 0, 0, &buf));

Then we get the flags here. They should all be blank.

> +	if (TST_RET == -1)
> +		tst_brk(TFAIL | TTERRNO,
> +			"sys_statx(AT_FDCWD, %s, 0, 0, &buf)", TESTDIR_FLAGGED);
> +
> +	if ((buf.stx_attributes_mask & FS_COMPR_FL) == 0) {
> +		supp_compr = 0;
> +		tst_res(TCONF, "filesystem doesn't support FS_COMPR_FL");
> +	}
> +	if ((buf.stx_attributes_mask & FS_APPEND_FL) == 0) {
> +		supp_append = 0;
> +		tst_res(TCONF, "filesystem doesn't support FS_APPEND_FL");
> +	}
> +	if ((buf.stx_attributes_mask & FS_IMMUTABLE_FL) == 0) {
> +		supp_immutable = 0;
> +		tst_res(TCONF, "filesystem doesn't support FS_IMMUTABLE_FL");
> +	}
> +	if ((buf.stx_attributes_mask & FS_NODUMP_FL) == 0) {
> +		supp_nodump = 0;
> +		tst_res(TCONF, "filesystem doesn't support FS_NODUMP_FL");
> +	}
> +	if (!(supp_compr || supp_append || supp_immutable || supp_nodump))
> +		tst_brk(TCONF,
> +			"filesystem doesn't support the above any attr,
> skip it");

So this will always fail with TCONF?

>  
>  	caid_flags_setup();

And then we set the flags here?

Note that even if you move setting the flags to the top. This will still
result in all unset flags becoming TCONF instead of TFAIL.

IMO statx is broken on older kernels except for ext4. It can be fixed by
backporting patches. One of the main use cases for LTP is to find
missing backports.

Hi!
> Note that even if you move setting the flags to the top. This will still
> result in all unset flags becoming TCONF instead of TFAIL.
> 
> IMO statx is broken on older kernels except for ext4. It can be fixed by
> backporting patches. One of the main use cases for LTP is to find
> missing backports.

That was my point as well.

I guess that the last time I've proposed to create a separate test that
just tests that these flags are set as expected for a all filesystems
and that test would have the patches that fixed that as a tags set. That
would make it clearer what is wrong and what should be backported.

Once we have that test implemented we can change this test as this patch
does.

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> Note that even if you move setting the flags to the top. This will still
>> result in all unset flags becoming TCONF instead of TFAIL.
>> 
>> IMO statx is broken on older kernels except for ext4. It can be fixed by
>> backporting patches. One of the main use cases for LTP is to find
>> missing backports.
>
> That was my point as well.
>
> I guess that the last time I've proposed to create a separate test that
> just tests that these flags are set as expected for a all filesystems
> and that test would have the patches that fixed that as a tags set. That
> would make it clearer what is wrong and what should be backported.
>
> Once we have that test implemented we can change this test as this patch
> does.

Or perhaps we could pass a flag to this test to ignore kernel versions
which are known not to work?