* [LTP] [PATCH 0/3 v2] tst_test.sh: Fix filesystem support detection
@ 2022-09-22 21:09 Petr Vorel
2022-09-22 21:09 ` [LTP] [PATCH 1/3 v2] tst_supported_fs: Unify messaging Petr Vorel
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Petr Vorel @ 2022-09-22 21:09 UTC (permalink / raw)
To: ltp
changes v1->v2:
* based on the top of Martin's patch [1]: using tst_supported_fs -s ..
-d. instead of detecting filesystem with df.
* remove tst_skip_filesystems_skip.sh test
* add TINFO message to tst_test.sh
new commits:
* tst_supported_fs: Unify messaging
* tst_supported_fs: Fix return codes
[1] https://patchwork.ozlabs.org/project/ltp/patch/20220921155006.13360-1-mdoucha@suse.cz/
Petr Vorel (3):
tst_supported_fs: Unify messaging
tst_supported_fs: Fix return codes
tst_test.sh: Fix filesystem support detection
.../shell/tst_skip_filesystems_skip.sh | 17 -----
testcases/lib/tst_supported_fs.c | 65 +++++++++----------
testcases/lib/tst_test.sh | 14 ++--
3 files changed, 40 insertions(+), 56 deletions(-)
delete mode 100755 lib/newlib_tests/shell/tst_skip_filesystems_skip.sh
--
2.37.3
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 13+ messages in thread* [LTP] [PATCH 1/3 v2] tst_supported_fs: Unify messaging 2022-09-22 21:09 [LTP] [PATCH 0/3 v2] tst_test.sh: Fix filesystem support detection Petr Vorel @ 2022-09-22 21:09 ` Petr Vorel 2022-09-23 2:43 ` Li Wang 2022-09-23 15:23 ` Cyril Hrubis 2022-09-22 21:09 ` [LTP] [PATCH 2/3 v2] tst_supported_fs: Fix return codes Petr Vorel 2022-09-22 21:09 ` [LTP] [PATCH 3/3 v2] tst_test.sh: Fix filesystem support detection Petr Vorel 2 siblings, 2 replies; 13+ messages in thread From: Petr Vorel @ 2022-09-22 21:09 UTC (permalink / raw) To: ltp Most of the messages used fprintf() instead of tst_{res,brk}(), thus convert all messages to fprintf(). Add macros to shorten code. Fixes: eb47b4497 ("tst_supported_fs: Support skip list when query single fs") Signed-off-by: Petr Vorel <pvorel@suse.cz> --- I'm not sure about this myself. Shouldn't we rather use tst_brk() and tst_res() instead? It's show tst_supported_fs as command. testcases/lib/tst_supported_fs.c | 61 ++++++++++++++++---------------- 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/testcases/lib/tst_supported_fs.c b/testcases/lib/tst_supported_fs.c index 26577c726..947aa4dae 100644 --- a/testcases/lib/tst_supported_fs.c +++ b/testcases/lib/tst_supported_fs.c @@ -15,6 +15,22 @@ #include "tst_test.h" #include "tst_fs.h" +#define err(...) ({ \ + fprintf(stderr, __VA_ARGS__); \ + fprintf(stderr, "\n"); \ + usage(); \ + return 2; }) + +#define fail(...) ({ \ + fprintf(stderr, __VA_ARGS__); \ + fprintf(stderr, "\n"); \ + return 1; }) + +#define info(...) ({ \ + fprintf(stderr, __VA_ARGS__); \ + fprintf(stderr, "\n"); \ + return 0; }) + static void usage(void) { fprintf(stderr, "Usage:\n"); @@ -90,67 +106,50 @@ int main(int argc, char *argv[]) break; case 'd': - if (fsname) { - fprintf(stderr, - "Can't specify multiple paths\n"); - usage(); - return 2; - } + if (fsname) + err("Can't specify multiple paths"); fsname = tst_fs_type_name(tst_fs_type(optarg)); break; } } - if (fsname && !skiplist) { - fprintf(stderr, "Parameter -d requires skiplist\n"); - usage(); - return 2; - } + if (fsname && !skiplist) + err("Parameter -d requires skiplist"); - if (argc - optind > 1) { - fprintf(stderr, "Can't specify multiple fs_type\n"); - usage(); - return 2; - } + if (argc - optind > 1) + err("Can't specify multiple fs_type"); /* fs_type */ if (optind < argc) { - if (fsname) { - fprintf(stderr, "Can't specify fs_type and -d together\n"); - usage(); - return 2; - } + if (fsname) + err("Can't specify fs_type and -d together"); fsname = argv[optind]; } if (fsname) { if (fsname[0] == '\0') - tst_brk(TCONF, "fs_type is empty"); + err("fs_type is empty"); if (skiplist) { if (tst_fs_in_skiplist(fsname, (const char * const*)skiplist)) - tst_brk(TCONF, "%s is skipped", fsname); - else - tst_res(TINFO, "%s is not skipped", fsname); + fail("%s is skipped", fsname); - return 0; + info("%s is not skipped", fsname); } if (tst_fs_is_supported(fsname) == TST_FS_UNSUPPORTED) - tst_brk(TCONF, "%s is not supported", fsname); - else - tst_res(TINFO, "%s is supported", fsname); + fail("%s is not supported", fsname); - return 0; + info("%s is supported", fsname); } /* all filesystems */ filesystems = tst_get_supported_fs_types((const char * const*)skiplist); if (!filesystems[0]) - tst_brk(TCONF, "There are no supported filesystems or all skipped"); + fail("There are no supported filesystems or all skipped"); for (i = 0; filesystems[i]; i++) printf("%s\n", filesystems[i]); -- 2.37.3 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH 1/3 v2] tst_supported_fs: Unify messaging 2022-09-22 21:09 ` [LTP] [PATCH 1/3 v2] tst_supported_fs: Unify messaging Petr Vorel @ 2022-09-23 2:43 ` Li Wang 2022-09-23 6:05 ` Petr Vorel 2022-09-23 15:23 ` Cyril Hrubis 1 sibling, 1 reply; 13+ messages in thread From: Li Wang @ 2022-09-23 2:43 UTC (permalink / raw) To: Petr Vorel; +Cc: LTP List [-- Attachment #1.1: Type: text/plain, Size: 4974 bytes --] On Fri, Sep 23, 2022 at 5:09 AM Petr Vorel <pvorel@suse.cz> wrote: > Most of the messages used fprintf() instead of tst_{res,brk}(), > thus convert all messages to fprintf(). > > Add macros to shorten code. > > Fixes: eb47b4497 ("tst_supported_fs: Support skip list when query single > fs") > > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > I'm not sure about this myself. Shouldn't we rather use tst_brk() and > tst_res() instead? It's show tst_supported_fs as command. > Not exactly, looking at testcase/lib/* tools, most of them are not written by LTP standard API, some even do not include tst_test.h. I personally think if we want more flexibility for those small programs as auxiliary tool, we should not apply API as dogmatism for everything. Btw, there is patch confliction when performing git-am, if you can rebase accordingly for solving that, the whole patchset will be great for me. Reviewed-by: Li Wang <liwang@redhat.com> > > > testcases/lib/tst_supported_fs.c | 61 ++++++++++++++++---------------- > 1 file changed, 30 insertions(+), 31 deletions(-) > > diff --git a/testcases/lib/tst_supported_fs.c > b/testcases/lib/tst_supported_fs.c > index 26577c726..947aa4dae 100644 > --- a/testcases/lib/tst_supported_fs.c > +++ b/testcases/lib/tst_supported_fs.c > @@ -15,6 +15,22 @@ > #include "tst_test.h" > #include "tst_fs.h" > > +#define err(...) ({ \ > + fprintf(stderr, __VA_ARGS__); \ > + fprintf(stderr, "\n"); \ > + usage(); \ > + return 2; }) > + > +#define fail(...) ({ \ > + fprintf(stderr, __VA_ARGS__); \ > + fprintf(stderr, "\n"); \ > + return 1; }) > + > +#define info(...) ({ \ > + fprintf(stderr, __VA_ARGS__); \ > + fprintf(stderr, "\n"); \ > + return 0; }) > + > static void usage(void) > { > fprintf(stderr, "Usage:\n"); > @@ -90,67 +106,50 @@ int main(int argc, char *argv[]) > break; > > case 'd': > - if (fsname) { > - fprintf(stderr, > - "Can't specify multiple paths\n"); > - usage(); > - return 2; > - } > + if (fsname) > + err("Can't specify multiple paths"); > > fsname = tst_fs_type_name(tst_fs_type(optarg)); > break; > } > } > > - if (fsname && !skiplist) { > - fprintf(stderr, "Parameter -d requires skiplist\n"); > - usage(); > - return 2; > - } > + if (fsname && !skiplist) > + err("Parameter -d requires skiplist"); > > - if (argc - optind > 1) { > - fprintf(stderr, "Can't specify multiple fs_type\n"); > - usage(); > - return 2; > - } > + if (argc - optind > 1) > + err("Can't specify multiple fs_type"); > > /* fs_type */ > if (optind < argc) { > - if (fsname) { > - fprintf(stderr, "Can't specify fs_type and -d > together\n"); > - usage(); > - return 2; > - } > + if (fsname) > + err("Can't specify fs_type and -d together"); > > fsname = argv[optind]; > } > > if (fsname) { > if (fsname[0] == '\0') > - tst_brk(TCONF, "fs_type is empty"); > + err("fs_type is empty"); > > if (skiplist) { > if (tst_fs_in_skiplist(fsname, (const char * > const*)skiplist)) > - tst_brk(TCONF, "%s is skipped", fsname); > - else > - tst_res(TINFO, "%s is not skipped", > fsname); > + fail("%s is skipped", fsname); > > - return 0; > + info("%s is not skipped", fsname); > } > > if (tst_fs_is_supported(fsname) == TST_FS_UNSUPPORTED) > - tst_brk(TCONF, "%s is not supported", fsname); > - else > - tst_res(TINFO, "%s is supported", fsname); > + fail("%s is not supported", fsname); > > - return 0; > + info("%s is supported", fsname); > } > > /* all filesystems */ > filesystems = tst_get_supported_fs_types((const char * > const*)skiplist); > > if (!filesystems[0]) > - tst_brk(TCONF, "There are no supported filesystems or all > skipped"); > + fail("There are no supported filesystems or all skipped"); > > for (i = 0; filesystems[i]; i++) > printf("%s\n", filesystems[i]); > -- > 2.37.3 > > -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 7693 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH 1/3 v2] tst_supported_fs: Unify messaging 2022-09-23 2:43 ` Li Wang @ 2022-09-23 6:05 ` Petr Vorel 0 siblings, 0 replies; 13+ messages in thread From: Petr Vorel @ 2022-09-23 6:05 UTC (permalink / raw) To: Li Wang; +Cc: LTP List Hi Li, all, > > I'm not sure about this myself. Shouldn't we rather use tst_brk() and > > tst_res() instead? It's show tst_supported_fs as command. > Not exactly, looking at testcase/lib/* tools, most of them are not written > by LTP standard API, some even do not include tst_test.h. FYI I meant tst_{brk,res} message starts with filename and line, which is kind of handy. > I personally think if we want more flexibility for those small programs > as auxiliary tool, we should not apply API as dogmatism for everything. Sure > Btw, there is patch confliction when performing git-am, if you can > rebase accordingly for solving that, the whole patchset will be > great for me. I'm sorry, indeed git-am does not work. I remember rebasing it (and my remote branch is rebased), I obviously must have run git format-patch before rebasing. In case of anybody want to actually run it, it's in my fork: https://github.com/pevik/ltp/commits/fix/TST_ALL_FILESYSTEMS.v2 > Reviewed-by: Li Wang <liwang@redhat.com> Thanks! Kind regards, Petr -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH 1/3 v2] tst_supported_fs: Unify messaging 2022-09-22 21:09 ` [LTP] [PATCH 1/3 v2] tst_supported_fs: Unify messaging Petr Vorel 2022-09-23 2:43 ` Li Wang @ 2022-09-23 15:23 ` Cyril Hrubis 2022-09-23 16:31 ` Petr Vorel 1 sibling, 1 reply; 13+ messages in thread From: Cyril Hrubis @ 2022-09-23 15:23 UTC (permalink / raw) To: Petr Vorel; +Cc: ltp Hi! > +#define err(...) ({ \ > + fprintf(stderr, __VA_ARGS__); \ > + fprintf(stderr, "\n"); \ > + usage(); \ > + return 2; }) ^ This should rather be exit(2); It's only matter of time until someone uses that in a function outside of main. > +#define fail(...) ({ \ > + fprintf(stderr, __VA_ARGS__); \ > + fprintf(stderr, "\n"); \ > + return 1; }) Here as well. > +#define info(...) ({ \ > + fprintf(stderr, __VA_ARGS__); \ > + fprintf(stderr, "\n"); \ > + return 0; }) The naming here is a bit of strange, I wouldn't expect that function called info() would exit the process. Maybe these three should include exit in the function name such as info_exit(), err_exit() and fail_exit(). > static void usage(void) > { > fprintf(stderr, "Usage:\n"); > @@ -90,67 +106,50 @@ int main(int argc, char *argv[]) > break; > > case 'd': > - if (fsname) { > - fprintf(stderr, > - "Can't specify multiple paths\n"); > - usage(); > - return 2; > - } > + if (fsname) > + err("Can't specify multiple paths"); > > fsname = tst_fs_type_name(tst_fs_type(optarg)); > break; > } > } > > - if (fsname && !skiplist) { > - fprintf(stderr, "Parameter -d requires skiplist\n"); > - usage(); > - return 2; > - } > + if (fsname && !skiplist) > + err("Parameter -d requires skiplist"); > > - if (argc - optind > 1) { > - fprintf(stderr, "Can't specify multiple fs_type\n"); > - usage(); > - return 2; > - } > + if (argc - optind > 1) > + err("Can't specify multiple fs_type"); > > /* fs_type */ > if (optind < argc) { > - if (fsname) { > - fprintf(stderr, "Can't specify fs_type and -d together\n"); > - usage(); > - return 2; > - } > + if (fsname) > + err("Can't specify fs_type and -d together"); > > fsname = argv[optind]; > } > > if (fsname) { > if (fsname[0] == '\0') > - tst_brk(TCONF, "fs_type is empty"); > + err("fs_type is empty"); > > if (skiplist) { > if (tst_fs_in_skiplist(fsname, (const char * const*)skiplist)) > - tst_brk(TCONF, "%s is skipped", fsname); > - else > - tst_res(TINFO, "%s is not skipped", fsname); > + fail("%s is skipped", fsname); > > - return 0; > + info("%s is not skipped", fsname); > } > > if (tst_fs_is_supported(fsname) == TST_FS_UNSUPPORTED) > - tst_brk(TCONF, "%s is not supported", fsname); > - else > - tst_res(TINFO, "%s is supported", fsname); > + fail("%s is not supported", fsname); > > - return 0; > + info("%s is supported", fsname); > } > > /* all filesystems */ > filesystems = tst_get_supported_fs_types((const char * const*)skiplist); > > if (!filesystems[0]) > - tst_brk(TCONF, "There are no supported filesystems or all skipped"); > + fail("There are no supported filesystems or all skipped"); > > for (i = 0; filesystems[i]; i++) > printf("%s\n", filesystems[i]); > -- > 2.37.3 > -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH 1/3 v2] tst_supported_fs: Unify messaging 2022-09-23 15:23 ` Cyril Hrubis @ 2022-09-23 16:31 ` Petr Vorel 0 siblings, 0 replies; 13+ messages in thread From: Petr Vorel @ 2022-09-23 16:31 UTC (permalink / raw) To: Cyril Hrubis; +Cc: ltp > Hi! > > +#define err(...) ({ \ > > + fprintf(stderr, __VA_ARGS__); \ > > + fprintf(stderr, "\n"); \ > > + usage(); \ > > + return 2; }) > ^ > This should rather be exit(2); Thanks! I thought that just after sending that. > It's only matter of time until someone uses that in a function outside > of main. > > +#define fail(...) ({ \ > > + fprintf(stderr, __VA_ARGS__); \ > > + fprintf(stderr, "\n"); \ > > + return 1; }) > Here as well. > > +#define info(...) ({ \ > > + fprintf(stderr, __VA_ARGS__); \ > > + fprintf(stderr, "\n"); \ > > + return 0; }) > The naming here is a bit of strange, I wouldn't expect that function > called info() would exit the process. > Maybe these three should include exit in the function name such as > info_exit(), err_exit() and fail_exit(). +1 I'll fix it in another version or fix it before merge (if all changes are trivial). Kind regards, Petr -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [PATCH 2/3 v2] tst_supported_fs: Fix return codes 2022-09-22 21:09 [LTP] [PATCH 0/3 v2] tst_test.sh: Fix filesystem support detection Petr Vorel 2022-09-22 21:09 ` [LTP] [PATCH 1/3 v2] tst_supported_fs: Unify messaging Petr Vorel @ 2022-09-22 21:09 ` Petr Vorel 2022-09-22 21:09 ` [LTP] [PATCH 3/3 v2] tst_test.sh: Fix filesystem support detection Petr Vorel 2 siblings, 0 replies; 13+ messages in thread From: Petr Vorel @ 2022-09-22 21:09 UTC (permalink / raw) To: ltp exit code 2 is for errors not related to filesystem detection. Fixes: eb47b4497 ("tst_supported_fs: Support skip list when query single fs") Signed-off-by: Petr Vorel <pvorel@suse.cz> --- testcases/lib/tst_supported_fs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testcases/lib/tst_supported_fs.c b/testcases/lib/tst_supported_fs.c index 947aa4dae..f644bbd3f 100644 --- a/testcases/lib/tst_supported_fs.c +++ b/testcases/lib/tst_supported_fs.c @@ -93,7 +93,7 @@ int main(int argc, char *argv[]) switch (ret) { case '?': usage(); - return 1; + return 2; case 'h': usage(); @@ -102,7 +102,7 @@ int main(int argc, char *argv[]) case 's': skiplist = parse_skiplist(optarg); if (!skiplist) - return 1; + return 2; break; case 'd': -- 2.37.3 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [LTP] [PATCH 3/3 v2] tst_test.sh: Fix filesystem support detection 2022-09-22 21:09 [LTP] [PATCH 0/3 v2] tst_test.sh: Fix filesystem support detection Petr Vorel 2022-09-22 21:09 ` [LTP] [PATCH 1/3 v2] tst_supported_fs: Unify messaging Petr Vorel 2022-09-22 21:09 ` [LTP] [PATCH 2/3 v2] tst_supported_fs: Fix return codes Petr Vorel @ 2022-09-22 21:09 ` Petr Vorel 2022-09-23 15:29 ` Cyril Hrubis 2 siblings, 1 reply; 13+ messages in thread From: Petr Vorel @ 2022-09-22 21:09 UTC (permalink / raw) To: ltp Filesystem detection of locally used filesystem was broken on tests which did not use TST_ALL_FILESYSTEMS as it 1) expected used filesystem is $TST_FS_TYPE 2) this variable was not yet set. Also this check makes sense only if test defines TST_SKIP_FILESYSTEMS (to align with the condition in do_test_setup() in C API). Move filesystem check after (optional) cd "$TST_TMPDIR" (TMPDIR can have different filesystem). Remove tst_skip_filesystems_skip.sh test, as filesystem on tmpfs would have to be detected before. Fixes: 1f6bd6e66 ("tst_test.sh: Add $TST_ALL_FILESYSTEMS") Reported-by: Martin Doucha <mdoucha@suse.cz> Signed-off-by: Petr Vorel <pvorel@suse.cz> --- .../shell/tst_skip_filesystems_skip.sh | 17 ----------------- testcases/lib/tst_test.sh | 14 ++++++++------ 2 files changed, 8 insertions(+), 23 deletions(-) delete mode 100755 lib/newlib_tests/shell/tst_skip_filesystems_skip.sh diff --git a/lib/newlib_tests/shell/tst_skip_filesystems_skip.sh b/lib/newlib_tests/shell/tst_skip_filesystems_skip.sh deleted file mode 100755 index 6748d021d..000000000 --- a/lib/newlib_tests/shell/tst_skip_filesystems_skip.sh +++ /dev/null @@ -1,17 +0,0 @@ -#!/bin/sh -# SPDX-License-Identifier: GPL-2.0-or-later -# Copyright (c) 2022 Petr Vorel <pvorel@suse.cz> - -TST_MOUNT_DEVICE=1 -TST_NEEDS_ROOT=1 -TST_FS_TYPE=ext4 -TST_TESTFUNC=test -TST_SKIP_FILESYSTEMS="ext4" - -test() -{ - tst_res TFAIL "test should be skipped with TCONF" -} - -. tst_test.sh -tst_run diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh index 33aadea91..01c01b79b 100644 --- a/testcases/lib/tst_test.sh +++ b/testcases/lib/tst_test.sh @@ -702,12 +702,6 @@ tst_run() [ "$TST_FORMAT_DEVICE" = 1 -o "$TST_ALL_FILESYSTEMS" = 1 ] && TST_NEEDS_DEVICE=1 [ "$TST_NEEDS_DEVICE" = 1 ] && TST_NEEDS_TMPDIR=1 - if [ "$TST_ALL_FILESYSTEMS" != 1 ]; then - if ! tst_supported_fs -s "$TST_SKIP_FILESYSTEMS" $TST_FS_TYPE > /dev/null; then - tst_brk TCONF "$TST_FS_TYPE is skipped by the test" - fi - fi - if [ "$TST_NEEDS_DEVICE" = 1 ]; then TST_DEVICE=$(tst_device acquire) @@ -735,6 +729,14 @@ tst_run() cd "$TST_TMPDIR" fi + if [ "$TST_ALL_FILESYSTEMS" != 1 -a "$TST_SKIP_FILESYSTEMS" ]; then + if ! tst_supported_fs -s "$TST_SKIP_FILESYSTEMS" -d . > /dev/null; then + tst_brk TCONF "filesystem is not supported by the test" + fi + + tst_res TINFO "filesystem is supported by the test" + fi + [ -n "$TST_NEEDS_CHECKPOINTS" ] && _tst_init_checkpoints TST_MNTPOINT="${TST_MNTPOINT:-$PWD/mntpoint}" -- 2.37.3 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH 3/3 v2] tst_test.sh: Fix filesystem support detection 2022-09-22 21:09 ` [LTP] [PATCH 3/3 v2] tst_test.sh: Fix filesystem support detection Petr Vorel @ 2022-09-23 15:29 ` Cyril Hrubis 2022-09-23 16:36 ` Petr Vorel 0 siblings, 1 reply; 13+ messages in thread From: Cyril Hrubis @ 2022-09-23 15:29 UTC (permalink / raw) To: Petr Vorel; +Cc: ltp Hi! > Fixes: 1f6bd6e66 ("tst_test.sh: Add $TST_ALL_FILESYSTEMS") > > Reported-by: Martin Doucha <mdoucha@suse.cz> > Signed-off-by: Petr Vorel <pvorel@suse.cz> Looks like this is the only real fix in the series, right? The actual change looks good, but I do wonder what exactly has been broken, git grpe TST_SKIP_FILESYSTEMS does not show any real tests that would use that variable. -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH 3/3 v2] tst_test.sh: Fix filesystem support detection 2022-09-23 15:29 ` Cyril Hrubis @ 2022-09-23 16:36 ` Petr Vorel 2022-09-26 9:13 ` Cyril Hrubis 0 siblings, 1 reply; 13+ messages in thread From: Petr Vorel @ 2022-09-23 16:36 UTC (permalink / raw) To: Cyril Hrubis; +Cc: ltp > Hi! > > Fixes: 1f6bd6e66 ("tst_test.sh: Add $TST_ALL_FILESYSTEMS") > > Reported-by: Martin Doucha <mdoucha@suse.cz> > > Signed-off-by: Petr Vorel <pvorel@suse.cz> > Looks like this is the only real fix in the series, right? > The actual change looks good, but I do wonder what exactly has been > broken, git grpe TST_SKIP_FILESYSTEMS does not show any real tests > that would use that variable. Broken were actually all shell tests which did not use TST_SKIP_FILESYSTEMS: e.g. all tests in net_stress.ipsec_* did run whole filesystem check: tst_supported_fs_types.c:93: TINFO: Kernel supports ext2 tst_supported_fs_types.c:55: TINFO: mkfs.ext2 does exist tst_supported_fs_types.c:93: TINFO: Kernel supports ext3 tst_supported_fs_types.c:55: TINFO: mkfs.ext3 does exist tst_supported_fs_types.c:93: TINFO: Kernel supports ext4 tst_supported_fs_types.c:55: TINFO: mkfs.ext4 does exist tst_supported_fs_types.c:93: TINFO: Kernel supports xfs tst_supported_fs_types.c:55: TINFO: mkfs.xfs does exist tst_supported_fs_types.c:93: TINFO: Kernel supports btrfs tst_supported_fs_types.c:55: TINFO: mkfs.btrfs does exist tst_supported_fs_types.c:93: TINFO: Kernel supports vfat tst_supported_fs_types.c:55: TINFO: mkfs.vfat does exist tst_supported_fs_types.c:93: TINFO: Kernel supports exfat tst_supported_fs_types.c:55: TINFO: mkfs.exfat does exist tst_supported_fs_types.c:123: TINFO: FUSE does support ntfs tst_supported_fs_types.c:55: TINFO: mkfs.ntfs does exist tst_supported_fs_types.c:93: TINFO: Kernel supports tmpfs tst_supported_fs_types.c:42: TINFO: mkfs is not needed for tmpfs sctp_ipsec 1 TINFO: timeout per run is 0h 5m 0s sctp_ipsec 1 TINFO: IPsec[ah/transport] sctp_ipsec 1 TINFO: run server 'netstress -T sctp -S 10.0.0.1 -D ltp_ns_veth1 -R 500000 -B /tmp/LTP_sctp_ipsec.hC471AeJ9L' ... instead checking just filesystem in TMPDIR due empty $TST_FS_TYPE (I should have quoted it). + there are IMA tests had this + specific problem. Kind regards, Petr -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH 3/3 v2] tst_test.sh: Fix filesystem support detection 2022-09-23 16:36 ` Petr Vorel @ 2022-09-26 9:13 ` Cyril Hrubis 2022-09-26 9:34 ` Petr Vorel 2022-09-26 9:45 ` Petr Vorel 0 siblings, 2 replies; 13+ messages in thread From: Cyril Hrubis @ 2022-09-26 9:13 UTC (permalink / raw) To: Petr Vorel; +Cc: ltp Hi! > Broken were actually all shell tests which did not use TST_SKIP_FILESYSTEMS: > e.g. all tests in net_stress.ipsec_* did run whole filesystem check: > > tst_supported_fs_types.c:93: TINFO: Kernel supports ext2 > tst_supported_fs_types.c:55: TINFO: mkfs.ext2 does exist > tst_supported_fs_types.c:93: TINFO: Kernel supports ext3 > tst_supported_fs_types.c:55: TINFO: mkfs.ext3 does exist > tst_supported_fs_types.c:93: TINFO: Kernel supports ext4 > tst_supported_fs_types.c:55: TINFO: mkfs.ext4 does exist > tst_supported_fs_types.c:93: TINFO: Kernel supports xfs > tst_supported_fs_types.c:55: TINFO: mkfs.xfs does exist > tst_supported_fs_types.c:93: TINFO: Kernel supports btrfs > tst_supported_fs_types.c:55: TINFO: mkfs.btrfs does exist > tst_supported_fs_types.c:93: TINFO: Kernel supports vfat > tst_supported_fs_types.c:55: TINFO: mkfs.vfat does exist > tst_supported_fs_types.c:93: TINFO: Kernel supports exfat > tst_supported_fs_types.c:55: TINFO: mkfs.exfat does exist > tst_supported_fs_types.c:123: TINFO: FUSE does support ntfs > tst_supported_fs_types.c:55: TINFO: mkfs.ntfs does exist > tst_supported_fs_types.c:93: TINFO: Kernel supports tmpfs > tst_supported_fs_types.c:42: TINFO: mkfs is not needed for tmpfs > sctp_ipsec 1 TINFO: timeout per run is 0h 5m 0s > sctp_ipsec 1 TINFO: IPsec[ah/transport] > sctp_ipsec 1 TINFO: run server 'netstress -T sctp -S 10.0.0.1 -D ltp_ns_veth1 -R 500000 -B /tmp/LTP_sctp_ipsec.hC471AeJ9L' > ... > > instead checking just filesystem in TMPDIR due empty $TST_FS_TYPE (I should have > quoted it). > > + there are IMA tests had this + specific problem. Ah right, so it did useless checks and printed out a lot of lines too. Reviewed-by: Cyril Hrubis <chrubis@suse.cz> -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH 3/3 v2] tst_test.sh: Fix filesystem support detection 2022-09-26 9:13 ` Cyril Hrubis @ 2022-09-26 9:34 ` Petr Vorel 2022-09-26 9:45 ` Petr Vorel 1 sibling, 0 replies; 13+ messages in thread From: Petr Vorel @ 2022-09-26 9:34 UTC (permalink / raw) To: Cyril Hrubis; +Cc: ltp > Hi! > > Broken were actually all shell tests which did not use TST_SKIP_FILESYSTEMS: > > e.g. all tests in net_stress.ipsec_* did run whole filesystem check: > > tst_supported_fs_types.c:93: TINFO: Kernel supports ext2 > > tst_supported_fs_types.c:55: TINFO: mkfs.ext2 does exist > > tst_supported_fs_types.c:93: TINFO: Kernel supports ext3 > > tst_supported_fs_types.c:55: TINFO: mkfs.ext3 does exist > > tst_supported_fs_types.c:93: TINFO: Kernel supports ext4 > > tst_supported_fs_types.c:55: TINFO: mkfs.ext4 does exist > > tst_supported_fs_types.c:93: TINFO: Kernel supports xfs > > tst_supported_fs_types.c:55: TINFO: mkfs.xfs does exist > > tst_supported_fs_types.c:93: TINFO: Kernel supports btrfs > > tst_supported_fs_types.c:55: TINFO: mkfs.btrfs does exist > > tst_supported_fs_types.c:93: TINFO: Kernel supports vfat > > tst_supported_fs_types.c:55: TINFO: mkfs.vfat does exist > > tst_supported_fs_types.c:93: TINFO: Kernel supports exfat > > tst_supported_fs_types.c:55: TINFO: mkfs.exfat does exist > > tst_supported_fs_types.c:123: TINFO: FUSE does support ntfs > > tst_supported_fs_types.c:55: TINFO: mkfs.ntfs does exist > > tst_supported_fs_types.c:93: TINFO: Kernel supports tmpfs > > tst_supported_fs_types.c:42: TINFO: mkfs is not needed for tmpfs > > sctp_ipsec 1 TINFO: timeout per run is 0h 5m 0s > > sctp_ipsec 1 TINFO: IPsec[ah/transport] > > sctp_ipsec 1 TINFO: run server 'netstress -T sctp -S 10.0.0.1 -D ltp_ns_veth1 -R 500000 -B /tmp/LTP_sctp_ipsec.hC471AeJ9L' > > ... > > instead checking just filesystem in TMPDIR due empty $TST_FS_TYPE (I should have > > quoted it). > > + there are IMA tests had this + specific problem. > Ah right, so it did useless checks and printed out a lot of lines too. Also IMA fixes patchset needs this, otherwise it gets: tst_supported_fs.c:134: TCONF: tmpfs is skipped > Reviewed-by: Cyril Hrubis <chrubis@suse.cz> Thanks! Kind regards, Petr -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH 3/3 v2] tst_test.sh: Fix filesystem support detection 2022-09-26 9:13 ` Cyril Hrubis 2022-09-26 9:34 ` Petr Vorel @ 2022-09-26 9:45 ` Petr Vorel 1 sibling, 0 replies; 13+ messages in thread From: Petr Vorel @ 2022-09-26 9:45 UTC (permalink / raw) To: Cyril Hrubis; +Cc: ltp Hi all, Cyril, Li, thanks a lot for your review, merged! Kind regards, Petr -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-09-26 9:46 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-22 21:09 [LTP] [PATCH 0/3 v2] tst_test.sh: Fix filesystem support detection Petr Vorel 2022-09-22 21:09 ` [LTP] [PATCH 1/3 v2] tst_supported_fs: Unify messaging Petr Vorel 2022-09-23 2:43 ` Li Wang 2022-09-23 6:05 ` Petr Vorel 2022-09-23 15:23 ` Cyril Hrubis 2022-09-23 16:31 ` Petr Vorel 2022-09-22 21:09 ` [LTP] [PATCH 2/3 v2] tst_supported_fs: Fix return codes Petr Vorel 2022-09-22 21:09 ` [LTP] [PATCH 3/3 v2] tst_test.sh: Fix filesystem support detection Petr Vorel 2022-09-23 15:29 ` Cyril Hrubis 2022-09-23 16:36 ` Petr Vorel 2022-09-26 9:13 ` Cyril Hrubis 2022-09-26 9:34 ` Petr Vorel 2022-09-26 9:45 ` Petr Vorel
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.