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

[LTP] [PATCH v2 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 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                                      | 130 ++++++++++++++++-----
 lib/tst_test.c                                     |  25 ++--
 testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c  |   7 +-
 testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c  |   7 +-
 .../kernel/syscalls/ioctl/ioctl_ficlonerange01.c   |   7 +-
 .../kernel/syscalls/ioctl/ioctl_ficlonerange02.c   |   7 +-
 testcases/lib/tst_run_shell.c                      |  10 ++
 9 files changed, 167 insertions(+), 41 deletions(-)
---
base-commit: 47aff4decc81ac837fd745278def6883fc2f197b
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 v2 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                 | 130 +++++++++++++++++++++++++++++++++---------
 lib/tst_test.c                |   5 +-
 testcases/lib/tst_run_shell.c |   5 ++
 5 files changed, 119 insertions(+), 31 deletions(-)

diff --git a/include/tst_private.h b/include/tst_private.h
index 6f4f39b15..a29f2d1c1 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 unless skip_on_error 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 unless skip_on_error is defined.
  */
-void tst_check_cmd(const char *cmd);
+void tst_check_cmd(const char *cmd, const int skip_on_error);
 
 #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..35dd71253 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)
+void tst_check_cmd(const char *cmd, const int skip_on_error)
 {
 	struct version_parser *p;
 	char *cmd_token, *op_token, *version_token, *next, *str;
+	char *check_msg;
 	char path[PATH_MAX];
 	char parser_cmd[100];
 	int ver_parser, ver_get;
@@ -302,45 +325,98 @@ 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);
+		if (ver_parser >= ver_get)
+			break;
+
+		check_msg = "%s required >= %d, but got %d, "
+			"the version is required in order run the test.";
+
+		if (skip_on_error) {
+			tst_resm(TCONF, check_msg, cmd, ver_get,
+				ver_parser);
+		} else {
+			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
+				ver_parser);
 		}
 		break;
 	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);
+		if (ver_parser > ver_get)
+			break;
+
+		check_msg = "%s required > %d, but got %d, "
+			"the version is required in order run the "
+			"test.";
+
+		if (skip_on_error) {
+			tst_resm(TCONF, check_msg, cmd, ver_get,
+				ver_parser);
+		} else {
+			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
+				ver_parser);
 		}
 		break;
 	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);
+		if (ver_parser <= ver_get)
+			break;
+
+		check_msg = "%s required <= %d, but got %d, "
+			"the version is required in order run the "
+			"test.";
+
+		if (skip_on_error) {
+			tst_resm(TCONF, check_msg, cmd, ver_get,
+				ver_parser);
+		} else {
+			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
+				ver_parser);
 		}
 		break;
 	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);
+		if (ver_parser < ver_get)
+			break;
+
+		check_msg = "%s required < %d, but got %d, "
+			"the version is required in order run the "
+			"test.";
+
+		if (skip_on_error) {
+			tst_resm(TCONF, check_msg, cmd, ver_get,
+				ver_parser);
+		} else {
+			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
+				ver_parser);
 		}
 		break;
 	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);
+		if (ver_parser == ver_get)
+			break;
+
+		check_msg = "%s required == %d, but got %d, "
+			"the version is required in order run the "
+			"test.";
+
+		if (skip_on_error) {
+			tst_resm(TCONF, check_msg, cmd, ver_get,
+				ver_parser);
+		} else {
+			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
+				ver_parser);
 		}
 		break;
 	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);
+		if (ver_parser != ver_get)
+			break;
+
+		check_msg = "%s required != %d, but got %d, "
+			"the version is required in order run the "
+			"test.";
+
+		if (skip_on_error) {
+			tst_resm(TCONF, check_msg, cmd, ver_get,
+				ver_parser);
+		} else {
+			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
+				ver_parser);
 		}
 		break;
 	}
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 918bee2a1..7dfab4677 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1154,6 +1154,9 @@ static void prepare_device(struct tst_fs *fs)
 
 	const char *const extra[] = {fs->mkfs_size_opt, NULL};
 
+	if (fs->mkfs_ver)
+		tst_check_cmd(fs->mkfs_ver, 1);
+
 	if (tst_test->format_device)
 		SAFE_MKFS(tdev.dev, tdev.fs_type, fs->mkfs_opts, extra);
 
@@ -1306,7 +1309,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, 0);
 	}
 
 	if (tst_test->needs_drivers) {
diff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
index 8ed0f21b6..fbfbe16a7 100644
--- a/testcases/lib/tst_run_shell.c
+++ b/testcases/lib/tst_run_shell.c
@@ -153,12 +153,14 @@ 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,
 };
 
 static ujson_obj_attr fs_attrs[] = {
 	UJSON_OBJ_ATTR_IDX(MKFS_OPTS, "mkfs_opts", UJSON_ARR),
+	UJSON_OBJ_ATTR_IDX(MKFS_VER, "mkfs_ver", UJSON_STR),
 	UJSON_OBJ_ATTR_IDX(MKFS_SIZE_OPT, "mkfs_size_opt", 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

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

[Cyril Hrubis]

Hi!
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  include/tst_private.h         |   6 +-
>  include/tst_test.h            |   4 ++
>  lib/tst_cmd.c                 | 130 +++++++++++++++++++++++++++++++++---------
>  lib/tst_test.c                |   5 +-
>  testcases/lib/tst_run_shell.c |   5 ++
>  5 files changed, 119 insertions(+), 31 deletions(-)
> 
> diff --git a/include/tst_private.h b/include/tst_private.h
> index 6f4f39b15..a29f2d1c1 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 unless skip_on_error 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 unless skip_on_error is defined.
>   */
> -void tst_check_cmd(const char *cmd);
> +void tst_check_cmd(const char *cmd, const int skip_on_error);
>  
>  #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..35dd71253 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)
> +void tst_check_cmd(const char *cmd, const int skip_on_error)

Technically it's not error, so maybe the flag should be called
brk_on_unsuitable or something along these lines.

>  {
>  	struct version_parser *p;
>  	char *cmd_token, *op_token, *version_token, *next, *str;
> +	char *check_msg;
>  	char path[PATH_MAX];
>  	char parser_cmd[100];
>  	int ver_parser, ver_get;
> @@ -302,45 +325,98 @@ 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);
> +		if (ver_parser >= ver_get)
> +			break;
> +
> +		check_msg = "%s required >= %d, but got %d, "
> +			"the version is required in order run the test.";
> +
> +		if (skip_on_error) {
> +			tst_resm(TCONF, check_msg, cmd, ver_get,
> +				ver_parser);
> +		} else {
> +			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
> +				ver_parser);
>  		}
>  		break;
>  	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);
> +		if (ver_parser > ver_get)
> +			break;
> +
> +		check_msg = "%s required > %d, but got %d, "
> +			"the version is required in order run the "
> +			"test.";
> +
> +		if (skip_on_error) {
> +			tst_resm(TCONF, check_msg, cmd, ver_get,
> +				ver_parser);
> +		} else {
> +			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
> +				ver_parser);
>  		}
>  		break;
>  	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);
> +		if (ver_parser <= ver_get)
> +			break;
> +
> +		check_msg = "%s required <= %d, but got %d, "
> +			"the version is required in order run the "
> +			"test.";
> +
> +		if (skip_on_error) {
> +			tst_resm(TCONF, check_msg, cmd, ver_get,
> +				ver_parser);
> +		} else {
> +			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
> +				ver_parser);
>  		}
>  		break;
>  	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);
> +		if (ver_parser < ver_get)
> +			break;
> +
> +		check_msg = "%s required < %d, but got %d, "
> +			"the version is required in order run the "
> +			"test.";
> +
> +		if (skip_on_error) {
> +			tst_resm(TCONF, check_msg, cmd, ver_get,
> +				ver_parser);
> +		} else {
> +			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
> +				ver_parser);
>  		}
>  		break;
>  	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);
> +		if (ver_parser == ver_get)
> +			break;
> +
> +		check_msg = "%s required == %d, but got %d, "
> +			"the version is required in order run the "
> +			"test.";
> +
> +		if (skip_on_error) {
> +			tst_resm(TCONF, check_msg, cmd, ver_get,
> +				ver_parser);
> +		} else {
> +			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
> +				ver_parser);
>  		}
>  		break;
>  	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);
> +		if (ver_parser != ver_get)
> +			break;
> +
> +		check_msg = "%s required != %d, but got %d, "
> +			"the version is required in order run the "
> +			"test.";
> +
> +		if (skip_on_error) {
> +			tst_resm(TCONF, check_msg, cmd, ver_get,
> +				ver_parser);
> +		} else {
> +			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
> +				ver_parser);
>  		}
>  		break;
>  	}
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 918bee2a1..7dfab4677 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1154,6 +1154,9 @@ static void prepare_device(struct tst_fs *fs)
>  
>  	const char *const extra[] = {fs->mkfs_size_opt, NULL};
>  
> +	if (fs->mkfs_ver)
> +		tst_check_cmd(fs->mkfs_ver, 1);

So this prints tst_resm(TCONF, "...") but then proceeds with the mkfs?

I suppose that both tst_check_cmd() and prepare_device() has to be
changed to return a success/failure so that we can propagate the result
fo the check and then we have to do:

	if (prepare_device(fs))
		return;

in the run_tcase_on_fs()...

Also there is a prepare_device(fs) in the do_setup() and I suppose that
we have to actually do tst_brkm() if called from there, so we may need
to add the brk_on_unsuitable flag to the prepare_device() as well.

I'm wondering if there is an easier way around these things. Maybe
putting the check to prepare_device is not the best solution. Maybe we
should put it into the do_setup() and the all filesystems loop
separatelly as:

diff --git a/lib/tst_test.c b/lib/tst_test.c
index d226157e0..55b01ea9c 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1415,8 +1415,12 @@ 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)
+                               tst_check_cmd(tst_test->filesystems->mkfs_ver, 0);
+
                        prepare_device(tst_test->filesystems);
+               }
        }

        if (tst_test->needs_overlay && !tst_test->mount_device)
@@ -1805,6 +1809,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, 1))
+               return TCONF;
+
        prepare_device(fs);

        ret = fork_testrun();


>  	if (tst_test->format_device)
>  		SAFE_MKFS(tdev.dev, tdev.fs_type, fs->mkfs_opts, extra);
>  
> @@ -1306,7 +1309,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, 0);
>  	}
>  
>  	if (tst_test->needs_drivers) {
> diff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
> index 8ed0f21b6..fbfbe16a7 100644
> --- a/testcases/lib/tst_run_shell.c
> +++ b/testcases/lib/tst_run_shell.c
> @@ -153,12 +153,14 @@ 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,
>  };
>  
>  static ujson_obj_attr fs_attrs[] = {
>  	UJSON_OBJ_ATTR_IDX(MKFS_OPTS, "mkfs_opts", UJSON_ARR),
> +	UJSON_OBJ_ATTR_IDX(MKFS_VER, "mkfs_ver", UJSON_STR),
>  	UJSON_OBJ_ATTR_IDX(MKFS_SIZE_OPT, "mkfs_size_opt", UJSON_STR),

These have to be sorted, so MKFS_VER has to go after MKFS_SIZE_OPT. As
it is the parser would exit with a failure when passed these attributes.

>  	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

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

[Andrea Cervesato via ltp]

Hi!

On 10/1/24 17:12, Cyril Hrubis wrote:
> Hi!
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>>   include/tst_private.h         |   6 +-
>>   include/tst_test.h            |   4 ++
>>   lib/tst_cmd.c                 | 130 +++++++++++++++++++++++++++++++++---------
>>   lib/tst_test.c                |   5 +-
>>   testcases/lib/tst_run_shell.c |   5 ++
>>   5 files changed, 119 insertions(+), 31 deletions(-)
>>
>> diff --git a/include/tst_private.h b/include/tst_private.h
>> index 6f4f39b15..a29f2d1c1 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 unless skip_on_error 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 unless skip_on_error is defined.
>>    */
>> -void tst_check_cmd(const char *cmd);
>> +void tst_check_cmd(const char *cmd, const int skip_on_error);
>>   
>>   #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..35dd71253 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)
>> +void tst_check_cmd(const char *cmd, const int skip_on_error)
> Technically it's not error, so maybe the flag should be called
> brk_on_unsuitable or something along these lines.
>
>>   {
>>   	struct version_parser *p;
>>   	char *cmd_token, *op_token, *version_token, *next, *str;
>> +	char *check_msg;
>>   	char path[PATH_MAX];
>>   	char parser_cmd[100];
>>   	int ver_parser, ver_get;
>> @@ -302,45 +325,98 @@ 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);
>> +		if (ver_parser >= ver_get)
>> +			break;
>> +
>> +		check_msg = "%s required >= %d, but got %d, "
>> +			"the version is required in order run the test.";
>> +
>> +		if (skip_on_error) {
>> +			tst_resm(TCONF, check_msg, cmd, ver_get,
>> +				ver_parser);
>> +		} else {
>> +			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
>> +				ver_parser);
>>   		}
>>   		break;
>>   	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);
>> +		if (ver_parser > ver_get)
>> +			break;
>> +
>> +		check_msg = "%s required > %d, but got %d, "
>> +			"the version is required in order run the "
>> +			"test.";
>> +
>> +		if (skip_on_error) {
>> +			tst_resm(TCONF, check_msg, cmd, ver_get,
>> +				ver_parser);
>> +		} else {
>> +			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
>> +				ver_parser);
>>   		}
>>   		break;
>>   	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);
>> +		if (ver_parser <= ver_get)
>> +			break;
>> +
>> +		check_msg = "%s required <= %d, but got %d, "
>> +			"the version is required in order run the "
>> +			"test.";
>> +
>> +		if (skip_on_error) {
>> +			tst_resm(TCONF, check_msg, cmd, ver_get,
>> +				ver_parser);
>> +		} else {
>> +			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
>> +				ver_parser);
>>   		}
>>   		break;
>>   	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);
>> +		if (ver_parser < ver_get)
>> +			break;
>> +
>> +		check_msg = "%s required < %d, but got %d, "
>> +			"the version is required in order run the "
>> +			"test.";
>> +
>> +		if (skip_on_error) {
>> +			tst_resm(TCONF, check_msg, cmd, ver_get,
>> +				ver_parser);
>> +		} else {
>> +			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
>> +				ver_parser);
>>   		}
>>   		break;
>>   	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);
>> +		if (ver_parser == ver_get)
>> +			break;
>> +
>> +		check_msg = "%s required == %d, but got %d, "
>> +			"the version is required in order run the "
>> +			"test.";
>> +
>> +		if (skip_on_error) {
>> +			tst_resm(TCONF, check_msg, cmd, ver_get,
>> +				ver_parser);
>> +		} else {
>> +			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
>> +				ver_parser);
>>   		}
>>   		break;
>>   	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);
>> +		if (ver_parser != ver_get)
>> +			break;
>> +
>> +		check_msg = "%s required != %d, but got %d, "
>> +			"the version is required in order run the "
>> +			"test.";
>> +
>> +		if (skip_on_error) {
>> +			tst_resm(TCONF, check_msg, cmd, ver_get,
>> +				ver_parser);
>> +		} else {
>> +			tst_brkm(TCONF, NULL, check_msg, cmd, ver_get,
>> +				ver_parser);
>>   		}
>>   		break;
>>   	}
>> diff --git a/lib/tst_test.c b/lib/tst_test.c
>> index 918bee2a1..7dfab4677 100644
>> --- a/lib/tst_test.c
>> +++ b/lib/tst_test.c
>> @@ -1154,6 +1154,9 @@ static void prepare_device(struct tst_fs *fs)
>>   
>>   	const char *const extra[] = {fs->mkfs_size_opt, NULL};
>>   
>> +	if (fs->mkfs_ver)
>> +		tst_check_cmd(fs->mkfs_ver, 1);
> So this prints tst_resm(TCONF, "...") but then proceeds with the mkfs?
>
> I suppose that both tst_check_cmd() and prepare_device() has to be
> changed to return a success/failure so that we can propagate the result
> fo the check and then we have to do:
>
> 	if (prepare_device(fs))
> 		return;
>
> in the run_tcase_on_fs()...
>
> Also there is a prepare_device(fs) in the do_setup() and I suppose that
> we have to actually do tst_brkm() if called from there, so we may need
> to add the brk_on_unsuitable flag to the prepare_device() as well.
>
> I'm wondering if there is an easier way around these things. Maybe
> putting the check to prepare_device is not the best solution. Maybe we
> should put it into the do_setup() and the all filesystems loop
> separatelly as:
>
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index d226157e0..55b01ea9c 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1415,8 +1415,12 @@ 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)
> +                               tst_check_cmd(tst_test->filesystems->mkfs_ver, 0);
> +
>                          prepare_device(tst_test->filesystems);
> +               }
>          }
>
>          if (tst_test->needs_overlay && !tst_test->mount_device)
> @@ -1805,6 +1809,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, 1))
> +               return TCONF;
> +
sounds good to me. Perhaps I just noticed that "ret" value is never used 
by run_tcases_per_fs method. that has to be fixed I guess..
>          prepare_device(fs);
>
>          ret = fork_testrun();
>
>
>>   	if (tst_test->format_device)
>>   		SAFE_MKFS(tdev.dev, tdev.fs_type, fs->mkfs_opts, extra);
>>   
>> @@ -1306,7 +1309,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, 0);
>>   	}
>>   
>>   	if (tst_test->needs_drivers) {
>> diff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
>> index 8ed0f21b6..fbfbe16a7 100644
>> --- a/testcases/lib/tst_run_shell.c
>> +++ b/testcases/lib/tst_run_shell.c
>> @@ -153,12 +153,14 @@ 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,
>>   };
>>   
>>   static ujson_obj_attr fs_attrs[] = {
>>   	UJSON_OBJ_ATTR_IDX(MKFS_OPTS, "mkfs_opts", UJSON_ARR),
>> +	UJSON_OBJ_ATTR_IDX(MKFS_VER, "mkfs_ver", UJSON_STR),
>>   	UJSON_OBJ_ATTR_IDX(MKFS_SIZE_OPT, "mkfs_size_opt", UJSON_STR),
> These have to be sorted, so MKFS_VER has to go after MKFS_SIZE_OPT. As
> it is the parser would exit with a failure when passed these attributes.
>
>>   	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

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

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

[Cyril Hrubis]

Hi!
> > +       if (fs->mkfs_ver && tst_check_cmd(fs->mkfs_ver, 1))
> > +               return TCONF;
> > +
> sounds good to me. Perhaps I just noticed that "ret" value is never used 
> by run_tcases_per_fs method. that has to be fixed I guess..

That was my mistake when adding the per-fs parameters and I've send a
patch to fix that yesterday.

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH v2 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                | 20 ++++++++++++++------
 testcases/lib/tst_run_shell.c |  5 +++++
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/include/tst_test.h b/include/tst_test.h
index 38d24f48c..ac3ee66af 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: 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 7dfab4677..49a46fc7b 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -946,19 +946,24 @@ static void do_exit(int ret)
 	exit(ret);
 }
 
-void check_kver(void)
+void check_kver(const char *min_kver, const int skip)
 {
+	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 (skip)
+			tst_res(TCONF, msg, min_kver);
+		else
+			tst_brk(TCONF, msg, min_kver);
 	}
 }
 
@@ -1150,6 +1155,9 @@ static void prepare_device(struct tst_fs *fs)
 	char buf[1024];
 	struct tst_fs dummy = {};
 
+	if (fs->min_kver)
+		check_kver(fs->min_kver, 1);
+
 	fs = fs ?: &dummy;
 
 	const char *const extra[] = {fs->mkfs_size_opt, NULL};
@@ -1287,7 +1295,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, 0);
 
 	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);
diff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
index fbfbe16a7..dc840782a 100644
--- a/testcases/lib/tst_run_shell.c
+++ b/testcases/lib/tst_run_shell.c
@@ -155,6 +155,7 @@ enum fs_ids {
 	MKFS_SIZE_OPT,
 	MKFS_VER,
 	MNT_FLAGS,
+	FS_MIN_KVER,
 	TYPE,
 };
 
@@ -163,6 +164,7 @@ static ujson_obj_attr fs_attrs[] = {
 	UJSON_OBJ_ATTR_IDX(MKFS_VER, "mkfs_ver", UJSON_STR),
 	UJSON_OBJ_ATTR_IDX(MKFS_SIZE_OPT, "mkfs_size_opt", UJSON_STR),
 	UJSON_OBJ_ATTR_IDX(MNT_FLAGS, "mnt_flags", UJSON_ARR),
+	UJSON_OBJ_ATTR_IDX(FS_MIN_KVER, "min_kver", UJSON_STR),
 	UJSON_OBJ_ATTR_IDX(TYPE, "type", UJSON_STR),
 };
 
@@ -229,6 +231,9 @@ static struct tst_fs *parse_filesystems(ujson_reader *reader, ujson_val *val)
 			case MKFS_VER:
 				ret[i].mkfs_ver = strdup(val->val_str);
 			break;
+			case FS_MIN_KVER:
+				ret[i].min_kver = 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

Re: [LTP] [PATCH v2 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                | 20 ++++++++++++++------
>  testcases/lib/tst_run_shell.c |  5 +++++
>  3 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 38d24f48c..ac3ee66af 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: minimum kernel version supporting the filesystem which has been
                 ^
		 Uppercase here.

> + *		created with mkfs.
       ^
       Should spaces or tabs be used here? The rest of the comments uses
       spaces.

>   */
>  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 7dfab4677..49a46fc7b 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -946,19 +946,24 @@ static void do_exit(int ret)
>  	exit(ret);
>  }
>  
> -void check_kver(void)
> +void check_kver(const char *min_kver, const int skip)
                                          ^
					  Maybe this should be called
					  brk_on_skip in both of the
					  cases.
>  {
> +	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 (skip)
> +			tst_res(TCONF, msg, min_kver);
> +		else
> +			tst_brk(TCONF, msg, min_kver);
>  	}
>  }
>  
> @@ -1150,6 +1155,9 @@ static void prepare_device(struct tst_fs *fs)
>  	char buf[1024];
>  	struct tst_fs dummy = {};
>  
> +	if (fs->min_kver)
> +		check_kver(fs->min_kver, 1);

And here as we, we print the message but continue with the action. I
suppose that we have to hook it up the same way as the mkfs check.

>  	fs = fs ?: &dummy;
>  
>  	const char *const extra[] = {fs->mkfs_size_opt, NULL};
> @@ -1287,7 +1295,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, 0);
>  
>  	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);
> diff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
> index fbfbe16a7..dc840782a 100644
> --- a/testcases/lib/tst_run_shell.c
> +++ b/testcases/lib/tst_run_shell.c
> @@ -155,6 +155,7 @@ enum fs_ids {
>  	MKFS_SIZE_OPT,
>  	MKFS_VER,
>  	MNT_FLAGS,
> +	FS_MIN_KVER,
>  	TYPE,
>  };
>  
> @@ -163,6 +164,7 @@ static ujson_obj_attr fs_attrs[] = {
>  	UJSON_OBJ_ATTR_IDX(MKFS_VER, "mkfs_ver", UJSON_STR),
>  	UJSON_OBJ_ATTR_IDX(MKFS_SIZE_OPT, "mkfs_size_opt", UJSON_STR),
>  	UJSON_OBJ_ATTR_IDX(MNT_FLAGS, "mnt_flags", UJSON_ARR),
> +	UJSON_OBJ_ATTR_IDX(FS_MIN_KVER, "min_kver", UJSON_STR),

Again has to be sorted.

>  	UJSON_OBJ_ATTR_IDX(TYPE, "type", UJSON_STR),
>  };
>  
> @@ -229,6 +231,9 @@ static struct tst_fs *parse_filesystems(ujson_reader *reader, ujson_val *val)
>  			case MKFS_VER:
>  				ret[i].mkfs_ver = strdup(val->val_str);
>  			break;
> +			case FS_MIN_KVER:
> +				ret[i].min_kver = 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

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH v2 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.

In particular:
- 4.16 removed experimental support for XFS reflink
- 6.7 added support for bcachefs

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

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c
index f5407f789..2e2962872 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone01.c
@@ -112,10 +112,15 @@ static struct tst_test test = {
 	.mount_device = 1,
 	.mntpoint = MNTPOINT,
 	.filesystems = (struct tst_fs []) {
-		{.type = "bcachefs"},
 		{.type = "btrfs"},
+		{
+			.type = "bcachefs",
+			.min_kver = "6.7",
+		},
 		{
 			.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..fa99d634e 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c
@@ -110,10 +110,15 @@ static struct tst_test test = {
 	.mount_device = 1,
 	.mntpoint = MNTPOINT,
 	.filesystems = (struct tst_fs []) {
-		{.type = "bcachefs"},
 		{.type = "btrfs"},
+		{
+			.type = "bcachefs",
+			.min_kver = "6.7",
+		},
 		{
 			.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..800bf871d 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_ficlonerange01.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlonerange01.c
@@ -145,10 +145,15 @@ static struct tst_test test = {
 	.mount_device = 1,
 	.mntpoint = MNTPOINT,
 	.filesystems = (struct tst_fs []) {
-		{.type = "bcachefs"},
 		{.type = "btrfs"},
+		{
+			.type = "bcachefs",
+			.min_kver = "6.7",
+		},
 		{
 			.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..6c76f83f7 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_ficlonerange02.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlonerange02.c
@@ -82,10 +82,15 @@ static struct tst_test test = {
 	.mount_device = 1,
 	.mntpoint = MNTPOINT,
 	.filesystems = (struct tst_fs []) {
-		{.type = "bcachefs"},
 		{.type = "btrfs"},
+		{
+			.type = "bcachefs",
+			.min_kver = "6.7",
+		},
 		{
 			.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 v2 3/3] Setup minimal kernel for ioctl_clone(range) tests

[Cyril Hrubis]

Hi!
> +		{
> +			.type = "bcachefs",
> +			.min_kver = "6.7",

I do not think that the min_kver is needed here, because if there is no
kernel support for bcachefs it will be skipped simply because it's
missing.

-- 
Cyril Hrubis
chrubis@suse.cz

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