[LTP] [PATCH v4] syscalls/statx: Add test for sync flags

[LTP] [PATCH v4] syscalls/statx: Add test for sync flags

[Vaishnavi]

Testcase 1: AT_STATX_DONT_SYNC
 With AT_STATX_DONT_SYNC server changes should not get sync with client.

Testcase 2: AT_STATX_FORCE_SYNC
 With AT_STATX_FORCE_SYNC server changes should get sync with client.

Signed-off-by: Kewal Ukunde <kewal@zilogic.com>
Signed-off-by: Vaishnavi.d <vaishnavi.d@zilogic.com>
Signed-off-by: Tarun.T.U <tarun@zilogic.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
---
 runtest/syscalls                           |   1 +
 testcases/kernel/syscalls/statx/.gitignore |   1 +
 testcases/kernel/syscalls/statx/statx07.c  | 170 +++++++++++++++++++++++++++++
 3 files changed, 172 insertions(+)
 create mode 100644 testcases/kernel/syscalls/statx/statx07.c

diff --git a/runtest/syscalls b/runtest/syscalls
index ac1d2d2..957b999 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1524,5 +1524,6 @@ statx03 statx03
 statx04 statx04
 statx05 statx05
 statx06 statx06
+statx07 statx07
 
 membarrier01 membarrier01
diff --git a/testcases/kernel/syscalls/statx/.gitignore b/testcases/kernel/syscalls/statx/.gitignore
index c1cedda..2f5457e 100644
--- a/testcases/kernel/syscalls/statx/.gitignore
+++ b/testcases/kernel/syscalls/statx/.gitignore
@@ -4,3 +4,4 @@
 /statx04
 /statx05
 /statx06
+/statx07
diff --git a/testcases/kernel/syscalls/statx/statx07.c b/testcases/kernel/syscalls/statx/statx07.c
new file mode 100644
index 0000000..b5fbaf3
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx07.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  Copyright (c) Zilogic Systems Pvt. Ltd., 2018
+ *  Email : code@zilogic.com
+ */
+
+/*
+ * Test statx
+ *
+ * This code tests the following flags:
+ * 1) AT_STATX_FORCE_SYNC
+ * 2) AT_STATX_DONT_SYNC
+ *
+ * By exportfs cmd creating NFS setup.
+ *
+ * A test file is created in server folder and statx is being
+ * done in client folder.
+ *
+ * TESTCASE 1:
+ * BY AT_STATX_SYNC_AS_STAT getting predefined mode value.
+
+ * Then, by using AT_STATX_FORCE_SYNC getting new updated vaue
+ * from server file changes.
+ *
+ * TESTCASE 2:
+ * BY AT_STATX_SYNC_AS_STAT getting predefined mode value.
+ * AT_STATX_FORCE_SYNC is called to create cache data of the file.
+ * Then, by using DONT_SYNC_FILE getting old cached data in client folder,
+ * but mode has been chaged in server file.
+ *
+ * Minimum kernel version required is 4.16.
+ */
+
+#include <netdb.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <linux/limits.h>
+#include <sys/mount.h>
+#include "tst_test.h"
+#include "lapi/stat.h"
+#include "old_tmpdir.h"
+
+#define MODE(X) (X & (~S_IFMT))
+#define BUFF_SIZE PATH_MAX
+#define DEFAULT_MODE 0644
+#define CURRENT_MODE 0777
+
+#define CLIENT_PATH "client"
+#define SERVER_PATH "server"
+#define CLI_FORCE_SYNC "client/force_sync_file"
+#define CLI_DONT_SYNC "client/dont_sync_file"
+#define SERV_FORCE_SYNC "server/force_sync_file"
+#define SERV_DONT_SYNC "server/dont_sync_file"
+
+static char *cwd;
+static char cmd[BUFF_SIZE];
+static int mounted;
+
+static int get_mode(char *file_name, int flag_type, char *flag_name)
+{
+	struct statx buff;
+
+	TEST(statx(AT_FDCWD, file_name, flag_type, STATX_ALL, &buff));
+
+	if (TST_RET == -1)
+		tst_brk(TFAIL | TST_ERR,
+			"statx(AT_FDCWD, %s, %s, STATX_ALL, &buff)",
+			file_name, flag_name);
+	else
+		tst_res(TINFO, "statx(AT_FDCWD, %s, %s, STATX_ALL, &buff)",
+			file_name, flag_name);
+
+	return buff.stx_mode;
+}
+
+const struct test_cases {
+	char *server_file;
+	char *client_file;
+	char *flag_name;
+	int flag;
+	unsigned int mode;
+
+} tcases[] = {
+	{SERV_DONT_SYNC,
+	 CLI_DONT_SYNC,
+	 "AT_STATX_DONT_SYNC",
+	 AT_STATX_DONT_SYNC,
+	 DEFAULT_MODE},
+	{SERV_FORCE_SYNC,
+	 CLI_FORCE_SYNC,
+	 "AT_STATX_FORCE_SYNC",
+	 AT_STATX_FORCE_SYNC,
+	 CURRENT_MODE}
+};
+
+static void test_statx(unsigned int nr)
+{
+	const struct test_cases *tc = &tcases[nr];
+	unsigned int cur_mode;
+
+	get_mode(tc->client_file, AT_STATX_FORCE_SYNC, "AT_STATX_FORCE_SYNC");
+
+	SAFE_CHMOD(tc->server_file, CURRENT_MODE);
+	cur_mode = get_mode(tc->client_file, tc->flag, tc->flag_name);
+
+	if (MODE(cur_mode) == tc->mode)
+		tst_res(TPASS,
+			"statx() with %s for mode %o %o",
+			tc->flag_name, tc->mode, MODE(cur_mode));
+	else
+		tst_res(TFAIL,
+			"statx() with %s for mode %o %o",
+			tc->flag_name, tc->mode, MODE(cur_mode));
+}
+
+static void setup(void)
+{
+	int ret;
+	char server_path[BUFF_SIZE];
+
+	cwd = tst_get_tmpdir();
+
+	SAFE_MKDIR(SERVER_PATH, DEFAULT_MODE);
+	SAFE_MKDIR(CLIENT_PATH, DEFAULT_MODE);
+	SAFE_CREAT(SERV_FORCE_SYNC, DEFAULT_MODE);
+	SAFE_CREAT(SERV_DONT_SYNC, DEFAULT_MODE);
+
+	snprintf(server_path, sizeof(server_path), ":%s/%s", cwd, SERVER_PATH);
+
+	snprintf(cmd, sizeof(cmd),
+		 "exportfs -i -o no_root_squash,rw,sync,no_subtree_check *%.1024s",
+		 server_path);
+
+	ret = tst_system(cmd);
+	if (WEXITSTATUS(ret) == 127)
+		tst_brk(TCONF | TST_ERR, "%s not found", cmd);
+	if (ret)
+		tst_brk(TBROK | TST_ERR, "failed to exportfs");
+
+	if (mount(server_path, CLIENT_PATH, "nfs", 0, "addr=127.0.0.1")) {
+		if (errno == EOPNOTSUPP)
+			tst_brk(TCONF | TERRNO, "nfs server not set up?");
+		tst_brk(TBROK | TERRNO, "mount() nfs failed");
+	}
+	mounted = 1;
+}
+
+static void cleanup(void)
+{
+	snprintf(cmd, sizeof(cmd),
+		 "exportfs -u *:%s/%s", cwd, SERVER_PATH);
+
+	if (tst_system(cmd) == -1)
+		tst_res(TWARN | TST_ERR, "failed to clear exportfs");
+
+	if (mounted)
+		SAFE_UMOUNT(CLIENT_PATH);
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = test_statx,
+	.setup = setup,
+	.cleanup = cleanup,
+	.min_kver = "4.16",
+	.needs_tmpdir = 1,
+	.dev_fs_type = "nfs",
+	.needs_root = 1,
+};
-- 
2.1.4

[LTP] [PATCH v4] syscalls/statx: Add test for sync flags

[Petr Vorel]

Hi Vaishnavi,


> Testcase 1: AT_STATX_DONT_SYNC
>  With AT_STATX_DONT_SYNC server changes should not get sync with client.

> Testcase 2: AT_STATX_FORCE_SYNC
>  With AT_STATX_FORCE_SYNC server changes should get sync with client.

> Signed-off-by: Kewal Ukunde <kewal@zilogic.com>
> Signed-off-by: Vaishnavi.d <vaishnavi.d@zilogic.com>
> Signed-off-by: Tarun.T.U <tarun@zilogic.com>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
Now reviewing it :).

...
> + * Minimum kernel version required is 4.16.
statx() was merged into 4.11 in commit a528d35e8bfc ("statx: Add a system call
to make enhanced file info available" as you had in v1. Why now requiring 4.16?
It looks like second test fails now on 4.11, because calling get_mode() for it
as well (see bellow).

BTW maybe .min_kver = "4.16" is self describing.

...
> +#include "old_tmpdir.h"
I think, we don't want to use old_*.h headers (although library itself
lib/tst_test.c uses it). IMHO previous version with getcwd() was better.

> +#define CLIENT_PATH "client"
> +#define SERVER_PATH "server"
> +#define CLI_FORCE_SYNC "client/force_sync_file"
> +#define CLI_DONT_SYNC "client/dont_sync_file"
> +#define SERV_FORCE_SYNC "server/force_sync_file"
> +#define SERV_DONT_SYNC "server/dont_sync_file"
Code style: SERVER_* vs. SERV_*, CLIENT_* vs. CLI_*. Can you unify them?
(pick only one variant for each?).

> +static int get_mode(char *file_name, int flag_type, char *flag_name)
I don't like passing flag and it's name, but no idea, how to avoid it.
> +{
> +	struct statx buff;
> +
> +	TEST(statx(AT_FDCWD, file_name, flag_type, STATX_ALL, &buff));
> +
> +	if (TST_RET == -1)
> +		tst_brk(TFAIL | TST_ERR,
> +			"statx(AT_FDCWD, %s, %s, STATX_ALL, &buff)",
> +			file_name, flag_name);
> +	else
> +		tst_res(TINFO, "statx(AT_FDCWD, %s, %s, STATX_ALL, &buff)",
> +			file_name, flag_name);
> +
> +	return buff.stx_mode;
> +}

> +const struct test_cases {
> +	char *server_file;
> +	char *client_file;
> +	char *flag_name;
> +	int flag;
> +	unsigned int mode;
> +
> +} tcases[] = {
> +	{SERV_DONT_SYNC,
> +	 CLI_DONT_SYNC,
> +	 "AT_STATX_DONT_SYNC",
> +	 AT_STATX_DONT_SYNC,
> +	 DEFAULT_MODE},
> +	{SERV_FORCE_SYNC,
> +	 CLI_FORCE_SYNC,
> +	 "AT_STATX_FORCE_SYNC",
> +	 AT_STATX_FORCE_SYNC,
> +	 CURRENT_MODE}
> +};
flag_name and flag is duplicity.

We often use macro for this purpose:

testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep01.c
#define TYPE_NAME(x) .ttype = x, .desc = #x
...
static struct test_case tcase[] = {
	{
		TYPE_NAME(NORMAL),

Note about code style:
tcases[] formatting: { } should be on single line.
If you can, use shorter names (it's C).

> +static void test_statx(unsigned int nr)
Maybe i?
static void test_statx(unsigned int nr)

> +{
> +	const struct test_cases *tc = &tcases[nr];
> +	unsigned int cur_mode;
> +
> +	get_mode(tc->client_file, AT_STATX_FORCE_SYNC, "AT_STATX_FORCE_SYNC");
> +
> +	SAFE_CHMOD(tc->server_file, CURRENT_MODE);
> +	cur_mode = get_mode(tc->client_file, tc->flag, tc->flag_name);

This helps to be 4.11 compatible:
	if (tc->flag == AT_STATX_DONT_SYNC)
		get_mode(tc->client_file, AT_STATX_FORCE_SYNC, "AT_STATX_FORCE_SYNC");

> +	if (MODE(cur_mode) == tc->mode)
> +		tst_res(TPASS,
> +			"statx() with %s for mode %o %o",
> +			tc->flag_name, tc->mode, MODE(cur_mode));
> +	else
> +		tst_res(TFAIL,
> +			"statx() with %s for mode %o %o",
> +			tc->flag_name, tc->mode, MODE(cur_mode));
> +}
...

> +static void setup(void)
> +{
...
> +	snprintf(cmd, sizeof(cmd),
> +		 "exportfs -i -o no_root_squash,rw,sync,no_subtree_check *%.1024s",
> +		 server_path);
Maybe whole cleanup should be skipped if exportfs fails?
	exported = 1;

> +
> +	ret = tst_system(cmd);
> +	if (WEXITSTATUS(ret) == 127)
> +		tst_brk(TCONF | TST_ERR, "%s not found", cmd);
> +	if (ret)
> +		tst_brk(TBROK | TST_ERR, "failed to exportfs");
> +
> +	if (mount(server_path, CLIENT_PATH, "nfs", 0, "addr=127.0.0.1")) {
> +		if (errno == EOPNOTSUPP)
> +			tst_brk(TCONF | TERRNO, "nfs server not set up?");
> +		tst_brk(TBROK | TERRNO, "mount() nfs failed");
> +	}
> +	mounted = 1;
> +}

> +static void cleanup(void)
> +{
	Here skip cleanup if exportfs during setup failed:
	if (!exported)
		return;

> +	snprintf(cmd, sizeof(cmd),
> +		 "exportfs -u *:%s/%s", cwd, SERVER_PATH);
> +
> +	if (tst_system(cmd) == -1)
> +		tst_res(TWARN | TST_ERR, "failed to clear exportfs");
> +
> +	if (mounted)
> +		SAFE_UMOUNT(CLIENT_PATH);
> +}
...


Kind regards,
Petr

[LTP] [PATCH v4] syscalls/statx: Add test for sync flags

[Cyril Hrubis]

Hi!
> > +#include "old_tmpdir.h"
> I think, we don't want to use old_*.h headers (although library itself
> lib/tst_test.c uses it). IMHO previous version with getcwd() was better.

My bad it does not seem to be exported to the new library, I thought it
was. I will fix the headers so that it's exported and remove this
include before applying this patch.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v4] syscalls/statx: Add test for sync flags

[vaishnavi.d]

Hi,

>> + * Minimum kernel version required is 4.16.
> statx() was merged into 4.11 in commit a528d35e8bfc ("statx: Add a system
> call
> to make enhanced file info available" as you had in v1. Why now requiring
> 4.16?
> It looks like second test fails now on 4.11, because calling get_mode()
> for it
> as well (see bellow).

Actually, support for sync flags was added only from kernel version 4.16.
Since, I was using 4.16, I didn't face that issue. Thats why minimum kernel
version has been updated to 4.16 in the patch v5.
(https://github.com/torvalds/linux/commit/9ccee940bd5b766b6dab6c1a80908b9490a4850d)



>> +#include "old_tmpdir.h"
> I think, we don't want to use old_*.h headers (although library itself
> lib/tst_test.c uses it). IMHO previous version with getcwd() was better.
>

The function get tempdir will be ported to the new library soon. So, I
have not modified that part. Other changes have been made in patch v5.

Regards,
Vaishnavi D

[LTP] [PATCH v4] syscalls/statx: Add test for sync flags

[Petr Vorel]

Hi Vaishnavi,

> Actually, support for sync flags was added only from kernel version 4.16.
> Since, I was using 4.16, I didn't face that issue. Thats why minimum kernel
> version has been updated to 4.16 in the patch v5.
> (https://github.com/torvalds/linux/commit/9ccee940bd5b766b6dab6c1a80908b9490a4850d)
Thanks for update.

> >> +#include "old_tmpdir.h"
> > I think, we don't want to use old_*.h headers (although library itself
> > lib/tst_test.c uses it). IMHO previous version with getcwd() was better.

> The function get tempdir will be ported to the new library soon. So, I
> have not modified that part. Other changes have been made in patch v5.
Good, I noticed Cyril's intention to fix the headers. We'll just wait for it and
remove import old_tmpdir.h before merging v5.

> Regards,
> Vaishnavi D

Kind regards,
Petr