Message ID | 20220915093639.2261-4-pvorel@suse.cz |
---|---|
State | Accepted |
Headers | show |
Series | shell: df01.sh: $TST_ALL_FILESYSTEMS | expand |
Hi! > And use this feature in zram01.sh. This looks like an leftover. > Also print TINFO if test it supported by the test, quit with TCONF > otherwise (code from do_test_setup() tst_test.c). > > Acked-by: Richard Palethorpe <rpalethorpe@suse.com> > Reviewed-by: Li Wang <liwang@redhat.com> > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > testcases/lib/tst_supported_fs.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/testcases/lib/tst_supported_fs.c b/testcases/lib/tst_supported_fs.c > index 2b42d5bb3..e2261244d 100644 > --- a/testcases/lib/tst_supported_fs.c > +++ b/testcases/lib/tst_supported_fs.c > @@ -80,14 +80,19 @@ int main(int argc, char *argv[]) > return 2; > } > > - if (optind < argc) > - return !tst_fs_is_supported(argv[optind]); > + if (optind < argc) { > + if (tst_fs_in_skiplist(argv[optind], (const char * const*)skiplist)) > + tst_brk(TCONF, "%s is not supported by the test", argv[optind]); > > + tst_res(TINFO, "%s is supported by the test", argv[optind]); > + > + return 0; > + } So we now do not check if filesystem is supported just check against the skiplist? I guess that it would mqake sense if -s option was present, but shouldn't we check for mkfs and kernel support without -s if filesystem was specified? > filesystems = tst_get_supported_fs_types((const char * const*)skiplist); > > if (!filesystems[0]) > - tst_brk(TCONF, "There are no supported filesystems"); > + tst_brk(TCONF, "There are no supported filesystems or all skipped"); > > for (i = 0; filesystems[i]; i++) > printf("%s\n", filesystems[i]); > -- > 2.37.3 >
> Hi! > > And use this feature in zram01.sh. > This looks like an leftover. > > Also print TINFO if test it supported by the test, quit with TCONF > > otherwise (code from do_test_setup() tst_test.c). > > Acked-by: Richard Palethorpe <rpalethorpe@suse.com> > > Reviewed-by: Li Wang <liwang@redhat.com> > > Signed-off-by: Petr Vorel <pvorel@suse.cz> > > --- > > testcases/lib/tst_supported_fs.c | 11 ++++++++--- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/testcases/lib/tst_supported_fs.c b/testcases/lib/tst_supported_fs.c > > index 2b42d5bb3..e2261244d 100644 > > --- a/testcases/lib/tst_supported_fs.c > > +++ b/testcases/lib/tst_supported_fs.c > > @@ -80,14 +80,19 @@ int main(int argc, char *argv[]) > > return 2; > > } > > - if (optind < argc) > > - return !tst_fs_is_supported(argv[optind]); > > + if (optind < argc) { > > + if (tst_fs_in_skiplist(argv[optind], (const char * const*)skiplist)) > > + tst_brk(TCONF, "%s is not supported by the test", argv[optind]); > > + tst_res(TINFO, "%s is supported by the test", argv[optind]); > > + > > + return 0; > > + } > So we now do not check if filesystem is supported just check against the > skiplist? Good catch, before there was check for mkfs.foo even for single filesystem (via. tst_fs_is_supported()). Now I removed it when checking single filesystem (It's still being checked for all filesystems in code below: filesystems = tst_get_supported_fs_types((const char * const*)skiplist); ). FYI skip list without mkfs is needed for $TST_SKIP_FILESYSTEMS used without TST_ALL_FILESYSTEMS=1, i.e. in tst_test.sh: 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 not supported" fi fi > I guess that it would make sense if -s option was present, but > shouldn't we check for mkfs and kernel support without -s if filesystem > was specified? This would solve problem for prepare_lvm.sh, where code: if ! tst_supported_fs $fsname; then checks just for mkfs.foo (no skip list). The problem is with certain inconsistency of mkfs check: because when checking skip list for all filesystems, both mkfs and skip list are being addressed (i.e. check for mkfs even -s is passed). Also it might be useful in the future to check both skip list and mkfs even for single filesystem. Shouldn't there be an getopt option to decide? e.g. by default both skip list and mkfs (no matter if -s is passed) and with option (e.g. -o) check only for a list? This is not needed when checking all filesystems, only for testing single filesystem, so I wonder if I should implement it for all filesystems mode. But as this is not needed I'm ok to implement what you suggest: tst_supported_fs -s skiplist foo would check only if the used filesystem is not filtered by skip list (used in tst_test.sh). tst_supported_fs foo would check only for mkfs.foo (used in prepare_lvm.sh). What do you prefer? Kind regards, Petr > > filesystems = tst_get_supported_fs_types((const char * const*)skiplist); > > if (!filesystems[0]) > > - tst_brk(TCONF, "There are no supported filesystems"); > > + tst_brk(TCONF, "There are no supported filesystems or all skipped"); > > for (i = 0; filesystems[i]; i++) > > printf("%s\n", filesystems[i]); > > -- > > 2.37.3
> Hi! > > And use this feature in zram01.sh. > This looks like an leftover. Thanks! Kind regards, Petr
Hi! > Shouldn't there be an getopt option to decide? > e.g. by default both skip list and mkfs (no matter if -s is passed) and with > option (e.g. -o) check only for a list? This is not needed when checking all > filesystems, only for testing single filesystem, so I wonder if I should > implement it for all filesystems mode. > > But as this is not needed I'm ok to implement what you suggest: > tst_supported_fs -s skiplist foo would check only if the used filesystem is not > filtered by skip list (used in tst_test.sh). > > tst_supported_fs foo would check only for mkfs.foo (used in prepare_lvm.sh). > > What do you prefer? I guess that I do not care that much what exact API we do have here as long was we can check for just skiplist or just support separatelly.
> Hi! > > Shouldn't there be an getopt option to decide? > > e.g. by default both skip list and mkfs (no matter if -s is passed) and with > > option (e.g. -o) check only for a list? This is not needed when checking all > > filesystems, only for testing single filesystem, so I wonder if I should > > implement it for all filesystems mode. > > But as this is not needed I'm ok to implement what you suggest: > > tst_supported_fs -s skiplist foo would check only if the used filesystem is not > > filtered by skip list (used in tst_test.sh). > > tst_supported_fs foo would check only for mkfs.foo (used in prepare_lvm.sh). > > What do you prefer? > I guess that I do not care that much what exact API we do have here as > long was we can check for just skiplist or just support separatelly. OK, atm we don't need the third option (skip list + both), so that I will not care about it. I might do this only for mode when single fs is being queried (haven't decided yet) as that's IMHO not needed atm.
diff --git a/testcases/lib/tst_supported_fs.c b/testcases/lib/tst_supported_fs.c index 2b42d5bb3..e2261244d 100644 --- a/testcases/lib/tst_supported_fs.c +++ b/testcases/lib/tst_supported_fs.c @@ -80,14 +80,19 @@ int main(int argc, char *argv[]) return 2; } - if (optind < argc) - return !tst_fs_is_supported(argv[optind]); + if (optind < argc) { + if (tst_fs_in_skiplist(argv[optind], (const char * const*)skiplist)) + tst_brk(TCONF, "%s is not supported by the test", argv[optind]); + tst_res(TINFO, "%s is supported by the test", argv[optind]); + + return 0; + } filesystems = tst_get_supported_fs_types((const char * const*)skiplist); if (!filesystems[0]) - tst_brk(TCONF, "There are no supported filesystems"); + tst_brk(TCONF, "There are no supported filesystems or all skipped"); for (i = 0; filesystems[i]; i++) printf("%s\n", filesystems[i]);