All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] syscalls/statx04: use stx_attributes_mask before test
@ 2019-08-02 10:34 Yang Xu
  2019-08-02 11:50 ` Cyril Hrubis
  0 siblings, 1 reply; 19+ messages in thread
From: Yang Xu @ 2019-08-02 10:34 UTC (permalink / raw)
  To: ltp

stx_attributes_mask shows what's supported in stx_attributes.
we should check four attrbutes whether supports on tested filesystem
and report the non-supported attributes  before test.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 testcases/kernel/syscalls/statx/statx04.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/syscalls/statx/statx04.c b/testcases/kernel/syscalls/statx/statx04.c
index 71de734f5..e4b198c07 100644
--- a/testcases/kernel/syscalls/statx/statx04.c
+++ b/testcases/kernel/syscalls/statx/statx04.c
@@ -138,23 +138,37 @@ static void caid_flags_setup(void)
 	attr |= FS_COMPR_FL | FS_APPEND_FL | FS_IMMUTABLE_FL | FS_NODUMP_FL;
 
 	ret = ioctl(fd, FS_IOC_SETFLAGS, &attr);
-	if (ret < 0) {
-		if (errno == EOPNOTSUPP)
-			tst_brk(TCONF, "Flags not supported");
+	if (ret < 0)
 		tst_brk(TBROK | TERRNO, "ioctl(%i, FS_IOC_SETFLAGS, %i)", fd, attr);
-	}
 
 	clear_flags = 1;
 }
 
 static void setup(void)
 {
+	struct statx buf;
+
 	SAFE_MKDIR(TESTDIR_FLAGGED, 0777);
 	SAFE_MKDIR(TESTDIR_UNFLAGGED, 0777);
 
 	if (!strcmp(tst_device->fs_type, "btrfs") && tst_kvercmp(4, 13, 0) < 0)
 		tst_brk(TCONF, "Btrfs statx() supported since 4.13");
 
+	TEST(statx(AT_FDCWD, TESTDIR_FLAGGED, 0, 0, &buf));
+	if (TST_RET == -1)
+		tst_brk(TFAIL | TTERRNO,
+			"sys_statx(AT_FDCWD, %s, 0, 0, &buf)", TESTDIR_FLAGGED);
+
+	/* Mask to show which attributes are supported on filesystem. */
+	if ((buf.stx_attributes_mask & FS_COMPR_FL) == 0)
+		tst_brk(TCONF, "filesystem doesn't support FS_COMPR_FL");
+	if ((buf.stx_attributes_mask & FS_APPEND_FL) == 0)
+		tst_brk(TCONF, "filesystem doesn't support FS_APPEND_FL");
+	if ((buf.stx_attributes_mask & FS_IMMUTABLE_FL) == 0)
+		tst_brk(TCONF, "filesystem doesn't support FS_IMMUTABLE_FL");
+	if ((buf.stx_attributes_mask & FS_NODUMP_FL) == 0)
+		tst_brk(TCONF, "filesystem doesn't support FS_NODUMP_FL");
+
 	caid_flags_setup();
 }
 
-- 
2.18.1




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

* [LTP] [PATCH] syscalls/statx04: use stx_attributes_mask before test
  2019-08-02 10:34 [LTP] [PATCH] syscalls/statx04: use stx_attributes_mask before test Yang Xu
@ 2019-08-02 11:50 ` Cyril Hrubis
  2019-08-20  6:33   ` [LTP] [PATCH v2] " Yang Xu
  0 siblings, 1 reply; 19+ messages in thread
From: Cyril Hrubis @ 2019-08-02 11:50 UTC (permalink / raw)
  To: ltp

Hi!
> +	/* Mask to show which attributes are supported on filesystem. */
> +	if ((buf.stx_attributes_mask & FS_COMPR_FL) == 0)
> +		tst_brk(TCONF, "filesystem doesn't support FS_COMPR_FL");
> +	if ((buf.stx_attributes_mask & FS_APPEND_FL) == 0)
> +		tst_brk(TCONF, "filesystem doesn't support FS_APPEND_FL");
> +	if ((buf.stx_attributes_mask & FS_IMMUTABLE_FL) == 0)
> +		tst_brk(TCONF, "filesystem doesn't support FS_IMMUTABLE_FL");
> +	if ((buf.stx_attributes_mask & FS_NODUMP_FL) == 0)
> +		tst_brk(TCONF, "filesystem doesn't support FS_NODUMP_FL");

I doubt that all these flags are either set or unset for a given
fileystem, can we rather than this set flags such as supp_compr etc and
disable only tests for a subset of unsupported flags rather than the
whole test?

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH v2] syscalls/statx04: use stx_attributes_mask before test
  2019-08-02 11:50 ` Cyril Hrubis
@ 2019-08-20  6:33   ` Yang Xu
  2019-08-27  9:25     ` Petr Vorel
  0 siblings, 1 reply; 19+ messages in thread
From: Yang Xu @ 2019-08-20  6:33 UTC (permalink / raw)
  To: ltp

stx_attributes_mask shows what's supported in stx_attributes.
we should check four attrbutes whether supports on tested filesystem
and only test supported flags on tested filesystem.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 testcases/kernel/syscalls/statx/statx04.c | 124 ++++++++++++++++------
 1 file changed, 91 insertions(+), 33 deletions(-)

diff --git a/testcases/kernel/syscalls/statx/statx04.c b/testcases/kernel/syscalls/statx/statx04.c
index 71de734f5..7b462c3f9 100644
--- a/testcases/kernel/syscalls/statx/statx04.c
+++ b/testcases/kernel/syscalls/statx/statx04.c
@@ -34,6 +34,10 @@
 #define TESTDIR_UNFLAGGED MOUNT_POINT"/test_dir2"
 
 static int fd, clear_flags;
+static int supp_compr;
+static int supp_append;
+static int supp_immutable;
+static int supp_nodump;
 
 static void test_flagged(void)
 {
@@ -47,25 +51,33 @@ static void test_flagged(void)
 		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 (supp_compr) {
+		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 (supp_append) {
+		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 (supp_immutable) {
+		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");
+	if (supp_nodump) {
+		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)
@@ -82,25 +94,33 @@ static void test_unflagged(void)
 			"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 (supp_compr) {
+		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 (supp_append) {
+		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 (supp_immutable) {
+		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");
+	if (supp_nodump) {
+		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");
+	}
 }
 
 struct test_cases {
@@ -135,7 +155,14 @@ 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) {
@@ -149,12 +176,43 @@ static void caid_flags_setup(void)
 
 static void setup(void)
 {
+	struct statx buf;
+
+	supp_compr = 1;
+	supp_append = 1;
+	supp_immutable = 1;
+	supp_nodump = 1;
 	SAFE_MKDIR(TESTDIR_FLAGGED, 0777);
 	SAFE_MKDIR(TESTDIR_UNFLAGGED, 0777);
 
 	if (!strcmp(tst_device->fs_type, "btrfs") && tst_kvercmp(4, 13, 0) < 0)
 		tst_brk(TCONF, "Btrfs statx() supported since 4.13");
 
+	TEST(statx(AT_FDCWD, TESTDIR_FLAGGED, 0, 0, &buf));
+	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");
+
 	caid_flags_setup();
 }
 
-- 
2.18.1




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

* [LTP] [PATCH v2] syscalls/statx04: use stx_attributes_mask before test
  2019-08-20  6:33   ` [LTP] [PATCH v2] " Yang Xu
@ 2019-08-27  9:25     ` Petr Vorel
  2019-08-27  9:58       ` Petr Vorel
  2019-08-28  3:56       ` [LTP] [PATCH v2] " Yang Xu
  0 siblings, 2 replies; 19+ messages in thread
From: Petr Vorel @ 2019-08-27  9:25 UTC (permalink / raw)
  To: ltp

Hi Yang,

> stx_attributes_mask shows what's supported in stx_attributes.
> we should check four attrbutes whether supports on tested filesystem
typo attrbutes
> and only test supported flags on tested filesystem.

I'd change it to
Set supp_{append,compr,immutable,nodump} attributes only on filesystems which
actually support them.

> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
> ---
>  testcases/kernel/syscalls/statx/statx04.c | 124 ++++++++++++++++------
...

> -	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) {
> @@ -149,12 +176,43 @@ static void caid_flags_setup(void)

Current code...
	if (supp_append)
		attr |= FS_APPEND_FL;
	if (supp_compr)
		attr |= FS_COMPR_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) {
I wonder, if this check is still needed. Probably it's still useful to have
sanity check, but "Flags not supported" has been caught
by supp_{append,compr,immutable,nodump} variables.

		if (errno == EOPNOTSUPP)
			tst_brk(TCONF, "Flags not supported");
		tst_brk(TBROK | TERRNO, "ioctl(%i, FS_IOC_SETFLAGS, %i)", fd, attr);
	}
...

Kind regards,
Petr

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

* [LTP] [PATCH v2] syscalls/statx04: use stx_attributes_mask before test
  2019-08-27  9:25     ` Petr Vorel
@ 2019-08-27  9:58       ` Petr Vorel
  2019-08-27 10:16         ` Cyril Hrubis
  2019-08-28  3:56       ` [LTP] [PATCH v2] " Yang Xu
  1 sibling, 1 reply; 19+ messages in thread
From: Petr Vorel @ 2019-08-27  9:58 UTC (permalink / raw)
  To: ltp

Hi Yang,

also Cyril noted on Github issue #557 [1]:
"However I'm not sure that we should do this since kernel devs declared this to be bug after all."

So maybe we should wait before applying it.

@Cyril: can you please post link to discussion in LKML?

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/issues/557#issuecomment-522593891

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

* [LTP] [PATCH v2] syscalls/statx04: use stx_attributes_mask before test
  2019-08-27  9:58       ` Petr Vorel
@ 2019-08-27 10:16         ` Cyril Hrubis
  2019-09-11 10:22           ` =?unknown-8bit?b?WWFuZy/lvpAg5p2o?=
  0 siblings, 1 reply; 19+ messages in thread
From: Cyril Hrubis @ 2019-08-27 10:16 UTC (permalink / raw)
  To: ltp

Hi!
> also Cyril noted on Github issue #557 [1]:
> "However I'm not sure that we should do this since kernel devs declared this to be bug after all."
> 
> So maybe we should wait before applying it.
> 
> @Cyril: can you please post link to discussion in LKML?

I've talked to Jan Kara in person while developing these tests, so there
is no track of this discussion.

I guess that the best we can do here is to:

* Apply this patch, since the test should generally check only for flags
  that are supported in the bitflag returned by the syscall

* Add another test that checks that these bitflags are enabled for new
  enough kernels for certain set of filesystems

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH v2] syscalls/statx04: use stx_attributes_mask before test
  2019-08-27  9:25     ` Petr Vorel
  2019-08-27  9:58       ` Petr Vorel
@ 2019-08-28  3:56       ` Yang Xu
  1 sibling, 0 replies; 19+ messages in thread
From: Yang Xu @ 2019-08-28  3:56 UTC (permalink / raw)
  To: ltp

on 2019/08/27 17:25, Petr Vorel wrote:

> Hi Yang,
>
>> stx_attributes_mask shows what's supported in stx_attributes.
>> we should check four attrbutes whether supports on tested filesystem
> typo attrbutes
>> and only test supported flags on tested filesystem.
> I'd change it to
> Set supp_{append,compr,immutable,nodump} attributes only on filesystems which
> actually support them.
>
>> Signed-off-by: Yang Xu<xuyang2018.jy@cn.fujitsu.com>
> Reviewed-by: Petr Vorel<pvorel@suse.cz>
Hi Petr

Thanks for your review.

>> ---
>>   testcases/kernel/syscalls/statx/statx04.c | 124 ++++++++++++++++------
> ...
>
>> -	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) {
>> @@ -149,12 +176,43 @@ static void caid_flags_setup(void)
> Current code...
> 	if (supp_append)
> 		attr |= FS_APPEND_FL;
> 	if (supp_compr)
> 		attr |= FS_COMPR_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) {
> I wonder, if this check is still needed. Probably it's still useful to have
> sanity check, but "Flags not supported" has been caught
> by supp_{append,compr,immutable,nodump} variables.
It seems this check is redundant. In principle, if attributes_mask support these flags, the attribute should also
support them. Even though xfs filesystem missed attributes_mask after[1] and doesn't add it until [2]. But we don't
have the situation of having attribute_mask but not having attribute. So I think we can remove it.

[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f955f26f3d42d
[2]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1b9598c8fb9965

Thanks
Yang Xu

> 		if (errno == EOPNOTSUPP)
> 			tst_brk(TCONF, "Flags not supported");
> 		tst_brk(TBROK | TERRNO, "ioctl(%i, FS_IOC_SETFLAGS, %i)", fd, attr);
> 	}
> ...
>
> Kind regards,
> Petr
>
>
> .
>




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

* [LTP] [PATCH v2] syscalls/statx04: use stx_attributes_mask before test
  2019-08-27 10:16         ` Cyril Hrubis
@ 2019-09-11 10:22           ` =?unknown-8bit?b?WWFuZy/lvpAg5p2o?=
  2019-09-11 12:47             ` Cyril Hrubis
  0 siblings, 1 reply; 19+ messages in thread
From: =?unknown-8bit?b?WWFuZy/lvpAg5p2o?= @ 2019-09-11 10:22 UTC (permalink / raw)
  To: ltp


on 2019/08/27 18:16, Cyril Hrubis wrote:

> Hi!
>> also Cyril noted on Github issue #557 [1]:
>> "However I'm not sure that we should do this since kernel devs declared this to be bug after all."
>>
>> So maybe we should wait before applying it.
>>
>> @Cyril: can you please post link to discussion in LKML?
> I've talked to Jan Kara in person while developing these tests, so there
> is no track of this discussion.
>
> I guess that the best we can do here is to:
>
> * Apply this patch, since the test should generally check only for flags
>    that are supported in the bitflag returned by the syscall
>
> * Add another test that checks that these bitflags are enabled for new
>    enough kernels for certain set of filesystems

Hi Cyril

Do you mean use getxattr to ensure bitflags are enable or a functions test?
I am confused.

Thanks
Yang Xu

>


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190911/674e33a1/attachment.htm>

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

* [LTP] [PATCH v2] syscalls/statx04: use stx_attributes_mask before test
  2019-09-11 10:22           ` =?unknown-8bit?b?WWFuZy/lvpAg5p2o?=
@ 2019-09-11 12:47             ` Cyril Hrubis
  2019-09-12  3:28               ` =?unknown-8bit?b?WWFuZy/lvpAg5p2o?=
  2019-10-25  3:53               ` [LTP] [PATCH] " Yang Xu
  0 siblings, 2 replies; 19+ messages in thread
From: Cyril Hrubis @ 2019-09-11 12:47 UTC (permalink / raw)
  To: ltp

Hi!
> Do you mean use getxattr to ensure bitflags are enable or a functions test?
> I am confused.

For a given filesystem the support for filling in these flags was added
at some point to the kernel. If any kernel newer that this version fails
to fill them up it's a bug.

For ext2 it has been added in:

commit 93bc420ed41df63a18ae794101f7cbf45226a6ef
Author: yangerkun <yangerkun@huawei.com>
Date:   Mon Feb 18 09:07:02 2019 +0800

    ext2: support statx syscall

Hence starting kernel 5.0 ext2 (with ext2 driver) has to set the mask.

For ext4 it has been added in:

commit 3209f68b3ca4667069923a325c88b21131bfdf9f
Author: David Howells <dhowells@redhat.com>
Date:   Fri Mar 31 18:32:17 2017 +0100

	statx: Include a mask for stx_attributes in struct statx


Hence for ext4 the flags should be enabled since kernel 4.11

etc.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH v2] syscalls/statx04: use stx_attributes_mask before test
  2019-09-11 12:47             ` Cyril Hrubis
@ 2019-09-12  3:28               ` =?unknown-8bit?b?WWFuZy/lvpAg5p2o?=
  2019-10-25  3:53               ` [LTP] [PATCH] " Yang Xu
  1 sibling, 0 replies; 19+ messages in thread
From: =?unknown-8bit?b?WWFuZy/lvpAg5p2o?= @ 2019-09-12  3:28 UTC (permalink / raw)
  To: ltp


on 2019/09/11 20:47, Cyril Hrubis wrote:

> Hi!
>> Do you mean use getxattr to ensure bitflags are enable or a functions test?
>> I am confused.
> For a given filesystem the support for filling in these flags was added
> at some point to the kernel. If any kernel newer that this version fails
> to fill them up it's a bug.
>
> For ext2 it has been added in:
>
> commit 93bc420ed41df63a18ae794101f7cbf45226a6ef
> Author: yangerkun <yangerkun@huawei.com>
> Date:   Mon Feb 18 09:07:02 2019 +0800
>
>      ext2: support statx syscall
>
> Hence starting kernel 5.0 ext2 (with ext2 driver) has to set the mask.
>
> For ext4 it has been added in:
>
> commit 3209f68b3ca4667069923a325c88b21131bfdf9f
> Author: David Howells <dhowells@redhat.com>
> Date:   Fri Mar 31 18:32:17 2017 +0100
>
> 	statx: Include a mask for stx_attributes in struct statx
>
>
> Hence for ext4 the flags should be enabled since kernel 4.11

Hi Cyril

Thanks, I see. It seems that kernel version check is useful for upstream kernel. But if an LTS linux distribution backports the commit
93bc420 for ext2, this kernel version will make no sense.  I remember ltp has a discussion between EISDIR and EBDAF about
copy_file_range[1]. Also ext2 should enable CONFIG_EXT2_FS_XATTR if we don't use ext4 instead of it.

IMO, we don't need to add kernel enable version check for various filesystems because QA or user can find their system doesn't support
these flags and report this to linux distribution vendor. So these flags may be supported on next release.

If kernel enables these flags fails to fill them up it's a bug.  We can only give some comments  about their enabled commit information.
So user can know it is a bug or a non-supported feature.

ps: I have a question about min_kernel all the time. If a new feature(such as PR_CAP_AMBIENT ) is introduced since upstream kernel 4.3,
but this feature is also backported on low version kernel on some linux distributions. What kind of situation can we use the min_kernel
to distinguish it? what kind of situation we don't need? test-writing-guidelines.txt doesn't mention it.

Thanks
Yang Xu

>
> etc.
>


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190912/4b3a53b7/attachment-0001.htm>

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

* [LTP] [PATCH] syscalls/statx04: use stx_attributes_mask before test
  2019-09-11 12:47             ` Cyril Hrubis
  2019-09-12  3:28               ` =?unknown-8bit?b?WWFuZy/lvpAg5p2o?=
@ 2019-10-25  3:53               ` Yang Xu
  2019-10-31 10:03                 ` Yang Xu
                                   ` (2 more replies)
  1 sibling, 3 replies; 19+ messages in thread
From: Yang Xu @ 2019-10-25  3:53 UTC (permalink / raw)
  To: ltp

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.

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

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/statx/statx04.c | 158 +++++++++++++---------
 1 file changed, 93 insertions(+), 65 deletions(-)

diff --git a/testcases/kernel/syscalls/statx/statx04.c b/testcases/kernel/syscalls/statx/statx04.c
index 71de734f5..c0fa06d46 100644
--- a/testcases/kernel/syscalls/statx/statx04.c
+++ b/testcases/kernel/syscalls/statx/statx04.c
@@ -34,85 +34,70 @@
 #define TESTDIR_UNFLAGGED MOUNT_POINT"/test_dir2"
 
 static int fd, clear_flags;
+static int supp_compr;
+static int supp_append;
+static int supp_immutable;
+static int supp_nodump;
 
-static void test_flagged(void)
+static void test_flag(int flag)
 {
 	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)
-{
-	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 +120,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 +139,49 @@ static void caid_flags_setup(void)
 
 static void setup(void)
 {
+	struct statx buf;
+
+	supp_compr = 1;
+	supp_append = 1;
+	supp_immutable = 1;
+	supp_nodump = 1;
 	SAFE_MKDIR(TESTDIR_FLAGGED, 0777);
 	SAFE_MKDIR(TESTDIR_UNFLAGGED, 0777);
 
+	// 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");
+
+	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");
+
+	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");
+
+	TEST(statx(AT_FDCWD, TESTDIR_FLAGGED, 0, 0, &buf));
+	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");
 
 	caid_flags_setup();
 }
-- 
2.18.0




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

* [LTP] [PATCH] syscalls/statx04: use stx_attributes_mask before test
  2019-10-25  3:53               ` [LTP] [PATCH] " Yang Xu
@ 2019-10-31 10:03                 ` Yang Xu
  2021-05-25 13:18                 ` Li Wang
  2021-11-10 13:28                 ` Richard Palethorpe
  2 siblings, 0 replies; 19+ messages in thread
From: Yang Xu @ 2019-10-31 10:03 UTC (permalink / raw)
  To: ltp

Hi All

Ping.

> 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.
>
> ---------------
> v2->v3:
> 1.add kernel version check for stx_attributes_mask
> 2. use test_flag(int) instead of test_flagged and test_unflagged
> ---------------
>
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> ---
>   testcases/kernel/syscalls/statx/statx04.c | 158 +++++++++++++---------
>   1 file changed, 93 insertions(+), 65 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/statx/statx04.c b/testcases/kernel/syscalls/statx/statx04.c
> index 71de734f5..c0fa06d46 100644
> --- a/testcases/kernel/syscalls/statx/statx04.c
> +++ b/testcases/kernel/syscalls/statx/statx04.c
> @@ -34,85 +34,70 @@
>   #define TESTDIR_UNFLAGGED MOUNT_POINT"/test_dir2"
>   
>   static int fd, clear_flags;
> +static int supp_compr;
> +static int supp_append;
> +static int supp_immutable;
> +static int supp_nodump;
>   
> -static void test_flagged(void)
> +static void test_flag(int flag)
>   {
>   	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)
> -{
> -	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 +120,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 +139,49 @@ static void caid_flags_setup(void)
>   
>   static void setup(void)
>   {
> +	struct statx buf;
> +
> +	supp_compr = 1;
> +	supp_append = 1;
> +	supp_immutable = 1;
> +	supp_nodump = 1;
>   	SAFE_MKDIR(TESTDIR_FLAGGED, 0777);
>   	SAFE_MKDIR(TESTDIR_UNFLAGGED, 0777);
>   
> +	// 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");
> +
> +	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");
> +
> +	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");
> +
> +	TEST(statx(AT_FDCWD, TESTDIR_FLAGGED, 0, 0, &buf));
> +	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");
>   
>   	caid_flags_setup();
>   }


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20191031/79aa4b66/attachment-0001.htm>

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

* [LTP] [PATCH] syscalls/statx04: use stx_attributes_mask before test
  2019-10-25  3:53               ` [LTP] [PATCH] " Yang Xu
  2019-10-31 10:03                 ` Yang Xu
@ 2021-05-25 13:18                 ` Li Wang
  2021-05-25 14:16                   ` Cyril Hrubis
  2021-11-10 13:28                 ` Richard Palethorpe
  2 siblings, 1 reply; 19+ messages in thread
From: Li Wang @ 2021-05-25 13:18 UTC (permalink / raw)
  To: ltp

Hi all,

Yang Xu <xuyang2018.jy@cn.fujitsu.com> wrote:

>
> 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.
>
> ---------------
> v2->v3:
> 1.add kernel version check for stx_attributes_mask
> 2. use test_flag(int) instead of test_flagged and test_unflagged
> ---------------
>
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>

Reviewed-by: Li Wang <liwang@redhat.com>

This patch makes sense to me, I'm not sure if any blocker issue for
holding the apply process. If _no_ I would help to merge it:).

-- 
Regards,
Li Wang


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

* [LTP] [PATCH] syscalls/statx04: use stx_attributes_mask before test
  2021-05-25 13:18                 ` Li Wang
@ 2021-05-25 14:16                   ` Cyril Hrubis
  2021-05-26  4:00                     ` xuyang2018.jy
  0 siblings, 1 reply; 19+ messages in thread
From: Cyril Hrubis @ 2021-05-25 14:16 UTC (permalink / raw)
  To: ltp

Hi!
> > 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.
> >
> > ---------------
> > v2->v3:
> > 1.add kernel version check for stx_attributes_mask
> > 2. use test_flag(int) instead of test_flagged and test_unflagged
> > ---------------
> >
> > Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> 
> Reviewed-by: Li Wang <liwang@redhat.com>
> 
> This patch makes sense to me, I'm not sure if any blocker issue for
> holding the apply process. If _no_ I would help to merge it:).

See:

https://github.com/linux-test-project/ltp/issues/557

Basically this change hides a kernel bug. I've proposed to create a
separate test for kernel that makes sure that all flags that are
supposed to be enabled are enabled for new enough kernels, then we can
apply this patch.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH] syscalls/statx04: use stx_attributes_mask before test
  2021-05-25 14:16                   ` Cyril Hrubis
@ 2021-05-26  4:00                     ` xuyang2018.jy
  2021-05-26  8:31                       ` Li Wang
  0 siblings, 1 reply; 19+ messages in thread
From: xuyang2018.jy @ 2021-05-26  4:00 UTC (permalink / raw)
  To: ltp

Hi Cyril
> Hi!
>>> 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.
>>>
>>> ---------------
>>> v2->v3:
>>> 1.add kernel version check for stx_attributes_mask
>>> 2. use test_flag(int) instead of test_flagged and test_unflagged
>>> ---------------
>>>
>>> Signed-off-by: Yang Xu<xuyang2018.jy@cn.fujitsu.com>
>>> Reviewed-by: Petr Vorel<pvorel@suse.cz>
>>
>> Reviewed-by: Li Wang<liwang@redhat.com>
>>
>> This patch makes sense to me, I'm not sure if any blocker issue for
>> holding the apply process. If _no_ I would help to merge it:).
>
> See:
>
> https://github.com/linux-test-project/ltp/issues/557
>
> Basically this change hides a kernel bug.
I don't think it is a kernel bug and it is only an non-supported feature 
before linux 5.1 when not using ext4 driver for ext2.

> I've proposed to create a
> separate test for kernel that makes sure that all flags that are
> supposed to be enabled are enabled for new enough kernels, then we can
> apply this patch.
But not all fs support all flags, like xfs it doesn't support 
STATX_ATTR_COMPRESSED flag even now.

[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/xfs/xfs_iops.c#n611
>


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

* [LTP] [PATCH] syscalls/statx04: use stx_attributes_mask before test
  2021-05-26  4:00                     ` xuyang2018.jy
@ 2021-05-26  8:31                       ` Li Wang
  2021-05-26 10:02                         ` xuyang2018.jy
  0 siblings, 1 reply; 19+ messages in thread
From: Li Wang @ 2021-05-26  8:31 UTC (permalink / raw)
  To: ltp

Hi all,

> >> This patch makes sense to me, I'm not sure if any blocker issue for
> >> holding the apply process. If _no_ I would help to merge it:).
> >
> > See:
> >
> > https://github.com/linux-test-project/ltp/issues/557
> >
> > Basically this change hides a kernel bug.
> I don't think it is a kernel bug and it is only an non-supported feature
> before linux 5.1 when not using ext4 driver for ext2.

Cyril hopes to have a reproducer to cover the bug [1] (though
you think it a non-support problem), but that is responsible
advice for FileSystem:).
Actually, the worth to say, there already a separate test
(xfstest/generic/424) for reproducing this problem[2].
See: https://github.com/kdave/xfstests/blob/master/tests/generic/424
I believe FS QE/Dev will run it for all filesystems regularly.

So maybe we can safely apply this patch in LTP without
adding a duplicated test?

[1] https://bugs.linaro.org/show_bug.cgi?id=4012
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=93bc420ed41df63

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210526/c12db3cf/attachment.htm>

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

* [LTP] [PATCH] syscalls/statx04: use stx_attributes_mask before test
  2021-05-26  8:31                       ` Li Wang
@ 2021-05-26 10:02                         ` xuyang2018.jy
  0 siblings, 0 replies; 19+ messages in thread
From: xuyang2018.jy @ 2021-05-26 10:02 UTC (permalink / raw)
  To: ltp

Hi Li
> Hi all,
>
>  > >> This patch makes sense to me, I'm not sure if any blocker issue for
>  > >> holding the apply process. If _no_ I would help to merge it:).
>  > >
>  > > See:
>  > >
>  > > https://github.com/linux-test-project/ltp/issues/557
>  > >
>  > > Basically this change hides a kernel bug.
>  > I don't think it is a kernel bug and it is only an non-supported feature
>  > before linux 5.1 when not using ext4 driver for ext2.
>
> Cyril hopes to have a reproducer to cover the bug [1](though
> you think it a non-support problem), but that is responsible
> advicefor FileSystem:).
Yes, xfstest is more situable to do this.
> Actually, the worth to say, there already a separate test
> (xfstest/generic/424) for reproducing this problem[2].
Yes, this fix is found by xfstest/generic/424.
> See: https://github.com/kdave/xfstests/blob/master/tests/generic/424
Usually I see xfstests case on offical repo repository
https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git
> I believe FS QE/Dev will run it for all filesystems regularly.
I think Theodore Ts'o may do it on xfstests-bld[1] and other 
project(like lkp) may also do it by using really ext2 driver. So having 
generic/424 is enough to find this kernel bug.

[1]https://git.kernel.org/pub/scm/fs/ext2/xfstests-bld.git/tree/kernel-configs/x86_64-config-5.10#n114
>
> So maybe we can safely apply this patch in LTP without
> adding a duplicated test?
+1. With this patch, we can also run this case on xfs filesystem.
>
> [1] https://bugs.linaro.org/show_bug.cgi?id=4012
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=93bc420ed41df63
>
> --
> Regards,
> Li Wang

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

* Re: [LTP] [PATCH] syscalls/statx04: use stx_attributes_mask before test
  2019-10-25  3:53               ` [LTP] [PATCH] " Yang Xu
  2019-10-31 10:03                 ` Yang Xu
  2021-05-25 13:18                 ` Li Wang
@ 2021-11-10 13:28                 ` Richard Palethorpe
  2021-11-11  3:25                   ` xuyang2018.jy
  2 siblings, 1 reply; 19+ messages in thread
From: Richard Palethorpe @ 2021-11-10 13:28 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hello Yang,

The fix/feature appears easy to backport, so I would suggest just adding the git
commit tags to the test. If the feature is required then people can
backport the patches, otherwise they can filter out the test.

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

* Re: [LTP] [PATCH] syscalls/statx04: use stx_attributes_mask before test
  2021-11-10 13:28                 ` Richard Palethorpe
@ 2021-11-11  3:25                   ` xuyang2018.jy
  0 siblings, 0 replies; 19+ messages in thread
From: xuyang2018.jy @ 2021-11-11  3:25 UTC (permalink / raw)
  To: rpalethorpe; +Cc: ltp

Hi Richard
> Hello Yang,
> 
> The fix/feature appears easy to backport, so I would suggest just adding the git
> commit tags to the test. If the feature is required then people can
> backport the patches, otherwise they can filter out the test.
> 

I will add these linux tags into description and send a v4.

Best Regards
Yang Xu

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

end of thread, other threads:[~2021-11-11  3:26 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-02 10:34 [LTP] [PATCH] syscalls/statx04: use stx_attributes_mask before test Yang Xu
2019-08-02 11:50 ` Cyril Hrubis
2019-08-20  6:33   ` [LTP] [PATCH v2] " Yang Xu
2019-08-27  9:25     ` Petr Vorel
2019-08-27  9:58       ` Petr Vorel
2019-08-27 10:16         ` Cyril Hrubis
2019-09-11 10:22           ` =?unknown-8bit?b?WWFuZy/lvpAg5p2o?=
2019-09-11 12:47             ` Cyril Hrubis
2019-09-12  3:28               ` =?unknown-8bit?b?WWFuZy/lvpAg5p2o?=
2019-10-25  3:53               ` [LTP] [PATCH] " Yang Xu
2019-10-31 10:03                 ` Yang Xu
2021-05-25 13:18                 ` Li Wang
2021-05-25 14:16                   ` Cyril Hrubis
2021-05-26  4:00                     ` xuyang2018.jy
2021-05-26  8:31                       ` Li Wang
2021-05-26 10:02                         ` xuyang2018.jy
2021-11-10 13:28                 ` Richard Palethorpe
2021-11-11  3:25                   ` xuyang2018.jy
2019-08-28  3:56       ` [LTP] [PATCH v2] " Yang Xu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.