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

[Cyril Hrubis <chrubis@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.

-- 
Cyril Hrubis
chrubis@suse.cz

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