Skip to contentSkip to conversation

Test statx syscall for SYNC_FLAGS

PATCH New kewal 3 participants 6 messages Sep 17, 2018 – Dec 26, 2018

lore
Share & tools
lore

kewal

author
#0 lore raw

PATCH v2 1/1 Test statx syscall for SYNC_FLAGS

* statx06.c :-

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 <[email protected]>
Signed-off-by Vaishnavi.d <[email protected]>
Signed-off-by Tarun.T.U <[email protected]>
---
 runtest/syscalls                           |   1 +
 testcases/kernel/syscalls/statx/.gitignore |   1 +
 testcases/kernel/syscalls/statx/statx06.c  | 195 +++++++++++++++++++++++++++++
 3 files changed, 197 insertions(+)
 create mode 100644 testcases/kernel/syscalls/statx/statx06.c
runtest/syscalls +1 -0
index 0d0be7713..71b83b25c 100644
@@ -1504,3 +1504,4 @@ statx02 statx02
 statx03 statx03
 statx04 statx04
 statx05 statx05
+statx06 statx06
testcases/kernel/syscalls/statx/.gitignore +1 -0
index 209fc3a33..c1ceddabf 100644
@@ -3,3 +3,4 @@
 /statx03
 /statx04
 /statx05
+/statx06
testcases/kernel/syscalls/statx/statx06.c new file +195 -0

195 lines not on this page · raw message

Signature · 1 line
2.11.0
#1 lore raw

PATCH v2 1/1 Test statx syscall for SYNC_FLAGS

Hi Kewal,

LGTM, bellow are some minor enhancement tips.

Quoted 1 line

* statx06.c :-

This could be deleted.

Maybe first line of commit message "syscalls/statx: Add test for sync flags" (SYNC_FLAGS looks like a constant, which is misleading).

Quoted 9 lines

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 <[email protected]>
Signed-off-by Vaishnavi.d <[email protected]>
Signed-off-by Tarun.T.U <[email protected]>
Reviewed-by Petr Vorel <[email protected]>
Quoted 6 lines
---
 runtest/syscalls                           |   1 +
 testcases/kernel/syscalls/statx/.gitignore |   1 +
 testcases/kernel/syscalls/statx/statx06.c  | 195 +++++++++++++++++++++++++++++
 3 files changed, 197 insertions(+)
 create mode 100644 testcases/kernel/syscalls/statx/statx06.c

It's needed to rename statx06.c to statx07.c (as previous was taken).

...

Quoted 1 line
testcases/kernel/syscalls/statx/statx06.c +0 -0

...

Quoted 43 lines
testcases/kernel/syscalls/statx/statx06.c +43 -0
+static void test_for_dont_sync(void)
+{
+	unsigned int cur_mode;
+	char server_path[F_PATH_SIZE];
+
+	snprintf(server_path, sizeof(server_path), "%s/%s", SERVER_PATH,
+		 DONT_SYNC_FILE);
+
+	get_mode(DONT_SYNC_FILE, AT_STATX_FORCE_SYNC,
+		 "AT_STATX_FORCE_SYNC");
+
+	SAFE_CHMOD(server_path, CURRENT_MODE);
+	cur_mode = get_mode(DONT_SYNC_FILE, AT_STATX_DONT_SYNC,
+			    "AT_STATX_DONT_SYNC");
+
+	if (MODE(cur_mode) == DEFAULT_MODE)
+		tst_res(TPASS,
+			"statx() with AT_STATX_DONT_SYNC for mode %o %o",
+			DEFAULT_MODE, MODE(cur_mode));
+	else
+		tst_res(TFAIL,
+			"statx() with AT_STATX_DONT_SYNC for mode %o %o",
+			DEFAULT_MODE, MODE(cur_mode));
+}
+
+static void test_for_force_sync(void)
+{
+	unsigned int cur_mode;
+	char server_path[F_PATH_SIZE];
+
+	snprintf(server_path, sizeof(server_path), "%s/%s", SERVER_PATH,
+		 FORCE_SYNC_FILE);
+
+	SAFE_CHMOD(server_path, CURRENT_MODE);
+	cur_mode = get_mode(FORCE_SYNC_FILE, AT_STATX_FORCE_SYNC,
+			    "AT_STATX_FORCE_SYNC");
+
+	if (MODE(cur_mode) == CURRENT_MODE)
+		tst_res(TPASS,
+			"statx() with AT_STATX_FORCE_SYNC for mode %o %o",
+			CURRENT_MODE, MODE(cur_mode));
+	else
+		tst_res(TFAIL,

testcases/kernel/syscalls/statx/statx06.c

Quoted 10 lines
testcases/kernel/syscalls/statx/statx06.c +10 -0
+			"statx() with AT_STATX_FORCE_SYNC for mode %o %o",
+			CURRENT_MODE, MODE(cur_mode));
+}
+
+const struct test_cases {
+	void (*func)(void);
+} tcases[] = {
+	{test_for_dont_sync},
+	{test_for_force_sync}
+};

Both testing functions are nearly the same, why to repeat yourself? How about, instead of passing test function pointer, pass mode and other needed parameters to single test function?

testcases/kernel/syscalls/statx/statx06.c

Quoted 7 lines
testcases/kernel/syscalls/statx/statx06.c +7 -0
+
+static void test_statx(unsigned int nr)
+{
+	const struct test_cases *tc = &tcases[nr];
+
+	tc->func();
+}

I suggest other changes (see following diff):

* Add mount flag to avoid false positive warning:
safe_macros.c:773: WARN: statx07.c:181: umount(client) failed: EINVAL

* Fix gcc-8 format-truncation error (1024 is far too much, it could be less).

* Use tst_res(TWARN, ...) for exportfs.

Kind regards,
Petr

kernel/syscalls/statx/statx07.c +6 -3
index f64322d7d..ff988fd00 100644
@@ -51,6 +51,7 @@
 
 static char cwd[PATH_MAX];
 static char cmd[BUFF_SIZE];
+static int mounted;
 
 static int get_mode(char *file_name, int flag_type, char *flag_name)
 {

@@ -163,7 +162,7 @@ static void setup(void)
 	snprintf(mount_data, sizeof(mount_data), "addr=%s", ip);
 
 	snprintf(cmd, sizeof(cmd),
-		 "exportfs -i -o no_root_squash,rw,sync,no_subtree_check *%s",
+		 "exportfs -i -o no_root_squash,rw,sync,no_subtree_check *%.1024s",
 		 server_path);
 
 	ret = tst_system(cmd);

@@ -173,14 +172,16 @@ static void setup(void)
 		tst_brk(TBROK | TST_ERR, "failed to exportfs");
 
 	SAFE_MOUNT(server_path, client_path, "nfs", 0, mount_data);
+	mounted = 1;
 }
 
 static void cleanup(void)
 {
 	if (tst_system("exportfs -ua") == -1)
-		tst_brk(TBROK | TST_ERR, "failed to clear exportfs");
+		tst_res(TWARN | TST_ERR, "failed to clear exportfs");
 
-	SAFE_UMOUNT(CLIENT_PATH);
+	if (mounted)
+		SAFE_UMOUNT(CLIENT_PATH);
 }
 
 static struct tst_test test = {
#2 lore raw

PATCH v2 1/1 Test statx syscall for SYNC_FLAGS

In reply to kewal

Hi Kewal,

Quoted 1 line

+++ b/testcases/kernel/syscalls/statx/statx07.c

...

Quoted 2 lines

+ * Minimum kernel version required is 4.11.
+ */

One more thing, you need to define _GNU_SOURCE here as it's needed for glibc and thus used for detection in m4/ltp-statx.m4.

#define _GNU_SOURCE

testcases/kernel/syscalls/statx/statx06.c line 39

Quoted 7 lines
testcases/kernel/syscalls/statx/statx06.c new file +7 -0
+#include <netdb.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <linux/limits.h>
+#include <sys/mount.h>
+#include "tst_test.h"
+#include "lapi/stat.h"

Without it it fails on glibc >= 2.28
https://travis-ci.org/pevik/ltp/builds/470077800

I thing it'd be better to remove _GNU_SOURCE from statx0*.c and put it into lapi/stat.h.

Kind regards,
Petr

#3 lore raw

PATCH v2 1/1 Test statx syscall for SYNC_FLAGS

Hi,

Quoted 25 lines

Hi Kewal,

Quoted 1 line

+++ b/testcases/kernel/syscalls/statx/statx07.c

...

Quoted 2 lines

+ * Minimum kernel version required is 4.11.
+ */

One more thing, you need to define _GNU_SOURCE here as it's needed for glibc and
thus used for detection in m4/ltp-statx.m4.

#define _GNU_SOURCE

testcases/kernel/syscalls/statx/statx06.c line 39

Quoted 7 lines
testcases/kernel/syscalls/statx/statx06.c new file +7 -0
+#include <netdb.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <linux/limits.h>
+#include <sys/mount.h>
+#include "tst_test.h"
+#include "lapi/stat.h"

Without it it fails on glibc >= 2.28
https://travis-ci.org/pevik/ltp/builds/470077800

Quoted 3 lines

I thing it'd be better to remove _GNU_SOURCE from statx0*.c and put it into
lapi/stat.h.

These changes have already been made in v5 of this test case. This patch has been renamed to syscalls/statx: Add test for sync flags. Waiting for the tst_get_tmpdir() function to be ported to new library for the patch to be merged.

Regards,
Vaishnavi D

#4 lore raw

PATCH v2 1/1 Test statx syscall for SYNC_FLAGS

Hi Kewal,

Quoted 18 lines
Quoted 17 lines

One more thing, you need to define _GNU_SOURCE here as it's needed for glibc and
thus used for detection in m4/ltp-statx.m4.

#define _GNU_SOURCE

testcases/kernel/syscalls/statx/statx06.c line 39

Quoted 7 lines
testcases/kernel/syscalls/statx/statx06.c new file +7 -0
+#include <netdb.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <linux/limits.h>
+#include <sys/mount.h>
+#include "tst_test.h"
+#include "lapi/stat.h"

Without it it fails on glibc >= 2.28
https://travis-ci.org/pevik/ltp/builds/470077800

Quoted 8 lines
Quoted 3 lines

I thing it'd be better to remove _GNU_SOURCE from statx0*.c and put it into
lapi/stat.h.

These changes have already been made in v5 of this test case. This patch has been renamed to syscalls/statx: Add test for sync flags. Waiting for the tst_get_tmpdir() function to be ported to new library for the patch to be merged.

ok, I'm sorry, I got confused as you're both working on the same test. But the problem remain even on v5 [1]: missing _GNU_SOURCE definition while using lapi/stat.h.

Quoted 2 lines

Regards,
Vaishnavi D

Kind regards,
Petr

[1] https://patchwork.ozlabs.org/patch/1011904/

#5 lore raw

PATCH v2 1/1 Test statx syscall for SYNC_FLAGS

Hi,

Quoted 4 lines

ok, I'm sorry, I got confused as you're both working on the same test. But the problem remain even on v5 [1]: missing _GNU_SOURCE definition while
using lapi/stat.h.

Sorry, I failed to check that. Have added _GNU_SOURCE definition in lapi/stat.h in patch v6.

Regards,
Vaishnavi D