* [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
* [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 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 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 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
* 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.