Re: [LTP] [PATCH] lib: LTP_SINGLE_FS_TYPE fix fuse blacklist

[Petr Vorel <pvorel@suse.cz>]

> Hi!
> > > The case of LTP_SINGLE_FS_TYPE did not proplerly used the fuse
> > > filesystem blacklist and we enabled fuse filesystem even for tests that
> > > explicitly stated in the tst_test structure that they do not work with
> > > fuse.

> > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> > > CC: Jan Polensky <japo@linux.ibm.com>
> > > ---
> > >  lib/tst_supported_fs_types.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)

> > > diff --git a/lib/tst_supported_fs_types.c b/lib/tst_supported_fs_types.c
> > > index bbbb8df19..3a394283c 100644
> > > --- a/lib/tst_supported_fs_types.c
> > > +++ b/lib/tst_supported_fs_types.c
> > > @@ -158,8 +158,10 @@ const char **tst_get_supported_fs_types(const char *const *skiplist)
> > >  	only_fs = getenv("LTP_SINGLE_FS_TYPE");

> > >  	if (only_fs) {
> > > +		enum tst_fs_impl ret;
> > >  		tst_res(TINFO, "WARNING: testing only %s", only_fs);
> > > -		if (tst_fs_is_supported(only_fs))
> > > +		ret = tst_fs_is_supported(only_fs);
> > > +		if (ret == TST_FS_KERNEL || (ret == TST_FS_FUSE && !skip_fuse))
> > >  			fs_types[0] = only_fs;

> > Is it really good thing to do?
> > Because LTP_SINGLE_FS_TYPE forces whatever filesystems, it ignores
> > .skip_filesystems. E.g. creat09.c don't want to be tested on ntfs (and others
> > but we can force it):

> Ah, right, we have to apply the skiplist as well, so on the top of that
> we need, I will send a v2.

> diff --git a/lib/tst_supported_fs_types.c b/lib/tst_supported_fs_types.c
> index 3a394283c..736e68dec 100644
> --- a/lib/tst_supported_fs_types.c
> +++ b/lib/tst_supported_fs_types.c
> @@ -159,10 +159,16 @@ const char **tst_get_supported_fs_types(const char *const *skiplist)

>         if (only_fs) {
>                 enum tst_fs_impl ret;
> +
>                 tst_res(TINFO, "WARNING: testing only %s", only_fs);
> +
> +               if (tst_fs_in_skiplist(only_fs, skiplist))
> +                       return fs_types;
> +
>                 ret = tst_fs_is_supported(only_fs);
>                 if (ret == TST_FS_KERNEL || (ret == TST_FS_FUSE && !skip_fuse))
>                         fs_types[0] = only_fs;
> +
>                 return fs_types;
>         }

+1 for this.


> > Could we clearly define what we expect from LTP_SINGLE_FS_TYPE?
> > FYI I put links to the discussion in the ticket
> > https://github.com/linux-test-project/ltp/issues/1242

> > I understand the wish to use LTP_SINGLE_FS_TYPE for a proper testing.  If that
> > requires to respect .skip_filesystems, but
> > 1) it should be done for all filesystems
> > 2) we should have another variable to allow to force the user defined
> > filesystem, e.g.
> > LTP_SINGLE_FS_TYPE=ntfs LTP_SINGLE_FS_TYPE_FORCE_FS=1

> What about LTP_FORCE_SINGLE_FS_TYPE=ntfs that would do the same as
> LTP_SINGLE_FS_TYPE=ntfs but would disable the skiplist?

+1, it should go to the release.

But please update also doc doc/users/setup_tests.rst + -h help in lib/tst_test.c
and testcases/lib/tst_test.sh (variable will of course affect both).

Kind regards,
Petr

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