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

[Petr Vorel <pvorel@suse.cz>]

> Hi!
> > > > +++ 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,

> Maybe just TST_FS_SKIPPED

> >         TST_FS_KERNEL = 1,
> >         TST_FS_FUSE = 2,
> >  };

> I wonder though if we really care about the difference between
> unsupported and skipped. Maybe just print TINFO message in case of the
> skipped filesystem and return zero in both cases? And maybe rename
> TST_FS_UNSUPPORTED to TST_FSNOT_USED.

I thought being precise why there is TCONF would help, but users will just read
previous TINFO messages.

Having just single "error" enum constant, I'm OK to keep TST_FS_UNSUPPORTED
(unless anybody really think renaming it to TST_FS_NOT_USED would be better).

So, there can be just single message "foo is not supported or skipped by
test" (in case of asking for particular fs support) and "no supported
filesystem" (in case of list of all supported fs).
I'd use tst_brk(TCONF, ...) here, because the binary exits 1 on "error", thus
here would be exit 32 on "error".

Thanks for your input!

Kind regards,
Petr

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