Re: [LTP] [PATCH] tst_supported_fs: Implement checking paths against skiplist

[Petr Vorel <pvorel@suse.cz>]

> On Thu, Sep 22, 2022 at 5:03 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi all,

> > > > Although the tst_fs_type_name() functions could use some improvements,
> > > > e.g. ext4 must be specified in skiplist as "ext2/ext3/ext4" to get
> > properly

> > > Yes, that's true, we need to make it keep consistent with two
> > > skipping ways. Otherwise below test output "ext2/ext3/ext4"
> > > looks like a bit mess to remember.

> > > Better going with a single FS for matching (i.e. "ext4" for both fs_type
> > > and '-d path').
> > > But we can solve this in a separate patch later.

> > > $ df -T . | tail -1 | awk '{print $2}'
> > > ext4

> > > $ ./tst_supported_fs -s "ext4" ext4
> > > tst_supported_fs.c:135: TCONF: ext4 is skipped
> > > $ echo $?
> > > 32

> > > $ ./tst_supported_fs -s "ext4" -d .
> > > tst_supported_fs.c:137: TINFO: ext2/ext3/ext4 is not skipped
> > > $ echo $?
> > > 0

> > > $ ./tst_supported_fs -s "ext2/ext3/ext4" -d .
> > > tst_supported_fs.c:135: TCONF: ext2/ext3/ext4 is skipped
> > > $ echo $?
> > > 32

> > The same problem is for .skip_filesystems on tests which does not use
> > .all_filesystems. We only haven't noticed, because there was no reason to
> > skip
> > ext[234] so far. I'm looking into this.


> This seems a bit tricky to distinguish EXT2,3,4, from what I know,
> they use the same magic number. It will be difficult to get the
> FS block and extract smaller granularity of feature without
> using fs helper tools.

Yep, magic is the same: 0xef53 (not count the old ext2: 0xef51).
I thought that I'd map ext[235] to "ext2/ext3/ext4".
But maybe it's not a good idea.

OTOH df (coreutils) manages to distinguish them, stat can't.
I looked into df sources, it looks read_file_system_list() from gnulib which
it uses, reads /proc/self/mountinfo. Wouldn't be better to follow this approach
in tst_fs_type_() instead of using statfs()? (using st_dev - major:minor from
stat to match). I guess we always read mounted filesystem, thus it'd be a
drop-in replacement, we'd not need tst_fs_type_name() any more.

Kind regards,
Petr


> > BTW .skip_filesystems without .all_filesystems have other problems, e.g.
> > using
> > filesystems which aren't in fs_type_whitelist[] array (e.g. ramfs, nfs).
> > That'd be nice to fix after the release.


> +1

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