| Message ID | 20241001-ioctl_ficlone01_fix-v2-1-dd0b021dd31d@suse.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix ioctl_ficlone(range) failures on certain FS | expand |
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
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
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.
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;