Re: [LTP] [PATCH 1/1] fsconfig03: Skip on FUSE

[Wei Gao via ltp <ltp@lists.linux.it>]

On Wed, Mar 29, 2023 at 03:38:18PM +0200, Petr Vorel wrote:
> > On Tue, Mar 28, 2023 at 04:40:31PM +0200, Petr Vorel wrote:
> > > fsconfig03 is broken not only on vfat and ntfs, but also over FUSE:
> 
> > > tst_supported_fs_types.c:120: TINFO: FUSE does support exfat
> 
> > I have a question on function has_kernel_support.
> 
> > If has_kernel_support start check exfat file system, if exfat not exist then start
> > check fuse, i have no idea why we still need check fuse, i suppose directly
> > return "exfat not support" instead of "FUSE does support exfat".
> 
> Because some filesystems can be supported by both Linux kernel or by FUSE
> (userspace). IMHO only NTFS and exfat are supported by both. We first check
> kernel implementation, but if it's not supported (e.g. kernel configured to
> support particular filesystem, but kernel module not being installed),
> we try to check if FUSE does support that filesystem.
> 
My opinion is has_kernel_support need ONLY check exfat implementation in kernel, this
can better match the name of function. Use for example has_fuse_support return exfat-fuse
or ntfs-3g support.

> > > +++ b/testcases/kernel/syscalls/fsconfig/fsconfig03.c
> > > @@ -88,7 +88,7 @@ static struct tst_test test = {
> > >  	.mntpoint = MNTPOINT,
> > >  	.all_filesystems = 1,
> > >  	.taint_check = TST_TAINT_W | TST_TAINT_D,
> > > -	.skip_filesystems = (const char *const []){"ntfs", "vfat", NULL},
> > > +	.skip_filesystems = (const char *const []){"fuse", NULL},
> 
> > I feel you can not skip fuse system since i found following list in LTP:
> > static const char *const fs_type_whitelist[] = {
> >         "ext2",
> >         "ext3",
> >         "ext4",
> >         "xfs",
> >         "btrfs",
> >         "vfat",
> >         "exfat",
> >         "ntfs",
> >         "tmpfs",
> >         NULL
> > };
> 
> This array name is quite confusing, that I even once proposed to rename it :) [1].
> It's used for .all_filesystems = 1 (if you don't define .skip_filesystems, all
> filesystems defined in fs_type_whitelist will be running. Therefore only
> filesystems defined in it makes sense to whitelist.
> 

I prefer replace .all_filesystems to .def_filesystems_check if we keep current logic.

> But on tests without .all_filesystems = 1, any filesystem can be defined in
> .skip_filesystems (see testcases/kernel/syscalls/fcntl/fcntl33.c, it has "ramfs"
> and "nfs"). In this case filesystem in $TMPDIR is checked and if the same as any
> member in .skip_filesystems, test is being skipped (see the beginning of
> do_test_setup()). I put some effort to document it, but mainly due
> "ext2/ext3/ext4" (when .all_filesystems = 1, is *not defined*) vs. using these
> separately (e.g..skip_filesystems = (const char *const []){"ext2", "ext3", NULL} ).

I have some difficult to understand above logic.

> 
> Back to your question, fuse is somehow special, see functions in lib/safe_macros.c
> Also, note, we don't even use kernel's NTFS driver, see lib/safe_macros.c:
> 

I prefer using more clear word describe fuse or non-fuse filesystem for white list/skip_filesystems such as:
*exfat // means we check exfat kernel version
*exfat-fuse //fuse version, we can add this into current white list
*ntfs // check ntfs kernel version
*ntfs-3g //fuse userspace implemen, we can add this into current white list

> int safe_mount(const char *file, const int lineno, void (*cleanup_fn)(void),
> 	       const char *source, const char *target,
> 	       const char *filesystemtype, unsigned long mountflags,
> 	       const void *data)
> {
> 	int rval = -1;
> 
> 	/*
> 	 * Don't try using the kernel's NTFS driver when mounting NTFS, since
> 	 * the kernel's NTFS driver doesn't have proper write support.
> 	 */
> 	if (!filesystemtype || strcmp(filesystemtype, "ntfs")) {
> 		rval = mount(source, target, filesystemtype, mountflags, data);
> 		if (!rval)
> 			return 0;
> 	}
> 
> 	/*
> 	 * The FUSE filesystem executes mount.fuse helper, which tries to
> 	 * execute corresponding binary name which is encoded at the start of
> 	 * the source string and separated by # from the device name.
>          *
> 	 * The mount helpers are called mount.$fs_type.
> 	 */
> 	if (possibly_fuse(filesystemtype)) {
> 		char buf[1024];
> 
> 		tst_resm_(file, lineno, TINFO, "Trying FUSE...");
> 		snprintf(buf, sizeof(buf), "mount.%s '%s' '%s'",
> 			filesystemtype, source, target);
> 
> Kind regards,
> Petr
> 
> [1] https://lore.kernel.org/ltp/20220126141152.6428-1-pvorel@suse.cz/
> [2] https://github.com/linux-test-project/ltp/wiki/C-Test-API#113-filesystem-type-detection-and-skiplist
Thanks again for support so much detail backgroud information!

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