diff mbox series

syscalls/statx12: Add basic test for STATX_ATTR_MOUNT_ROOT flag

Message ID 1684748642-6373-1-git-send-email-xuyang2018.jy@fujitsu.com
State Accepted
Headers show
Series syscalls/statx12: Add basic test for STATX_ATTR_MOUNT_ROOT flag | expand

Commit Message

Yang Xu May 22, 2023, 9:44 a.m. UTC 
This flag was introduced since kernel 5.10 and was used to indicates
whether the path or fd refers to the root of a mount or not.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/lapi/stat.h                        |   4 +
 runtest/syscalls                           |   1 +
 testcases/kernel/syscalls/statx/.gitignore |   1 +
 testcases/kernel/syscalls/statx/statx12.c  | 101 +++++++++++++++++++++
 4 files changed, 107 insertions(+)
 create mode 100644 testcases/kernel/syscalls/statx/statx12.c

Comments

Petr Vorel May 29, 2023, 5:50 p.m. UTC | #1 
Hi Xu,

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>

I tested it on recent kernel 6.3.1 and very old kernel 3.16.0.
Works as expected.


> This flag was introduced since kernel 5.10 and was used to indicates

80340fe3605c ("statx: add mount_root") was released in v5.8. Unless you refer to
something different, please update it here.

> whether the path or fd refers to the root of a mount or not.

> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  include/lapi/stat.h                        |   4 +
>  runtest/syscalls                           |   1 +
>  testcases/kernel/syscalls/statx/.gitignore |   1 +
>  testcases/kernel/syscalls/statx/statx12.c  | 101 +++++++++++++++++++++
>  4 files changed, 107 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/statx/statx12.c

> diff --git a/include/lapi/stat.h b/include/lapi/stat.h
> index c7e6fdac0..3606c9eb0 100644
> --- a/include/lapi/stat.h
> +++ b/include/lapi/stat.h
> @@ -221,6 +221,10 @@ static inline int statx(int dirfd, const char *pathname, unsigned int flags,
>  # define STATX_ATTR_AUTOMOUNT	0x00001000
>  #endif

> +#ifndef STATX_ATTR_MOUNT_ROOT
> +# define STATX_ATTR_MOUNT_ROOT	0x00002000
> +#endif
> +
>  #ifndef STATX_ATTR_VERITY
>  # define STATX_ATTR_VERITY	0x00100000
>  #endif
> diff --git a/runtest/syscalls b/runtest/syscalls
> index e5ad2c2f9..0c1993421 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1767,6 +1767,7 @@ statx08 statx08
>  statx09 statx09
>  statx10 statx10
>  statx11 statx11
> +statx12 statx12

>  membarrier01 membarrier01

> diff --git a/testcases/kernel/syscalls/statx/.gitignore b/testcases/kernel/syscalls/statx/.gitignore
> index 48ac4078b..f6a423eed 100644
> --- a/testcases/kernel/syscalls/statx/.gitignore
> +++ b/testcases/kernel/syscalls/statx/.gitignore
> @@ -9,3 +9,4 @@
>  /statx09
>  /statx10
>  /statx11
> +/statx12
> diff --git a/testcases/kernel/syscalls/statx/statx12.c b/testcases/kernel/syscalls/statx/statx12.c
> new file mode 100644
> index 000000000..ae6bbb1e2
> --- /dev/null
> +++ b/testcases/kernel/syscalls/statx/statx12.c
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * It is a basic test for STATX_ATTR_MOUNT_ROOT flag.
> + *
> + * This flag indicates whether the path or fd refers to the root of a mount
> + * or not.
> + *
> + * Minimum Linux version required is v5.10.
And here as well.
> + */
> +
> +#define _GNU_SOURCE
> +#include <sys/types.h>
> +#include <sys/mount.h>
Do we need these two for anything?

> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include "tst_test.h"
> +#include "lapi/stat.h"
> +
> +#define MNTPOINT "mntpoint"
> +#define TESTFILE MNTPOINT"/testfile"
> +
> +static int dir_fd = -1, file_fd = -1;
> +
> +static struct tcase {
> +	const char *path;
> +	int mnt_root;
> +	int flag;
Since you're already using <stdbool.h>, mnt_root and flag could be bool.
> +	int *fd;
> +} tcases[] = {
> +	{MNTPOINT, 1, 1, &dir_fd},
> +	{MNTPOINT, 1, 0, &dir_fd},
> +	{TESTFILE, 0, 1, &file_fd},
> +	{TESTFILE, 0, 0, &file_fd}
I'd even consider replacing int flag in struct with counted from n:

static struct tcase {
	const char *path;
	bool mnt_root;
	int *fd;
} tcases[] = {
	{MNTPOINT, 1, &dir_fd},
	{TESTFILE, 0, &file_fd}
};

static void verify_statx(unsigned int n)
{
	struct tcase *tc = &tcases[n/2];
	struct statx buf;
	bool flag = n % 2;

	if (flag) {
		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
				tc->path);
		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf));
	} else {
		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
				tc->path);

		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf));
	}
	...

static struct tst_test test = {
	...
	.tcnt = 2* ARRAY_SIZE(tcases)


> +};
> +
> +static void verify_statx(unsigned int n)
> +{
> +	struct tcase *tc = &tcases[n];
> +	struct statx buf;
> +
> +	if (tc->flag) {
> +		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
> +				tc->path);
> +
> +		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf),
> +			"statx(AT_FDCWD, %s, 0, 0, &buf)", tc->path);
nit: I wonder if we need to duplicate the call in string, just to get tc->path,
which we have printed in TINFO above. Wouldn't be enough just:
		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf));

> +	} else {
> +		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
> +				tc->path);
> +		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf),
> +			"statx(%d, "", 0, 0, &buf)", *tc->fd);
Well, here you probably meant
		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf),
			"statx(%d, \"\", 0, 0, &buf)", *tc->fd);
checkpatch.pl (via make check-statx12) warns about it (slightly cryptic):
statx12.c:60: WARNING: Consecutive strings are generally better as a single string
TODO: this is the second thing which should be fixed before merge.

But again, I'd go just for:
		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf));
> +	}
> +
> +	if (!(buf.stx_attributes_mask & STATX_ATTR_MOUNT_ROOT)) {
> +		tst_res(TCONF, "Filesystem does not support STATX_ATTR_MOUNT_ROOT");
> +		return;
> +	}
> +
> +	if (buf.stx_attributes & STATX_ATTR_MOUNT_ROOT) {
> +		tst_res(tc->mnt_root ? TPASS : TFAIL,
> +			"STATX_ATTR_MOUNT_ROOT flag is set");
> +	} else {
> +		tst_res(tc->mnt_root ? TFAIL : TPASS,
> +			"STATX_ATTR_MOUNT_ROOT flag is not set");
> +	}
> +}
> +
> +static void setup(void)
> +{
> +	SAFE_CREAT(TESTFILE, 0755);
> +	dir_fd = SAFE_OPEN(MNTPOINT, O_DIRECTORY);
> +	file_fd = SAFE_OPEN(TESTFILE, O_RDWR);
> +}
> +
> +static void cleanup(void)
> +{
> +	if (dir_fd > -1)
> +		SAFE_CLOSE(dir_fd);
nit: Here could be empty line (readability).
> +	if (file_fd > -1)
> +		SAFE_CLOSE(file_fd);
> +}
> +
> +static struct tst_test test = {
> +	.test = verify_statx,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.mntpoint = MNTPOINT,
> +	.mount_device = 1,
> +	.all_filesystems = 1,
> +	.needs_root = 1,
> +	.tcnt = ARRAY_SIZE(tcases)
> +};

All my suggestion (ok to ignore).

Kind regards,
Petr

diff --git testcases/kernel/syscalls/statx/statx12.c testcases/kernel/syscalls/statx/statx12.c
index 40367c8b1..6b76b6e49 100644
--- testcases/kernel/syscalls/statx/statx12.c
+++ testcases/kernel/syscalls/statx/statx12.c
@@ -12,12 +12,10 @@
  * This flag indicates whether the path or fd refers to the root of a mount
  * or not.
  *
- * Minimum Linux version required is v5.10.
+ * Minimum Linux version required is v5.8.
  */
 
 #define _GNU_SOURCE
-#include <sys/types.h>
-#include <sys/mount.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <stdbool.h>
@@ -32,32 +30,27 @@ static int dir_fd = -1, file_fd = -1;
 
 static struct tcase {
 	const char *path;
-	int mnt_root;
-	int flag;
+	bool mnt_root;
 	int *fd;
 } tcases[] = {
-	{MNTPOINT, 1, 1, &dir_fd},
-	{MNTPOINT, 1, 0, &dir_fd},
-	{TESTFILE, 0, 1, &file_fd},
-	{TESTFILE, 0, 0, &file_fd}
+	{MNTPOINT, 1, &dir_fd},
+	{TESTFILE, 0, &file_fd}
 };
 
 static void verify_statx(unsigned int n)
 {
-	struct tcase *tc = &tcases[n];
+	struct tcase *tc = &tcases[n/2];
 	struct statx buf;
+	bool flag = n % 2;
 
-	if (tc->flag) {
-		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
+	if (flag) {
+		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
 				tc->path);
-
-		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf),
-			"statx(AT_FDCWD, %s, 0, 0, &buf)", tc->path);
+		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf));
 	} else {
-		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
+		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
 				tc->path);
-		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf),
-			"statx(%d, "", 0, 0, &buf)", *tc->fd);
+		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf));
 	}
 
 	if (!(buf.stx_attributes_mask & STATX_ATTR_MOUNT_ROOT)) {
@@ -85,6 +78,7 @@ static void cleanup(void)
 {
 	if (dir_fd > -1)
 		SAFE_CLOSE(dir_fd);
+
 	if (file_fd > -1)
 		SAFE_CLOSE(file_fd);
 }
@@ -97,5 +91,5 @@ static struct tst_test test = {
 	.mount_device = 1,
 	.all_filesystems = 1,
 	.needs_root = 1,
-	.tcnt = ARRAY_SIZE(tcases)
+	.tcnt = 2* ARRAY_SIZE(tcases)
 };
Yang Xu May 30, 2023, 5:52 a.m. UTC | #2 
Hi Petr


> Hi Xu,
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Tested-by: Petr Vorel <pvorel@suse.cz>
> 
> I tested it on recent kernel 6.3.1 and very old kernel 3.16.0.
> Works as expected.
> 
> 
>> This flag was introduced since kernel 5.10 and was used to indicates
> 
> 80340fe3605c ("statx: add mount_root") was released in v5.8. Unless you refer to
> something different, please update it here.
# git tag --contains 80340fe3
Oh, sorry, I used the following command to search and miss v5.8
v5.10
v5.10-rc1
v5.10-rc2
v5.10-rc3
v5.10-rc4
v5.10-rc5
v5.10-rc6
v5.10-rc7
v5.11
v5.11-rc1
v5.11-rc2
v5.11-rc3
v5.11-rc4
v5.11-rc5
v5.11-rc6
v5.11-rc7
v5.12
v5.12-rc1
v5.12-rc1-dontuse
v5.12-rc2
v5.12-rc3
v5.12-rc4
v5.12-rc5
v5.12-rc6
v5.12-rc7
....
v5.8-rc1

> 
>> whether the path or fd refers to the root of a mount or not.
> 
>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>> ---
>>   include/lapi/stat.h                        |   4 +
>>   runtest/syscalls                           |   1 +
>>   testcases/kernel/syscalls/statx/.gitignore |   1 +
>>   testcases/kernel/syscalls/statx/statx12.c  | 101 +++++++++++++++++++++
>>   4 files changed, 107 insertions(+)
>>   create mode 100644 testcases/kernel/syscalls/statx/statx12.c
> 
>> diff --git a/include/lapi/stat.h b/include/lapi/stat.h
>> index c7e6fdac0..3606c9eb0 100644
>> --- a/include/lapi/stat.h
>> +++ b/include/lapi/stat.h
>> @@ -221,6 +221,10 @@ static inline int statx(int dirfd, const char *pathname, unsigned int flags,
>>   # define STATX_ATTR_AUTOMOUNT	0x00001000
>>   #endif
> 
>> +#ifndef STATX_ATTR_MOUNT_ROOT
>> +# define STATX_ATTR_MOUNT_ROOT	0x00002000
>> +#endif
>> +
>>   #ifndef STATX_ATTR_VERITY
>>   # define STATX_ATTR_VERITY	0x00100000
>>   #endif
>> diff --git a/runtest/syscalls b/runtest/syscalls
>> index e5ad2c2f9..0c1993421 100644
>> --- a/runtest/syscalls
>> +++ b/runtest/syscalls
>> @@ -1767,6 +1767,7 @@ statx08 statx08
>>   statx09 statx09
>>   statx10 statx10
>>   statx11 statx11
>> +statx12 statx12
> 
>>   membarrier01 membarrier01
> 
>> diff --git a/testcases/kernel/syscalls/statx/.gitignore b/testcases/kernel/syscalls/statx/.gitignore
>> index 48ac4078b..f6a423eed 100644
>> --- a/testcases/kernel/syscalls/statx/.gitignore
>> +++ b/testcases/kernel/syscalls/statx/.gitignore
>> @@ -9,3 +9,4 @@
>>   /statx09
>>   /statx10
>>   /statx11
>> +/statx12
>> diff --git a/testcases/kernel/syscalls/statx/statx12.c b/testcases/kernel/syscalls/statx/statx12.c
>> new file mode 100644
>> index 000000000..ae6bbb1e2
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/statx/statx12.c
>> @@ -0,0 +1,101 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
>> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * It is a basic test for STATX_ATTR_MOUNT_ROOT flag.
>> + *
>> + * This flag indicates whether the path or fd refers to the root of a mount
>> + * or not.
>> + *
>> + * Minimum Linux version required is v5.10.
> And here as well.

Yes.
>> + */
>> +
>> +#define _GNU_SOURCE
>> +#include <sys/types.h>
>> +#include <sys/mount.h>
> Do we need these two for anything?
I guess we can remove them.
> 
>> +#include <unistd.h>
>> +#include <stdlib.h>
>> +#include <stdbool.h>
>> +#include <stdio.h>
>> +#include "tst_test.h"
>> +#include "lapi/stat.h"
>> +
>> +#define MNTPOINT "mntpoint"
>> +#define TESTFILE MNTPOINT"/testfile"
>> +
>> +static int dir_fd = -1, file_fd = -1;
>> +
>> +static struct tcase {
>> +	const char *path;
>> +	int mnt_root;
>> +	int flag;
> Since you're already using <stdbool.h>, mnt_root and flag could be bool.

Yes.
>> +	int *fd;
>> +} tcases[] = {
>> +	{MNTPOINT, 1, 1, &dir_fd},
>> +	{MNTPOINT, 1, 0, &dir_fd},
>> +	{TESTFILE, 0, 1, &file_fd},
>> +	{TESTFILE, 0, 0, &file_fd}
> I'd even consider replacing int flag in struct with counted from n:
> 
> static struct tcase {
> 	const char *path;
> 	bool mnt_root;
> 	int *fd;
> } tcases[] = {
> 	{MNTPOINT, 1, &dir_fd},
> 	{TESTFILE, 0, &file_fd}
> };
> 
> static void verify_statx(unsigned int n)
> {
> 	struct tcase *tc = &tcases[n/2];
> 	struct statx buf;
> 	bool flag = n % 2;
> 
> 	if (flag) {
> 		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
> 				tc->path);
> 		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf));
> 	} else {
> 		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
> 				tc->path);
> 
> 		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf));
> 	}
> 	...
> 
> static struct tst_test test = {
> 	...
> 	.tcnt = 2* ARRAY_SIZE(tcases)
> 
> 
>> +};
>> +
>> +static void verify_statx(unsigned int n)
>> +{
>> +	struct tcase *tc = &tcases[n];
>> +	struct statx buf;
>> +
>> +	if (tc->flag) {
>> +		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
>> +				tc->path);
>> +
>> +		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf),
>> +			"statx(AT_FDCWD, %s, 0, 0, &buf)", tc->path);
> nit: I wonder if we need to duplicate the call in string, just to get tc->path,
> which we have printed in TINFO above. Wouldn't be enough just:
> 		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf));

Sound reasonable.
> 
>> +	} else {
>> +		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
>> +				tc->path);
>> +		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf),
>> +			"statx(%d, "", 0, 0, &buf)", *tc->fd);
> Well, here you probably meant
> 		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf),
> 			"statx(%d, \"\", 0, 0, &buf)", *tc->fd);
> checkpatch.pl (via make check-statx12) warns about it (slightly cryptic):
> statx12.c:60: WARNING: Consecutive strings are generally better as a single string
> TODO: this is the second thing which should be fixed before merge.

Sound reasonable.
> 
> But again, I'd go just for:
> 		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf));

OK.
>> +	}
>> +
>> +	if (!(buf.stx_attributes_mask & STATX_ATTR_MOUNT_ROOT)) {
>> +		tst_res(TCONF, "Filesystem does not support STATX_ATTR_MOUNT_ROOT");
>> +		return;
>> +	}
>> +
>> +	if (buf.stx_attributes & STATX_ATTR_MOUNT_ROOT) {
>> +		tst_res(tc->mnt_root ? TPASS : TFAIL,
>> +			"STATX_ATTR_MOUNT_ROOT flag is set");
>> +	} else {
>> +		tst_res(tc->mnt_root ? TFAIL : TPASS,
>> +			"STATX_ATTR_MOUNT_ROOT flag is not set");
>> +	}
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	SAFE_CREAT(TESTFILE, 0755);
>> +	dir_fd = SAFE_OPEN(MNTPOINT, O_DIRECTORY);
>> +	file_fd = SAFE_OPEN(TESTFILE, O_RDWR);
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	if (dir_fd > -1)
>> +		SAFE_CLOSE(dir_fd);
> nit: Here could be empty line (readability).
>> +	if (file_fd > -1)
>> +		SAFE_CLOSE(file_fd);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.test = verify_statx,
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.mntpoint = MNTPOINT,
>> +	.mount_device = 1,
>> +	.all_filesystems = 1,
>> +	.needs_root = 1,
>> +	.tcnt = ARRAY_SIZE(tcases)
>> +};
> 
> All my suggestion (ok to ignore).

I agree with your all suggestion.

Best Regards
Yang Xu
> 
> Kind regards,
> Petr
> 
> diff --git testcases/kernel/syscalls/statx/statx12.c testcases/kernel/syscalls/statx/statx12.c
> index 40367c8b1..6b76b6e49 100644
> --- testcases/kernel/syscalls/statx/statx12.c
> +++ testcases/kernel/syscalls/statx/statx12.c
> @@ -12,12 +12,10 @@
>    * This flag indicates whether the path or fd refers to the root of a mount
>    * or not.
>    *
> - * Minimum Linux version required is v5.10.
> + * Minimum Linux version required is v5.8.
>    */
>   
>   #define _GNU_SOURCE
> -#include <sys/types.h>
> -#include <sys/mount.h>
>   #include <unistd.h>
>   #include <stdlib.h>
>   #include <stdbool.h>
> @@ -32,32 +30,27 @@ static int dir_fd = -1, file_fd = -1;
>   
>   static struct tcase {
>   	const char *path;
> -	int mnt_root;
> -	int flag;
> +	bool mnt_root;
>   	int *fd;
>   } tcases[] = {
> -	{MNTPOINT, 1, 1, &dir_fd},
> -	{MNTPOINT, 1, 0, &dir_fd},
> -	{TESTFILE, 0, 1, &file_fd},
> -	{TESTFILE, 0, 0, &file_fd}
> +	{MNTPOINT, 1, &dir_fd},
> +	{TESTFILE, 0, &file_fd}
>   };
>   
>   static void verify_statx(unsigned int n)
>   {
> -	struct tcase *tc = &tcases[n];
> +	struct tcase *tc = &tcases[n/2];
>   	struct statx buf;
> +	bool flag = n % 2;
>   
> -	if (tc->flag) {
> -		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
> +	if (flag) {
> +		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
>   				tc->path);
> -
> -		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf),
> -			"statx(AT_FDCWD, %s, 0, 0, &buf)", tc->path);
> +		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf));
>   	} else {
> -		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
> +		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
>   				tc->path);
> -		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf),
> -			"statx(%d, "", 0, 0, &buf)", *tc->fd);
> +		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf));
>   	}
>   
>   	if (!(buf.stx_attributes_mask & STATX_ATTR_MOUNT_ROOT)) {
> @@ -85,6 +78,7 @@ static void cleanup(void)
>   {
>   	if (dir_fd > -1)
>   		SAFE_CLOSE(dir_fd);
> +
>   	if (file_fd > -1)
>   		SAFE_CLOSE(file_fd);
>   }
> @@ -97,5 +91,5 @@ static struct tst_test test = {
>   	.mount_device = 1,
>   	.all_filesystems = 1,
>   	.needs_root = 1,
> -	.tcnt = ARRAY_SIZE(tcases)
> +	.tcnt = 2* ARRAY_SIZE(tcases)
>   };
Yang Xu June 2, 2023, 3:13 a.m. UTC | #3 
Hi Petr

Should I send a v2 patch for this, or you merged the modified patch 
directly.


Best Regards
Yang Xu
> Hi Petr
> 
> 
>> Hi Xu,
>>
>> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>> Tested-by: Petr Vorel <pvorel@suse.cz>
>>
>> I tested it on recent kernel 6.3.1 and very old kernel 3.16.0.
>> Works as expected.
>>
>>
>>> This flag was introduced since kernel 5.10 and was used to indicates
>>
>> 80340fe3605c ("statx: add mount_root") was released in v5.8. Unless you refer to
>> something different, please update it here.
> # git tag --contains 80340fe3
> Oh, sorry, I used the following command to search and miss v5.8
> v5.10
> v5.10-rc1
> v5.10-rc2
> v5.10-rc3
> v5.10-rc4
> v5.10-rc5
> v5.10-rc6
> v5.10-rc7
> v5.11
> v5.11-rc1
> v5.11-rc2
> v5.11-rc3
> v5.11-rc4
> v5.11-rc5
> v5.11-rc6
> v5.11-rc7
> v5.12
> v5.12-rc1
> v5.12-rc1-dontuse
> v5.12-rc2
> v5.12-rc3
> v5.12-rc4
> v5.12-rc5
> v5.12-rc6
> v5.12-rc7
> ....
> v5.8-rc1
> 
>>
>>> whether the path or fd refers to the root of a mount or not.
>>
>>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>>> ---
>>>    include/lapi/stat.h                        |   4 +
>>>    runtest/syscalls                           |   1 +
>>>    testcases/kernel/syscalls/statx/.gitignore |   1 +
>>>    testcases/kernel/syscalls/statx/statx12.c  | 101 +++++++++++++++++++++
>>>    4 files changed, 107 insertions(+)
>>>    create mode 100644 testcases/kernel/syscalls/statx/statx12.c
>>
>>> diff --git a/include/lapi/stat.h b/include/lapi/stat.h
>>> index c7e6fdac0..3606c9eb0 100644
>>> --- a/include/lapi/stat.h
>>> +++ b/include/lapi/stat.h
>>> @@ -221,6 +221,10 @@ static inline int statx(int dirfd, const char *pathname, unsigned int flags,
>>>    # define STATX_ATTR_AUTOMOUNT	0x00001000
>>>    #endif
>>
>>> +#ifndef STATX_ATTR_MOUNT_ROOT
>>> +# define STATX_ATTR_MOUNT_ROOT	0x00002000
>>> +#endif
>>> +
>>>    #ifndef STATX_ATTR_VERITY
>>>    # define STATX_ATTR_VERITY	0x00100000
>>>    #endif
>>> diff --git a/runtest/syscalls b/runtest/syscalls
>>> index e5ad2c2f9..0c1993421 100644
>>> --- a/runtest/syscalls
>>> +++ b/runtest/syscalls
>>> @@ -1767,6 +1767,7 @@ statx08 statx08
>>>    statx09 statx09
>>>    statx10 statx10
>>>    statx11 statx11
>>> +statx12 statx12
>>
>>>    membarrier01 membarrier01
>>
>>> diff --git a/testcases/kernel/syscalls/statx/.gitignore b/testcases/kernel/syscalls/statx/.gitignore
>>> index 48ac4078b..f6a423eed 100644
>>> --- a/testcases/kernel/syscalls/statx/.gitignore
>>> +++ b/testcases/kernel/syscalls/statx/.gitignore
>>> @@ -9,3 +9,4 @@
>>>    /statx09
>>>    /statx10
>>>    /statx11
>>> +/statx12
>>> diff --git a/testcases/kernel/syscalls/statx/statx12.c b/testcases/kernel/syscalls/statx/statx12.c
>>> new file mode 100644
>>> index 000000000..ae6bbb1e2
>>> --- /dev/null
>>> +++ b/testcases/kernel/syscalls/statx/statx12.c
>>> @@ -0,0 +1,101 @@
>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>> +/*
>>> + * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
>>> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
>>> + */
>>> +
>>> +/*\
>>> + * [Description]
>>> + *
>>> + * It is a basic test for STATX_ATTR_MOUNT_ROOT flag.
>>> + *
>>> + * This flag indicates whether the path or fd refers to the root of a mount
>>> + * or not.
>>> + *
>>> + * Minimum Linux version required is v5.10.
>> And here as well.
> 
> Yes.
>>> + */
>>> +
>>> +#define _GNU_SOURCE
>>> +#include <sys/types.h>
>>> +#include <sys/mount.h>
>> Do we need these two for anything?
> I guess we can remove them.
>>
>>> +#include <unistd.h>
>>> +#include <stdlib.h>
>>> +#include <stdbool.h>
>>> +#include <stdio.h>
>>> +#include "tst_test.h"
>>> +#include "lapi/stat.h"
>>> +
>>> +#define MNTPOINT "mntpoint"
>>> +#define TESTFILE MNTPOINT"/testfile"
>>> +
>>> +static int dir_fd = -1, file_fd = -1;
>>> +
>>> +static struct tcase {
>>> +	const char *path;
>>> +	int mnt_root;
>>> +	int flag;
>> Since you're already using <stdbool.h>, mnt_root and flag could be bool.
> 
> Yes.
>>> +	int *fd;
>>> +} tcases[] = {
>>> +	{MNTPOINT, 1, 1, &dir_fd},
>>> +	{MNTPOINT, 1, 0, &dir_fd},
>>> +	{TESTFILE, 0, 1, &file_fd},
>>> +	{TESTFILE, 0, 0, &file_fd}
>> I'd even consider replacing int flag in struct with counted from n:
>>
>> static struct tcase {
>> 	const char *path;
>> 	bool mnt_root;
>> 	int *fd;
>> } tcases[] = {
>> 	{MNTPOINT, 1, &dir_fd},
>> 	{TESTFILE, 0, &file_fd}
>> };
>>
>> static void verify_statx(unsigned int n)
>> {
>> 	struct tcase *tc = &tcases[n/2];
>> 	struct statx buf;
>> 	bool flag = n % 2;
>>
>> 	if (flag) {
>> 		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
>> 				tc->path);
>> 		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf));
>> 	} else {
>> 		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
>> 				tc->path);
>>
>> 		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf));
>> 	}
>> 	...
>>
>> static struct tst_test test = {
>> 	...
>> 	.tcnt = 2* ARRAY_SIZE(tcases)
>>
>>
>>> +};
>>> +
>>> +static void verify_statx(unsigned int n)
>>> +{
>>> +	struct tcase *tc = &tcases[n];
>>> +	struct statx buf;
>>> +
>>> +	if (tc->flag) {
>>> +		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
>>> +				tc->path);
>>> +
>>> +		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf),
>>> +			"statx(AT_FDCWD, %s, 0, 0, &buf)", tc->path);
>> nit: I wonder if we need to duplicate the call in string, just to get tc->path,
>> which we have printed in TINFO above. Wouldn't be enough just:
>> 		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf));
> 
> Sound reasonable.
>>
>>> +	} else {
>>> +		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
>>> +				tc->path);
>>> +		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf),
>>> +			"statx(%d, "", 0, 0, &buf)", *tc->fd);
>> Well, here you probably meant
>> 		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf),
>> 			"statx(%d, \"\", 0, 0, &buf)", *tc->fd);
>> checkpatch.pl (via make check-statx12) warns about it (slightly cryptic):
>> statx12.c:60: WARNING: Consecutive strings are generally better as a single string
>> TODO: this is the second thing which should be fixed before merge.
> 
> Sound reasonable.
>>
>> But again, I'd go just for:
>> 		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf));
> 
> OK.
>>> +	}
>>> +
>>> +	if (!(buf.stx_attributes_mask & STATX_ATTR_MOUNT_ROOT)) {
>>> +		tst_res(TCONF, "Filesystem does not support STATX_ATTR_MOUNT_ROOT");
>>> +		return;
>>> +	}
>>> +
>>> +	if (buf.stx_attributes & STATX_ATTR_MOUNT_ROOT) {
>>> +		tst_res(tc->mnt_root ? TPASS : TFAIL,
>>> +			"STATX_ATTR_MOUNT_ROOT flag is set");
>>> +	} else {
>>> +		tst_res(tc->mnt_root ? TFAIL : TPASS,
>>> +			"STATX_ATTR_MOUNT_ROOT flag is not set");
>>> +	}
>>> +}
>>> +
>>> +static void setup(void)
>>> +{
>>> +	SAFE_CREAT(TESTFILE, 0755);
>>> +	dir_fd = SAFE_OPEN(MNTPOINT, O_DIRECTORY);
>>> +	file_fd = SAFE_OPEN(TESTFILE, O_RDWR);
>>> +}
>>> +
>>> +static void cleanup(void)
>>> +{
>>> +	if (dir_fd > -1)
>>> +		SAFE_CLOSE(dir_fd);
>> nit: Here could be empty line (readability).
>>> +	if (file_fd > -1)
>>> +		SAFE_CLOSE(file_fd);
>>> +}
>>> +
>>> +static struct tst_test test = {
>>> +	.test = verify_statx,
>>> +	.setup = setup,
>>> +	.cleanup = cleanup,
>>> +	.mntpoint = MNTPOINT,
>>> +	.mount_device = 1,
>>> +	.all_filesystems = 1,
>>> +	.needs_root = 1,
>>> +	.tcnt = ARRAY_SIZE(tcases)
>>> +};
>>
>> All my suggestion (ok to ignore).
> 
> I agree with your all suggestion.
> 
> Best Regards
> Yang Xu
>>
>> Kind regards,
>> Petr
>>
>> diff --git testcases/kernel/syscalls/statx/statx12.c testcases/kernel/syscalls/statx/statx12.c
>> index 40367c8b1..6b76b6e49 100644
>> --- testcases/kernel/syscalls/statx/statx12.c
>> +++ testcases/kernel/syscalls/statx/statx12.c
>> @@ -12,12 +12,10 @@
>>     * This flag indicates whether the path or fd refers to the root of a mount
>>     * or not.
>>     *
>> - * Minimum Linux version required is v5.10.
>> + * Minimum Linux version required is v5.8.
>>     */
>>    
>>    #define _GNU_SOURCE
>> -#include <sys/types.h>
>> -#include <sys/mount.h>
>>    #include <unistd.h>
>>    #include <stdlib.h>
>>    #include <stdbool.h>
>> @@ -32,32 +30,27 @@ static int dir_fd = -1, file_fd = -1;
>>    
>>    static struct tcase {
>>    	const char *path;
>> -	int mnt_root;
>> -	int flag;
>> +	bool mnt_root;
>>    	int *fd;
>>    } tcases[] = {
>> -	{MNTPOINT, 1, 1, &dir_fd},
>> -	{MNTPOINT, 1, 0, &dir_fd},
>> -	{TESTFILE, 0, 1, &file_fd},
>> -	{TESTFILE, 0, 0, &file_fd}
>> +	{MNTPOINT, 1, &dir_fd},
>> +	{TESTFILE, 0, &file_fd}
>>    };
>>    
>>    static void verify_statx(unsigned int n)
>>    {
>> -	struct tcase *tc = &tcases[n];
>> +	struct tcase *tc = &tcases[n/2];
>>    	struct statx buf;
>> +	bool flag = n % 2;
>>    
>> -	if (tc->flag) {
>> -		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
>> +	if (flag) {
>> +		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
>>    				tc->path);
>> -
>> -		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf),
>> -			"statx(AT_FDCWD, %s, 0, 0, &buf)", tc->path);
>> +		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf));
>>    	} else {
>> -		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
>> +		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
>>    				tc->path);
>> -		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf),
>> -			"statx(%d, "", 0, 0, &buf)", *tc->fd);
>> +		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf));
>>    	}
>>    
>>    	if (!(buf.stx_attributes_mask & STATX_ATTR_MOUNT_ROOT)) {
>> @@ -85,6 +78,7 @@ static void cleanup(void)
>>    {
>>    	if (dir_fd > -1)
>>    		SAFE_CLOSE(dir_fd);
>> +
>>    	if (file_fd > -1)
>>    		SAFE_CLOSE(file_fd);
>>    }
>> @@ -97,5 +91,5 @@ static struct tst_test test = {
>>    	.mount_device = 1,
>>    	.all_filesystems = 1,
>>    	.needs_root = 1,
>> -	.tcnt = ARRAY_SIZE(tcases)
>> +	.tcnt = 2* ARRAY_SIZE(tcases)
>>    };
>
Yang Xu June 25, 2023, 2:47 p.m. UTC | #4 
Hi  Petr


Thanks for you review, merged with your suggestion.

Best Regards
Yang Xu
> Hi Xu,
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Tested-by: Petr Vorel <pvorel@suse.cz>
> 
> I tested it on recent kernel 6.3.1 and very old kernel 3.16.0.
> Works as expected.
> 
> 
>> This flag was introduced since kernel 5.10 and was used to indicates
> 
> 80340fe3605c ("statx: add mount_root") was released in v5.8. Unless you refer to
> something different, please update it here.
> 
>> whether the path or fd refers to the root of a mount or not.
> 
>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>> ---
>>   include/lapi/stat.h                        |   4 +
>>   runtest/syscalls                           |   1 +
>>   testcases/kernel/syscalls/statx/.gitignore |   1 +
>>   testcases/kernel/syscalls/statx/statx12.c  | 101 +++++++++++++++++++++
>>   4 files changed, 107 insertions(+)
>>   create mode 100644 testcases/kernel/syscalls/statx/statx12.c
> 
>> diff --git a/include/lapi/stat.h b/include/lapi/stat.h
>> index c7e6fdac0..3606c9eb0 100644
>> --- a/include/lapi/stat.h
>> +++ b/include/lapi/stat.h
>> @@ -221,6 +221,10 @@ static inline int statx(int dirfd, const char *pathname, unsigned int flags,
>>   # define STATX_ATTR_AUTOMOUNT	0x00001000
>>   #endif
> 
>> +#ifndef STATX_ATTR_MOUNT_ROOT
>> +# define STATX_ATTR_MOUNT_ROOT	0x00002000
>> +#endif
>> +
>>   #ifndef STATX_ATTR_VERITY
>>   # define STATX_ATTR_VERITY	0x00100000
>>   #endif
>> diff --git a/runtest/syscalls b/runtest/syscalls
>> index e5ad2c2f9..0c1993421 100644
>> --- a/runtest/syscalls
>> +++ b/runtest/syscalls
>> @@ -1767,6 +1767,7 @@ statx08 statx08
>>   statx09 statx09
>>   statx10 statx10
>>   statx11 statx11
>> +statx12 statx12
> 
>>   membarrier01 membarrier01
> 
>> diff --git a/testcases/kernel/syscalls/statx/.gitignore b/testcases/kernel/syscalls/statx/.gitignore
>> index 48ac4078b..f6a423eed 100644
>> --- a/testcases/kernel/syscalls/statx/.gitignore
>> +++ b/testcases/kernel/syscalls/statx/.gitignore
>> @@ -9,3 +9,4 @@
>>   /statx09
>>   /statx10
>>   /statx11
>> +/statx12
>> diff --git a/testcases/kernel/syscalls/statx/statx12.c b/testcases/kernel/syscalls/statx/statx12.c
>> new file mode 100644
>> index 000000000..ae6bbb1e2
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/statx/statx12.c
>> @@ -0,0 +1,101 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
>> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * It is a basic test for STATX_ATTR_MOUNT_ROOT flag.
>> + *
>> + * This flag indicates whether the path or fd refers to the root of a mount
>> + * or not.
>> + *
>> + * Minimum Linux version required is v5.10.
> And here as well.
>> + */
>> +
>> +#define _GNU_SOURCE
>> +#include <sys/types.h>
>> +#include <sys/mount.h>
> Do we need these two for anything?
> 
>> +#include <unistd.h>
>> +#include <stdlib.h>
>> +#include <stdbool.h>
>> +#include <stdio.h>
>> +#include "tst_test.h"
>> +#include "lapi/stat.h"
>> +
>> +#define MNTPOINT "mntpoint"
>> +#define TESTFILE MNTPOINT"/testfile"
>> +
>> +static int dir_fd = -1, file_fd = -1;
>> +
>> +static struct tcase {
>> +	const char *path;
>> +	int mnt_root;
>> +	int flag;
> Since you're already using <stdbool.h>, mnt_root and flag could be bool.
>> +	int *fd;
>> +} tcases[] = {
>> +	{MNTPOINT, 1, 1, &dir_fd},
>> +	{MNTPOINT, 1, 0, &dir_fd},
>> +	{TESTFILE, 0, 1, &file_fd},
>> +	{TESTFILE, 0, 0, &file_fd}
> I'd even consider replacing int flag in struct with counted from n:
> 
> static struct tcase {
> 	const char *path;
> 	bool mnt_root;
> 	int *fd;
> } tcases[] = {
> 	{MNTPOINT, 1, &dir_fd},
> 	{TESTFILE, 0, &file_fd}
> };
> 
> static void verify_statx(unsigned int n)
> {
> 	struct tcase *tc = &tcases[n/2];
> 	struct statx buf;
> 	bool flag = n % 2;
> 
> 	if (flag) {
> 		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
> 				tc->path);
> 		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf));
> 	} else {
> 		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
> 				tc->path);
> 
> 		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf));
> 	}
> 	...
> 
> static struct tst_test test = {
> 	...
> 	.tcnt = 2* ARRAY_SIZE(tcases)
> 
> 
>> +};
>> +
>> +static void verify_statx(unsigned int n)
>> +{
>> +	struct tcase *tc = &tcases[n];
>> +	struct statx buf;
>> +
>> +	if (tc->flag) {
>> +		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
>> +				tc->path);
>> +
>> +		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf),
>> +			"statx(AT_FDCWD, %s, 0, 0, &buf)", tc->path);
> nit: I wonder if we need to duplicate the call in string, just to get tc->path,
> which we have printed in TINFO above. Wouldn't be enough just:
> 		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf));
> 
>> +	} else {
>> +		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
>> +				tc->path);
>> +		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf),
>> +			"statx(%d, "", 0, 0, &buf)", *tc->fd);
> Well, here you probably meant
> 		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf),
> 			"statx(%d, \"\", 0, 0, &buf)", *tc->fd);
> checkpatch.pl (via make check-statx12) warns about it (slightly cryptic):
> statx12.c:60: WARNING: Consecutive strings are generally better as a single string
> TODO: this is the second thing which should be fixed before merge.
> 
> But again, I'd go just for:
> 		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf));
>> +	}
>> +
>> +	if (!(buf.stx_attributes_mask & STATX_ATTR_MOUNT_ROOT)) {
>> +		tst_res(TCONF, "Filesystem does not support STATX_ATTR_MOUNT_ROOT");
>> +		return;
>> +	}
>> +
>> +	if (buf.stx_attributes & STATX_ATTR_MOUNT_ROOT) {
>> +		tst_res(tc->mnt_root ? TPASS : TFAIL,
>> +			"STATX_ATTR_MOUNT_ROOT flag is set");
>> +	} else {
>> +		tst_res(tc->mnt_root ? TFAIL : TPASS,
>> +			"STATX_ATTR_MOUNT_ROOT flag is not set");
>> +	}
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	SAFE_CREAT(TESTFILE, 0755);
>> +	dir_fd = SAFE_OPEN(MNTPOINT, O_DIRECTORY);
>> +	file_fd = SAFE_OPEN(TESTFILE, O_RDWR);
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	if (dir_fd > -1)
>> +		SAFE_CLOSE(dir_fd);
> nit: Here could be empty line (readability).
>> +	if (file_fd > -1)
>> +		SAFE_CLOSE(file_fd);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.test = verify_statx,
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.mntpoint = MNTPOINT,
>> +	.mount_device = 1,
>> +	.all_filesystems = 1,
>> +	.needs_root = 1,
>> +	.tcnt = ARRAY_SIZE(tcases)
>> +};
> 
> All my suggestion (ok to ignore).
> 
> Kind regards,
> Petr
> 
> diff --git testcases/kernel/syscalls/statx/statx12.c testcases/kernel/syscalls/statx/statx12.c
> index 40367c8b1..6b76b6e49 100644
> --- testcases/kernel/syscalls/statx/statx12.c
> +++ testcases/kernel/syscalls/statx/statx12.c
> @@ -12,12 +12,10 @@
>    * This flag indicates whether the path or fd refers to the root of a mount
>    * or not.
>    *
> - * Minimum Linux version required is v5.10.
> + * Minimum Linux version required is v5.8.
>    */
>   
>   #define _GNU_SOURCE
> -#include <sys/types.h>
> -#include <sys/mount.h>
>   #include <unistd.h>
>   #include <stdlib.h>
>   #include <stdbool.h>
> @@ -32,32 +30,27 @@ static int dir_fd = -1, file_fd = -1;
>   
>   static struct tcase {
>   	const char *path;
> -	int mnt_root;
> -	int flag;
> +	bool mnt_root;
>   	int *fd;
>   } tcases[] = {
> -	{MNTPOINT, 1, 1, &dir_fd},
> -	{MNTPOINT, 1, 0, &dir_fd},
> -	{TESTFILE, 0, 1, &file_fd},
> -	{TESTFILE, 0, 0, &file_fd}
> +	{MNTPOINT, 1, &dir_fd},
> +	{TESTFILE, 0, &file_fd}
>   };
>   
>   static void verify_statx(unsigned int n)
>   {
> -	struct tcase *tc = &tcases[n];
> +	struct tcase *tc = &tcases[n/2];
>   	struct statx buf;
> +	bool flag = n % 2;
>   
> -	if (tc->flag) {
> -		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
> +	if (flag) {
> +		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
>   				tc->path);
> -
> -		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf),
> -			"statx(AT_FDCWD, %s, 0, 0, &buf)", tc->path);
> +		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf));
>   	} else {
> -		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
> +		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
>   				tc->path);
> -		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf),
> -			"statx(%d, "", 0, 0, &buf)", *tc->fd);
> +		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf));
>   	}
>   
>   	if (!(buf.stx_attributes_mask & STATX_ATTR_MOUNT_ROOT)) {
> @@ -85,6 +78,7 @@ static void cleanup(void)
>   {
>   	if (dir_fd > -1)
>   		SAFE_CLOSE(dir_fd);
> +
>   	if (file_fd > -1)
>   		SAFE_CLOSE(file_fd);
>   }
> @@ -97,5 +91,5 @@ static struct tst_test test = {
>   	.mount_device = 1,
>   	.all_filesystems = 1,
>   	.needs_root = 1,
> -	.tcnt = ARRAY_SIZE(tcases)
> +	.tcnt = 2* ARRAY_SIZE(tcases)
>   };
diff mbox series

Patch

diff --git a/include/lapi/stat.h b/include/lapi/stat.h
index c7e6fdac0..3606c9eb0 100644
--- a/include/lapi/stat.h
+++ b/include/lapi/stat.h
@@ -221,6 +221,10 @@  static inline int statx(int dirfd, const char *pathname, unsigned int flags,
 # define STATX_ATTR_AUTOMOUNT	0x00001000
 #endif
 
+#ifndef STATX_ATTR_MOUNT_ROOT
+# define STATX_ATTR_MOUNT_ROOT	0x00002000
+#endif
+
 #ifndef STATX_ATTR_VERITY
 # define STATX_ATTR_VERITY	0x00100000
 #endif
diff --git a/runtest/syscalls b/runtest/syscalls
index e5ad2c2f9..0c1993421 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1767,6 +1767,7 @@  statx08 statx08
 statx09 statx09
 statx10 statx10
 statx11 statx11
+statx12 statx12
 
 membarrier01 membarrier01
 
diff --git a/testcases/kernel/syscalls/statx/.gitignore b/testcases/kernel/syscalls/statx/.gitignore
index 48ac4078b..f6a423eed 100644
--- a/testcases/kernel/syscalls/statx/.gitignore
+++ b/testcases/kernel/syscalls/statx/.gitignore
@@ -9,3 +9,4 @@ 
 /statx09
 /statx10
 /statx11
+/statx12
diff --git a/testcases/kernel/syscalls/statx/statx12.c b/testcases/kernel/syscalls/statx/statx12.c
new file mode 100644
index 000000000..ae6bbb1e2
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx12.c
@@ -0,0 +1,101 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * It is a basic test for STATX_ATTR_MOUNT_ROOT flag.
+ *
+ * This flag indicates whether the path or fd refers to the root of a mount
+ * or not.
+ *
+ * Minimum Linux version required is v5.10.
+ */
+
+#define _GNU_SOURCE
+#include <sys/types.h>
+#include <sys/mount.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include "tst_test.h"
+#include "lapi/stat.h"
+
+#define MNTPOINT "mntpoint"
+#define TESTFILE MNTPOINT"/testfile"
+
+static int dir_fd = -1, file_fd = -1;
+
+static struct tcase {
+	const char *path;
+	int mnt_root;
+	int flag;
+	int *fd;
+} tcases[] = {
+	{MNTPOINT, 1, 1, &dir_fd},
+	{MNTPOINT, 1, 0, &dir_fd},
+	{TESTFILE, 0, 1, &file_fd},
+	{TESTFILE, 0, 0, &file_fd}
+};
+
+static void verify_statx(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+	struct statx buf;
+
+	if (tc->flag) {
+		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by path",
+				tc->path);
+
+		TST_EXP_PASS_SILENT(statx(AT_FDCWD, tc->path, 0, 0, &buf),
+			"statx(AT_FDCWD, %s, 0, 0, &buf)", tc->path);
+	} else {
+		tst_res(TINFO, "Testing %s with STATX_ATTR_MOUNT_ROOT by fd",
+				tc->path);
+		TST_EXP_PASS_SILENT(statx(*tc->fd, "", AT_EMPTY_PATH, 0, &buf),
+			"statx(%d, "", 0, 0, &buf)", *tc->fd);
+	}
+
+	if (!(buf.stx_attributes_mask & STATX_ATTR_MOUNT_ROOT)) {
+		tst_res(TCONF, "Filesystem does not support STATX_ATTR_MOUNT_ROOT");
+		return;
+	}
+
+	if (buf.stx_attributes & STATX_ATTR_MOUNT_ROOT) {
+		tst_res(tc->mnt_root ? TPASS : TFAIL,
+			"STATX_ATTR_MOUNT_ROOT flag is set");
+	} else {
+		tst_res(tc->mnt_root ? TFAIL : TPASS,
+			"STATX_ATTR_MOUNT_ROOT flag is not set");
+	}
+}
+
+static void setup(void)
+{
+	SAFE_CREAT(TESTFILE, 0755);
+	dir_fd = SAFE_OPEN(MNTPOINT, O_DIRECTORY);
+	file_fd = SAFE_OPEN(TESTFILE, O_RDWR);
+}
+
+static void cleanup(void)
+{
+	if (dir_fd > -1)
+		SAFE_CLOSE(dir_fd);
+	if (file_fd > -1)
+		SAFE_CLOSE(file_fd);
+}
+
+static struct tst_test test = {
+	.test = verify_statx,
+	.setup = setup,
+	.cleanup = cleanup,
+	.mntpoint = MNTPOINT,
+	.mount_device = 1,
+	.all_filesystems = 1,
+	.needs_root = 1,
+	.tcnt = ARRAY_SIZE(tcases)
+};