Re: [LTP] [PATCH v4 4/9] tst_supported_fs: Support skip list when query single fs

[Petr Vorel <pvorel@suse.cz>]

Hi all,

> > +++ b/lib/tst_supported_fs_types.c
> > @@ -134,6 +134,24 @@ enum tst_fs_impl tst_fs_is_supported(const char *fs_type)
> >  	return TST_FS_UNSUPPORTED;
> >  }

> > +enum tst_fs_impl tst_fs_is_supported_skiplist(const char *fs_type, const char
> > +					      *const *skiplist)
> > +{
> > +	int ret;
> > +
> > +	ret = tst_fs_is_supported(fs_type);
> > +
> > +	if (!ret)
> > +		return ret;
> > +
> > +	if (tst_fs_in_skiplist(fs_type, skiplist)) {
> > +		tst_brk(TCONF, "%s is not supported by the test",
> > +			fs_type);

> I would expect to return TST_FS_UNSUPPORTED here. It's a bit hacky
> solution to exit a library call with tst_brk(TCONF, "");
Understand. There are 2 cases of not supported filesystems: 1) filesystem not
supported 2) filesystem skipped. Therefore I'd like to introduce
TST_FS_SKIPPED_BY_TEST (that would be 1 (therefore exit code
will have to be tested against the constants. That's IMHO better than
reinventing a wheel to detect this in testcases/lib/tst_supported_fs.c.

e.g.:

+++ b/include/tst_fs.h
@@ -171,7 +171,8 @@ int tst_fill_file(const char *path, char pattern, size_t bs, size_t bcount);
 int tst_prealloc_file(const char *path, size_t bs, size_t bcount);
 
 enum tst_fs_impl {
-       TST_FS_UNSUPPORTED = 0,
+       TST_FS_UNSUPPORTED = -2,
+       TST_FS_SKIPPED_BY_TEST = -1,
        TST_FS_KERNEL = 1,
        TST_FS_FUSE = 2,
 };

I also wonder if testcases/lib/tst_supported_fs.c should print TCONF in case of
error or just exit 0 or 1 (that's done now).

check for all filesystems (tst_supported_fs without params) is run in:
* zram01.sh - it handles no suitable filesystem, but the check could be based
  on tst_supported_fs return code
* generate_lvm_runfile.sh - does handle no suitable filesystem, but with
  'set -e' and trap - adding TCONF info would make things more clear
* prepare_lvm.sh (in prepare_lvm()) - does *not* handle no suitable filesystem.
  Although it's unlikely there is no supported filesystem it'd be better to
  print the message (it'd be more clear when testing with $LTP_SINGLE_FS_TYPE)

check for particular filesystem (tst_supported_fs foo) is run in:
* prepare_lvm.sh (in create_volume) - it check for unsupported filesystem and
  just quietly skip creating LVM. IMHO have here TCONF message would not harm.

=> Unless anybody objects I'll add TCONF messages in v5.

Kind regards,
Petr

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