[LTP] [PATCH v3 0/3] Fix ioctl_ficlone(range) failures on certain FS

[LTP] [PATCH v3 0/3] Fix ioctl_ficlone(range) failures on certain FS

[Andrea Cervesato]

Filesystems implementing copy-on-write are usually supported by the kernel
from a certain version. This is the case of XFS which carried an
experimental support for CoW from 4.9 until 4.16.
At the same time, some mkfs tools might not support CoW before a certain
version. This is the case of mkfs.xfs < 1.5.0.

For these reasons, this LTP patch series is meant to cover a particular
scenario, where kernel and mkfs tools versions have to be cross-checked
in order to run ioctl_ficlone(range) tests.

Two new flags have been added to the tst_fs struct:
- mkfs_ver: test the mkfs tool version before using it and eventually
  skip test if it's not satisfied
- min_kver: test the minimum kernel version and skip the test if running
  kernel is too old

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
Changes in v3:
- invert skip flag logic in check_kver and tst_check_cmd, so now it
  TBROK if defined
- test skip if FS is not supported by kernel
- remove bcachefs version check
- Link to v2: https://lore.kernel.org/r/20241001-ioctl_ficlone01_fix-v2-0-dd0b021dd31d@suse.com

Changes in v2:
- add mkfs_ver
- add min_kver
- fix mkfs.xfs version when using CoW
- fix bcachefs version
- Link to v1: https://lore.kernel.org/r/20240924-ioctl_ficlone01_fix-v1-1-7741e2e13cc2@suse.com

---
Andrea Cervesato (3):
      Filter mkfs version in tst_fs
      Add minimum kernel requirement for FS setup
      Setup minimal kernel for ioctl_clone(range) tests

 include/tst_private.h                              |   6 +-
 include/tst_test.h                                 |   9 ++
 lib/tst_cmd.c                                      | 125 ++++++++++++++-------
 lib/tst_test.c                                     |  44 ++++++--
 testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c  |   4 +-
 testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c  |   4 +-
 .../kernel/syscalls/ioctl/ioctl_ficlonerange01.c   |   4 +-
 .../kernel/syscalls/ioctl/ioctl_ficlonerange02.c   |   4 +-
 testcases/lib/tst_run_shell.c                      |  10 ++
 9 files changed, 154 insertions(+), 56 deletions(-)
---
base-commit: 701212f08d6e850457cc2b10238f28662ef48471
change-id: 20240924-ioctl_ficlone01_fix-88a17ef58543

Best regards,
-- 
Andrea Cervesato <andrea.cervesato@suse.com>


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

[LTP] [PATCH v3 1/3] Filter mkfs version in tst_fs

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

Introduce mkfs_ver attribute in the tst_fs declaration, in order to
filter specific mkfs.* tools versions.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 include/tst_private.h         |   6 +-
 include/tst_test.h            |   4 ++
 lib/tst_cmd.c                 | 125 ++++++++++++++++++++++++++++--------------
 lib/tst_test.c                |  17 +++++-
 testcases/lib/tst_run_shell.c |   5 ++
 5 files changed, 111 insertions(+), 46 deletions(-)

diff --git a/include/tst_private.h b/include/tst_private.h
index 6f4f39b15..4c6479f4b 100644
--- a/include/tst_private.h
+++ b/include/tst_private.h
@@ -40,11 +40,11 @@ char tst_kconfig_get(const char *confname);
 
 /*
  * If cmd argument is a single command, this function just checks command
- * whether exists. If not, case skips.
+ * whether exists. If not, case breaks if brk_nosupp is defined.
  * If cmd argument is a complex string ie 'mkfs.ext4 >= 1.43.0', this
  * function checks command version whether meets this requirement.
- * If not, case skips.
+ * If not, case breaks if brk_nosupp is defined.
  */
-void tst_check_cmd(const char *cmd);
+int tst_check_cmd(const char *cmd, const int brk_nosupp);
 
 #endif
diff --git a/include/tst_test.h b/include/tst_test.h
index d0fa84a71..38d24f48c 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -262,6 +262,9 @@ struct tst_ulimit_val {
  *                 passed to mkfs after the device path and can be used to
  *                 limit the file system not to use the whole block device.
  *
+ * @mkfs_ver: mkfs tool version. The string format supports relational
+ *            operators such as < > <= >= ==.
+ *
  * @mnt_flags: MS_* flags passed to mount(2) when the test library mounts a
  *             device in the case of 'tst_test.mount_device'.
  *
@@ -273,6 +276,7 @@ struct tst_fs {
 
 	const char *const *mkfs_opts;
 	const char *mkfs_size_opt;
+	const char *mkfs_ver;
 
 	unsigned int mnt_flags;
 	const void *mnt_data;
diff --git a/lib/tst_cmd.c b/lib/tst_cmd.c
index b3f8a95ab..8cfaa7368 100644
--- a/lib/tst_cmd.c
+++ b/lib/tst_cmd.c
@@ -210,7 +210,7 @@ static int mkfs_ext4_version_parser(void)
 	return major * 10000 +  minor * 100 + patch;
 }
 
-static int mkfs_ext4_version_table_get(char *version)
+static int mkfs_generic_version_table_get(char *version)
 {
 	int major, minor, patch;
 	int len;
@@ -228,19 +228,42 @@ static int mkfs_ext4_version_table_get(char *version)
 	return major * 10000 + minor * 100 + patch;
 }
 
+static int mkfs_xfs_version_parser(void)
+{
+	FILE *f;
+	int rc, major, minor, patch;
+
+	f = popen("mkfs.xfs -V 2>&1", "r");
+	if (!f) {
+		tst_resm(TWARN, "Could not run mkfs.xfs -V 2>&1 cmd");
+		return -1;
+	}
+
+	rc = fscanf(f, "mkfs.xfs version %d.%d.%d", &major, &minor, &patch);
+	pclose(f);
+	if (rc != 3) {
+		tst_resm(TWARN, "Unable to parse mkfs.xfs version");
+		return -1;
+	}
+
+	return major * 10000 +  minor * 100 + patch;
+}
+
 static struct version_parser {
 	const char *cmd;
 	int (*parser)(void);
 	int (*table_get)(char *version);
 } version_parsers[] = {
-	{"mkfs.ext4", mkfs_ext4_version_parser, mkfs_ext4_version_table_get},
+	{"mkfs.ext4", mkfs_ext4_version_parser, mkfs_generic_version_table_get},
+	{"mkfs.xfs", mkfs_xfs_version_parser, mkfs_generic_version_table_get},
 	{},
 };
 
-void tst_check_cmd(const char *cmd)
+int tst_check_cmd(const char *cmd, const int brk_nosupp)
 {
 	struct version_parser *p;
 	char *cmd_token, *op_token, *version_token, *next, *str;
+	char *check_msg = NULL;
 	char path[PATH_MAX];
 	char parser_cmd[100];
 	int ver_parser, ver_get;
@@ -257,7 +280,7 @@ void tst_check_cmd(const char *cmd)
 		tst_brkm(TCONF, NULL, "Couldn't find '%s' in $PATH", cmd_token);
 
 	if (!op_token)
-		return;
+		goto error;
 
 	if (!strcmp(op_token, ">="))
 		op_flag = OP_GE;
@@ -302,46 +325,68 @@ void tst_check_cmd(const char *cmd)
 
 	switch (op_flag) {
 	case OP_GE:
-		if (ver_parser < ver_get) {
-			tst_brkm(TCONF, NULL, "%s required >= %d, but got %d, "
-				"the version is required in order run the test.",
-				cmd, ver_get, ver_parser);
-		}
-		break;
+		if (ver_parser >= ver_get)
+			break;
+
+		check_msg = "%s required >= %d, but got %d, "
+			"the version is required in order run the test.";
+
+		goto error;
 	case OP_GT:
-		if (ver_parser <= ver_get) {
-			tst_brkm(TCONF, NULL, "%s required > %d, but got %d, "
-				"the version is required in order run the test.",
-				cmd, ver_get, ver_parser);
-		}
-		break;
+		if (ver_parser > ver_get)
+			break;
+
+		check_msg = "%s required > %d, but got %d, "
+			"the version is required in order run the "
+			"test.";
+
+		goto error;
 	case OP_LE:
-		if (ver_parser > ver_get) {
-			tst_brkm(TCONF, NULL, "%s required <= %d, but got %d, "
-				"the version is required in order run the test.",
-				cmd, ver_get, ver_parser);
-		}
-		break;
+		if (ver_parser <= ver_get)
+			break;
+
+		check_msg = "%s required <= %d, but got %d, "
+			"the version is required in order run the "
+			"test.";
+
+		goto error;
 	case OP_LT:
-		if (ver_parser >= ver_get) {
-			tst_brkm(TCONF, NULL, "%s required < %d, but got %d, "
-				"the version is required in order run the test.",
-				cmd, ver_get, ver_parser);
-		}
-		break;
+		if (ver_parser < ver_get)
+			break;
+
+		check_msg = "%s required < %d, but got %d, "
+			"the version is required in order run the "
+			"test.";
+
+		goto error;
 	case OP_EQ:
-		if (ver_parser != ver_get) {
-			tst_brkm(TCONF, NULL, "%s required == %d, but got %d, "
-				"the version is required in order run the test.",
-				cmd, ver_get, ver_parser);
-		}
-		break;
+		if (ver_parser == ver_get)
+			break;
+
+		check_msg = "%s required == %d, but got %d, "
+			"the version is required in order run the "
+			"test.";
+
+		goto error;
 	case OP_NE:
-		if (ver_parser == ver_get) {
-			tst_brkm(TCONF, NULL, "%s required != %d, but got %d, "
-				"the version is required in order run the test.",
-				cmd, ver_get, ver_parser);
-		}
-		break;
+		if (ver_parser != ver_get)
+			break;
+
+		check_msg = "%s required != %d, but got %d, "
+			"the version is required in order run the "
+			"test.";
+
+		goto error;
 	}
+
+	return 0;
+error:
+	if (check_msg) {
+		if (brk_nosupp)
+			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get, ver_parser);
+		else
+			tst_resm(TCONF, check_msg, cmd, ver_get, ver_parser);
+	}
+
+	return 1;
 }
diff --git a/lib/tst_test.c b/lib/tst_test.c
index d226157e0..192fee309 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1250,6 +1250,7 @@ static const char *default_fs_type(void)
 static void do_setup(int argc, char *argv[])
 {
 	char *tdebug_env = getenv("LTP_ENABLE_DEBUG");
+	int ret = 0;
 
 	if (!tst_test)
 		tst_brk(TBROK, "No tests to run");
@@ -1310,7 +1311,7 @@ static void do_setup(int argc, char *argv[])
 		int i;
 
 		for (i = 0; (cmd = tst_test->needs_cmds[i]); ++i)
-			tst_check_cmd(cmd);
+			tst_check_cmd(cmd, 1);
 	}
 
 	if (tst_test->needs_drivers) {
@@ -1415,8 +1416,15 @@ static void do_setup(int argc, char *argv[])
 
 		tdev.fs_type = default_fs_type();
 
-		if (!tst_test->all_filesystems && count_fs_descs() <= 1)
+		if (!tst_test->all_filesystems && count_fs_descs() <= 1) {
+			if (tst_test->filesystems->mkfs_ver)
+				ret = tst_check_cmd(tst_test->filesystems->mkfs_ver, 0);
+
+			if (ret)
+				return;
+
 			prepare_device(tst_test->filesystems);
+		}
 	}
 
 	if (tst_test->needs_overlay && !tst_test->mount_device)
@@ -1805,6 +1813,9 @@ static int run_tcase_on_fs(struct tst_fs *fs, const char *fs_type)
 	tst_res(TINFO, "=== Testing on %s ===", fs_type);
 	tdev.fs_type = fs_type;
 
+	if (fs->mkfs_ver && tst_check_cmd(fs->mkfs_ver, 0))
+		return TCONF;
+
 	prepare_device(fs);
 
 	ret = fork_testrun();
@@ -1832,7 +1843,7 @@ static int run_tcases_per_fs(void)
 		if (!fs)
 			continue;
 
-		run_tcase_on_fs(fs, filesystems[i]);
+		ret = run_tcase_on_fs(fs, filesystems[i]);
 
 		if (ret == TCONF)
 			continue;
diff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
index 8ed0f21b6..ee029b666 100644
--- a/testcases/lib/tst_run_shell.c
+++ b/testcases/lib/tst_run_shell.c
@@ -153,6 +153,7 @@ static const char *const *parse_strarr(ujson_reader *reader, ujson_val *val)
 enum fs_ids {
 	MKFS_OPTS,
 	MKFS_SIZE_OPT,
+	MKFS_VER,
 	MNT_FLAGS,
 	TYPE,
 };
@@ -160,6 +161,7 @@ enum fs_ids {
 static ujson_obj_attr fs_attrs[] = {
 	UJSON_OBJ_ATTR_IDX(MKFS_OPTS, "mkfs_opts", UJSON_ARR),
 	UJSON_OBJ_ATTR_IDX(MKFS_SIZE_OPT, "mkfs_size_opt", UJSON_STR),
+	UJSON_OBJ_ATTR_IDX(MKFS_VER, "mkfs_ver", UJSON_STR),
 	UJSON_OBJ_ATTR_IDX(MNT_FLAGS, "mnt_flags", UJSON_ARR),
 	UJSON_OBJ_ATTR_IDX(TYPE, "type", UJSON_STR),
 };
@@ -224,6 +226,9 @@ static struct tst_fs *parse_filesystems(ujson_reader *reader, ujson_val *val)
 			case MKFS_SIZE_OPT:
 				ret[i].mkfs_size_opt = strdup(val->val_str);
 			break;
+			case MKFS_VER:
+				ret[i].mkfs_ver = strdup(val->val_str);
+			break;
 			case MNT_FLAGS:
 				ret[i].mnt_flags = parse_mnt_flags(reader, val);
 			break;

-- 
2.43.0


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

[LTP] [PATCH v3 2/3] Add minimum kernel requirement for FS setup

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

In some cases, a filesystem that is going to be created and mounted
by LTP can't be supported by certain kernel versions. This is the case
of the CoW support: mkfs creates a CoW filesystem, while underlying
kernel can't mount it.

To cover this scenario, a new flag called .min_kver has been
introduced in the tst_fs structure, giving the user a possibility to
filter out certain kernels not supporting certain FS features.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 include/tst_test.h            |  5 +++++
 lib/tst_test.c                | 27 +++++++++++++++++++++------
 testcases/lib/tst_run_shell.c |  5 +++++
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/include/tst_test.h b/include/tst_test.h
index 38d24f48c..8d1819f74 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -270,6 +270,9 @@ struct tst_ulimit_val {
  *
  * @mnt_data: The data passed to mount(2) when the test library mounts a device
  *            in the case of 'tst_test.mount_device'.
+ *
+ * @min_kver: A minimum kernel version supporting the filesystem which has been
+ *            created with mkfs.
  */
 struct tst_fs {
 	const char *type;
@@ -280,6 +283,8 @@ struct tst_fs {
 
 	unsigned int mnt_flags;
 	const void *mnt_data;
+
+	const char *min_kver;
 };
 
 /**
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 192fee309..fe07c4d98 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -950,20 +950,29 @@ static void do_exit(int ret)
 	exit(ret);
 }
 
-void check_kver(void)
+int check_kver(const char *min_kver, const int brk_nosupp)
 {
+	char *msg;
 	int v1, v2, v3;
 
-	if (tst_parse_kver(tst_test->min_kver, &v1, &v2, &v3)) {
+	if (tst_parse_kver(min_kver, &v1, &v2, &v3)) {
 		tst_res(TWARN,
 			"Invalid kernel version %s, expected %%d.%%d.%%d",
-			tst_test->min_kver);
+			min_kver);
 	}
 
 	if (tst_kvercmp(v1, v2, v3) < 0) {
-		tst_brk(TCONF, "The test requires kernel %s or newer",
-			tst_test->min_kver);
+		msg = "The test requires kernel %s or newer";
+
+		if (brk_nosupp)
+			tst_brk(TCONF, msg, min_kver);
+		else
+			tst_res(TCONF, msg, min_kver);
+
+		return 1;
 	}
+
+	return 0;
 }
 
 static int results_equal(struct results *a, struct results *b)
@@ -1289,7 +1298,7 @@ static void do_setup(int argc, char *argv[])
 		tst_brk(TCONF, "Test needs to be run as root");
 
 	if (tst_test->min_kver)
-		check_kver();
+		check_kver(tst_test->min_kver, 1);
 
 	if (tst_test->supported_archs && !tst_is_on_arch(tst_test->supported_archs))
 		tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name);
@@ -1420,6 +1429,9 @@ static void do_setup(int argc, char *argv[])
 			if (tst_test->filesystems->mkfs_ver)
 				ret = tst_check_cmd(tst_test->filesystems->mkfs_ver, 0);
 
+			if (tst_test->filesystems->min_kver)
+				ret = check_kver(tst_test->filesystems->min_kver, 0);
+
 			if (ret)
 				return;
 
@@ -1816,6 +1828,9 @@ static int run_tcase_on_fs(struct tst_fs *fs, const char *fs_type)
 	if (fs->mkfs_ver && tst_check_cmd(fs->mkfs_ver, 0))
 		return TCONF;
 
+	if (fs->min_kver && check_kver(fs->min_kver, 0))
+		return TCONF;
+
 	prepare_device(fs);
 
 	ret = fork_testrun();
diff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
index ee029b666..6b714c51c 100644
--- a/testcases/lib/tst_run_shell.c
+++ b/testcases/lib/tst_run_shell.c
@@ -156,6 +156,7 @@ enum fs_ids {
 	MKFS_VER,
 	MNT_FLAGS,
 	TYPE,
+	FS_MIN_KVER,
 };
 
 static ujson_obj_attr fs_attrs[] = {
@@ -164,6 +165,7 @@ static ujson_obj_attr fs_attrs[] = {
 	UJSON_OBJ_ATTR_IDX(MKFS_VER, "mkfs_ver", UJSON_STR),
 	UJSON_OBJ_ATTR_IDX(MNT_FLAGS, "mnt_flags", UJSON_ARR),
 	UJSON_OBJ_ATTR_IDX(TYPE, "type", UJSON_STR),
+	UJSON_OBJ_ATTR_IDX(FS_MIN_KVER, "min_kver", UJSON_STR),
 };
 
 static ujson_obj fs_obj = {
@@ -235,6 +237,9 @@ static struct tst_fs *parse_filesystems(ujson_reader *reader, ujson_val *val)
 			case TYPE:
 				ret[i].type = strdup(val->val_str);
 			break;
+			case FS_MIN_KVER:
+				ret[i].min_kver = strdup(val->val_str);
+			break;
 			}
 
 		}

-- 
2.43.0


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

[LTP] [PATCH v3 3/3] Setup minimal kernel for ioctl_clone(range) tests

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

Some linux kernels don't support copy-on-write feature for the
filesystems we are going to use during ioctl_ficlone and
ioctl_ficlonerange tests. For this reason, fix the minimal kernel
version required in order to skip test when it's needed.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c      | 4 +++-
 testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c      | 4 +++-
 testcases/kernel/syscalls/ioctl/ioctl_ficlonerange01.c | 4 +++-
 testcases/kernel/syscalls/ioctl/ioctl_ficlonerange02.c | 4 +++-
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c
index f5407f789..2574bb6a2 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c
@@ -112,10 +112,12 @@ static struct tst_test test = {
 	.mount_device = 1,
 	.mntpoint = MNTPOINT,
 	.filesystems = (struct tst_fs []) {
-		{.type = "bcachefs"},
 		{.type = "btrfs"},
+		{.type = "bcachefs"},
 		{
 			.type = "xfs",
+			.min_kver = "4.16",
+			.mkfs_ver = "mkfs.xfs >= 1.5.0",
 			.mkfs_opts = (const char *const []) {"-m", "reflink=1", NULL},
 		},
 		{}
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c
index 3cc386c59..f12c4e7fd 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c
@@ -110,10 +110,12 @@ static struct tst_test test = {
 	.mount_device = 1,
 	.mntpoint = MNTPOINT,
 	.filesystems = (struct tst_fs []) {
-		{.type = "bcachefs"},
 		{.type = "btrfs"},
+		{.type = "bcachefs"},
 		{
 			.type = "xfs",
+			.min_kver = "4.16",
+			.mkfs_ver = "mkfs.xfs >= 1.5.0",
 			.mkfs_opts = (const char *const []) {"-m", "reflink=1", NULL},
 		},
 		{}
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlonerange01.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlonerange01.c
index e352c513b..8fd1d299a 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_ficlonerange01.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlonerange01.c
@@ -145,10 +145,12 @@ static struct tst_test test = {
 	.mount_device = 1,
 	.mntpoint = MNTPOINT,
 	.filesystems = (struct tst_fs []) {
-		{.type = "bcachefs"},
 		{.type = "btrfs"},
+		{.type = "bcachefs"},
 		{
 			.type = "xfs",
+			.min_kver = "4.16",
+			.mkfs_ver = "mkfs.xfs >= 1.5.0",
 			.mkfs_opts = (const char *const []) {"-m", "reflink=1", NULL},
 		},
 		{}
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlonerange02.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlonerange02.c
index ad36df162..eb941c3fc 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_ficlonerange02.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlonerange02.c
@@ -82,10 +82,12 @@ static struct tst_test test = {
 	.mount_device = 1,
 	.mntpoint = MNTPOINT,
 	.filesystems = (struct tst_fs []) {
-		{.type = "bcachefs"},
 		{.type = "btrfs"},
+		{.type = "bcachefs"},
 		{
 			.type = "xfs",
+			.min_kver = "4.16",
+			.mkfs_ver = "mkfs.xfs >= 1.5.0",
 			.mkfs_opts = (const char *const []) {"-m", "reflink=1", NULL},
 		},
 		{}

-- 
2.43.0


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

Re: [LTP] [PATCH v3 1/3] Filter mkfs version in tst_fs

[Cyril Hrubis]

Hi!
> +		if (ver_parser >= ver_get)
> +			break;
> +
> +		check_msg = "%s required >= %d, but got %d, "
> +			"the version is required in order run the test.";

I would drop the "the version is required in order to run the test."
part from these messages, since it does not add any more value on the
top of the first part of the meassage.

> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index d226157e0..192fee309 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1250,6 +1250,7 @@ static const char *default_fs_type(void)
>  static void do_setup(int argc, char *argv[])
>  {
>  	char *tdebug_env = getenv("LTP_ENABLE_DEBUG");
> +	int ret = 0;
>  
>  	if (!tst_test)
>  		tst_brk(TBROK, "No tests to run");
> @@ -1310,7 +1311,7 @@ static void do_setup(int argc, char *argv[])
>  		int i;
>  
>  		for (i = 0; (cmd = tst_test->needs_cmds[i]); ++i)
> -			tst_check_cmd(cmd);
> +			tst_check_cmd(cmd, 1);
>  	}
>  
>  	if (tst_test->needs_drivers) {
> @@ -1415,8 +1416,15 @@ static void do_setup(int argc, char *argv[])
>  
>  		tdev.fs_type = default_fs_type();
>  
> -		if (!tst_test->all_filesystems && count_fs_descs() <= 1)
> +		if (!tst_test->all_filesystems && count_fs_descs() <= 1) {
> +			if (tst_test->filesystems->mkfs_ver)
> +				ret = tst_check_cmd(tst_test->filesystems->mkfs_ver, 0);
> +
> +			if (ret)
> +				return;

And if we are here, it means that the test runs only for a single
filesystem, so we should instead do:

			if (tst_test->filesystem->mkfs_ver)
				tst_check_mcd(tst_test->filesystems->mkfs_ver, 1);

The rest looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v3 2/3] Add minimum kernel requirement for FS setup

[Cyril Hrubis]

Hi!
> In some cases, a filesystem that is going to be created and mounted
> by LTP can't be supported by certain kernel versions. This is the case
> of the CoW support: mkfs creates a CoW filesystem, while underlying
> kernel can't mount it.
> 
> To cover this scenario, a new flag called .min_kver has been
> introduced in the tst_fs structure, giving the user a possibility to
> filter out certain kernels not supporting certain FS features.
> 
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  include/tst_test.h            |  5 +++++
>  lib/tst_test.c                | 27 +++++++++++++++++++++------
>  testcases/lib/tst_run_shell.c |  5 +++++
>  3 files changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 38d24f48c..8d1819f74 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -270,6 +270,9 @@ struct tst_ulimit_val {
>   *
>   * @mnt_data: The data passed to mount(2) when the test library mounts a device
>   *            in the case of 'tst_test.mount_device'.
> + *
> + * @min_kver: A minimum kernel version supporting the filesystem which has been
> + *            created with mkfs.
>   */
>  struct tst_fs {
>  	const char *type;
> @@ -280,6 +283,8 @@ struct tst_fs {
>  
>  	unsigned int mnt_flags;
>  	const void *mnt_data;
> +
> +	const char *min_kver;
>  };
>  
>  /**
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 192fee309..fe07c4d98 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -950,20 +950,29 @@ static void do_exit(int ret)
>  	exit(ret);
>  }
>  
> -void check_kver(void)
> +int check_kver(const char *min_kver, const int brk_nosupp)
>  {
> +	char *msg;
>  	int v1, v2, v3;
>  
> -	if (tst_parse_kver(tst_test->min_kver, &v1, &v2, &v3)) {
> +	if (tst_parse_kver(min_kver, &v1, &v2, &v3)) {
>  		tst_res(TWARN,
>  			"Invalid kernel version %s, expected %%d.%%d.%%d",
> -			tst_test->min_kver);
> +			min_kver);
>  	}
>  
>  	if (tst_kvercmp(v1, v2, v3) < 0) {
> -		tst_brk(TCONF, "The test requires kernel %s or newer",
> -			tst_test->min_kver);
> +		msg = "The test requires kernel %s or newer";
> +
> +		if (brk_nosupp)
> +			tst_brk(TCONF, msg, min_kver);
> +		else
> +			tst_res(TCONF, msg, min_kver);
> +
> +		return 1;
>  	}
> +
> +	return 0;
>  }
>  
>  static int results_equal(struct results *a, struct results *b)
> @@ -1289,7 +1298,7 @@ static void do_setup(int argc, char *argv[])
>  		tst_brk(TCONF, "Test needs to be run as root");
>  
>  	if (tst_test->min_kver)
> -		check_kver();
> +		check_kver(tst_test->min_kver, 1);
>  
>  	if (tst_test->supported_archs && !tst_is_on_arch(tst_test->supported_archs))
>  		tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name);
> @@ -1420,6 +1429,9 @@ static void do_setup(int argc, char *argv[])
>  			if (tst_test->filesystems->mkfs_ver)
>  				ret = tst_check_cmd(tst_test->filesystems->mkfs_ver, 0);
>  
> +			if (tst_test->filesystems->min_kver)
> +				ret = check_kver(tst_test->filesystems->min_kver, 0);

Here as well, we should pass 1 as the last argument.

>  			if (ret)
>  				return;
>  
> @@ -1816,6 +1828,9 @@ static int run_tcase_on_fs(struct tst_fs *fs, const char *fs_type)
>  	if (fs->mkfs_ver && tst_check_cmd(fs->mkfs_ver, 0))
>  		return TCONF;
>  
> +	if (fs->min_kver && check_kver(fs->min_kver, 0))
> +		return TCONF;
> +
>  	prepare_device(fs);
>  
>  	ret = fork_testrun();
> diff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
> index ee029b666..6b714c51c 100644
> --- a/testcases/lib/tst_run_shell.c
> +++ b/testcases/lib/tst_run_shell.c
> @@ -156,6 +156,7 @@ enum fs_ids {
>  	MKFS_VER,
>  	MNT_FLAGS,
>  	TYPE,
> +	FS_MIN_KVER,
>  };
>  
>  static ujson_obj_attr fs_attrs[] = {
> @@ -164,6 +165,7 @@ static ujson_obj_attr fs_attrs[] = {
>  	UJSON_OBJ_ATTR_IDX(MKFS_VER, "mkfs_ver", UJSON_STR),
>  	UJSON_OBJ_ATTR_IDX(MNT_FLAGS, "mnt_flags", UJSON_ARR),
>  	UJSON_OBJ_ATTR_IDX(TYPE, "type", UJSON_STR),
> +	UJSON_OBJ_ATTR_IDX(FS_MIN_KVER, "min_kver", UJSON_STR),

This is stil not sorted properly, the min_kver should go before the
mkfs_ver.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v3 2/3] Add minimum kernel requirement for FS setup

[Andrea Cervesato via ltp]

Hi,

On 10/4/24 16:08, Cyril Hrubis wrote:
> Hi!
>> In some cases, a filesystem that is going to be created and mounted
>> by LTP can't be supported by certain kernel versions. This is the case
>> of the CoW support: mkfs creates a CoW filesystem, while underlying
>> kernel can't mount it.
>>
>> To cover this scenario, a new flag called .min_kver has been
>> introduced in the tst_fs structure, giving the user a possibility to
>> filter out certain kernels not supporting certain FS features.
>>
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>>   include/tst_test.h            |  5 +++++
>>   lib/tst_test.c                | 27 +++++++++++++++++++++------
>>   testcases/lib/tst_run_shell.c |  5 +++++
>>   3 files changed, 31 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/tst_test.h b/include/tst_test.h
>> index 38d24f48c..8d1819f74 100644
>> --- a/include/tst_test.h
>> +++ b/include/tst_test.h
>> @@ -270,6 +270,9 @@ struct tst_ulimit_val {
>>    *
>>    * @mnt_data: The data passed to mount(2) when the test library mounts a device
>>    *            in the case of 'tst_test.mount_device'.
>> + *
>> + * @min_kver: A minimum kernel version supporting the filesystem which has been
>> + *            created with mkfs.
>>    */
>>   struct tst_fs {
>>   	const char *type;
>> @@ -280,6 +283,8 @@ struct tst_fs {
>>   
>>   	unsigned int mnt_flags;
>>   	const void *mnt_data;
>> +
>> +	const char *min_kver;
>>   };
>>   
>>   /**
>> diff --git a/lib/tst_test.c b/lib/tst_test.c
>> index 192fee309..fe07c4d98 100644
>> --- a/lib/tst_test.c
>> +++ b/lib/tst_test.c
>> @@ -950,20 +950,29 @@ static void do_exit(int ret)
>>   	exit(ret);
>>   }
>>   
>> -void check_kver(void)
>> +int check_kver(const char *min_kver, const int brk_nosupp)
>>   {
>> +	char *msg;
>>   	int v1, v2, v3;
>>   
>> -	if (tst_parse_kver(tst_test->min_kver, &v1, &v2, &v3)) {
>> +	if (tst_parse_kver(min_kver, &v1, &v2, &v3)) {
>>   		tst_res(TWARN,
>>   			"Invalid kernel version %s, expected %%d.%%d.%%d",
>> -			tst_test->min_kver);
>> +			min_kver);
>>   	}
>>   
>>   	if (tst_kvercmp(v1, v2, v3) < 0) {
>> -		tst_brk(TCONF, "The test requires kernel %s or newer",
>> -			tst_test->min_kver);
>> +		msg = "The test requires kernel %s or newer";
>> +
>> +		if (brk_nosupp)
>> +			tst_brk(TCONF, msg, min_kver);
>> +		else
>> +			tst_res(TCONF, msg, min_kver);
>> +
>> +		return 1;
>>   	}
>> +
>> +	return 0;
>>   }
>>   
>>   static int results_equal(struct results *a, struct results *b)
>> @@ -1289,7 +1298,7 @@ static void do_setup(int argc, char *argv[])
>>   		tst_brk(TCONF, "Test needs to be run as root");
>>   
>>   	if (tst_test->min_kver)
>> -		check_kver();
>> +		check_kver(tst_test->min_kver, 1);
>>   
>>   	if (tst_test->supported_archs && !tst_is_on_arch(tst_test->supported_archs))
>>   		tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name);
>> @@ -1420,6 +1429,9 @@ static void do_setup(int argc, char *argv[])
>>   			if (tst_test->filesystems->mkfs_ver)
>>   				ret = tst_check_cmd(tst_test->filesystems->mkfs_ver, 0);
>>   
>> +			if (tst_test->filesystems->min_kver)
>> +				ret = check_kver(tst_test->filesystems->min_kver, 0);
> Here as well, we should pass 1 as the last argument.
>
>>   			if (ret)
>>   				return;
>>   
>> @@ -1816,6 +1828,9 @@ static int run_tcase_on_fs(struct tst_fs *fs, const char *fs_type)
>>   	if (fs->mkfs_ver && tst_check_cmd(fs->mkfs_ver, 0))
>>   		return TCONF;
>>   
>> +	if (fs->min_kver && check_kver(fs->min_kver, 0))
>> +		return TCONF;
>> +
>>   	prepare_device(fs);
>>   
>>   	ret = fork_testrun();
>> diff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
>> index ee029b666..6b714c51c 100644
>> --- a/testcases/lib/tst_run_shell.c
>> +++ b/testcases/lib/tst_run_shell.c
>> @@ -156,6 +156,7 @@ enum fs_ids {
>>   	MKFS_VER,
>>   	MNT_FLAGS,
>>   	TYPE,
>> +	FS_MIN_KVER,
>>   };
>>   
>>   static ujson_obj_attr fs_attrs[] = {
>> @@ -164,6 +165,7 @@ static ujson_obj_attr fs_attrs[] = {
>>   	UJSON_OBJ_ATTR_IDX(MKFS_VER, "mkfs_ver", UJSON_STR),
>>   	UJSON_OBJ_ATTR_IDX(MNT_FLAGS, "mnt_flags", UJSON_ARR),
>>   	UJSON_OBJ_ATTR_IDX(TYPE, "type", UJSON_STR),
>> +	UJSON_OBJ_ATTR_IDX(FS_MIN_KVER, "min_kver", UJSON_STR),
> This is stil not sorted properly, the min_kver should go before the
> mkfs_ver.
>
This is not clear to me. The objects are sorted mirroring the tst_fs 
struct. Can you explain it better please?

Andrea


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

Re: [LTP] [PATCH v3 2/3] Add minimum kernel requirement for FS setup

[Cyril Hrubis]

Hi!
> >> diff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
> >> index ee029b666..6b714c51c 100644
> >> --- a/testcases/lib/tst_run_shell.c
> >> +++ b/testcases/lib/tst_run_shell.c
> >> @@ -156,6 +156,7 @@ enum fs_ids {
> >>   	MKFS_VER,
> >>   	MNT_FLAGS,
> >>   	TYPE,
> >> +	FS_MIN_KVER,
> >>   };
> >>   
> >>   static ujson_obj_attr fs_attrs[] = {
> >> @@ -164,6 +165,7 @@ static ujson_obj_attr fs_attrs[] = {
> >>   	UJSON_OBJ_ATTR_IDX(MKFS_VER, "mkfs_ver", UJSON_STR),
> >>   	UJSON_OBJ_ATTR_IDX(MNT_FLAGS, "mnt_flags", UJSON_ARR),
> >>   	UJSON_OBJ_ATTR_IDX(TYPE, "type", UJSON_STR),
> >> +	UJSON_OBJ_ATTR_IDX(FS_MIN_KVER, "min_kver", UJSON_STR),
> > This is stil not sorted properly, the min_kver should go before the
> > mkfs_ver.
> >
> This is not clear to me. The objects are sorted mirroring the tst_fs 
> struct. Can you explain it better please?

This is a lookup table that has to be sorted by the string ID we are
looking up. Well technically the fs_ids enum has to be sorted so that
the string ids are sorted and the order of this array does not matter,
but it's easier to keep them both sorted alphabetically by the ID.

-- 
Cyril Hrubis
chrubis@suse.cz

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