Re: [PATCH v12 2/6] generic/632: add fstests for idmapped mounts

["Theodore Ts'o" <tytso@mit.edu>]

On Mon, Apr 12, 2021 at 01:54:26PM +0200, Christian Brauner wrote:
>So we can detect it pretty reliably at runtime by trying whether we can
>create an idmapped mount on the given filesystem. That is enough for the
>idmapped mount tests here but of course has at least two drawbacks:
>1. there might be scenarios where we get false negatives
>   (e.g. open_tree() could fail for a lack of permissions or sm else,
>   kernel might be compiled without userns support etc. pp)

In the ideal world, if the kernel wasn't compiled with the necessary
CONFIG options enabled, it's desirable of the test can detect that
fact and skip running the test instead failing and forcing the person
running the test to try to figure out whether this is a legitmate file
system bug or a just a test setup bug.

This is one of the other functions of /sys/fs/ext4/features/...  for
example:

#ifdef CONFIG_UNICODE
EXT4_ATTR_FEATURE(casefold);
#endif

That way, if a particular kernel CONFIG is not compiled in, we can
prevent the feature from being advertised, without having to rely on
/proc/config.gz be present.

>2. it's heavy in so far as we have to do the whole exercise of creating
>   a detached mount

Yeah, there are times when I've added a new feature flag file in
/sys/fs/ext4/features because it was simpler than being able to doing
some kind of heavyweight test.

> So having a reliable way to detect whether or not the underlying fs
> supports it could be worth it (My hope was for the fsinfo() API to grow
> this "feature check" ability but oh well.).
> 
> One possibility might be to extend fstatfs() and steal one u32 from the
> padding that is currently in there?

That would require mounting a file system in order to do the check,
which is not necesarily always convenient, but that would certainly
work.  (See below for an example of where it was much easier for
mke2fs to be able to test for a file system "capability" without
needing to mount a test file system.)

> > If you can't do this by checking to see if the file system will
> > support a particular mount option, or some other run-time test, for
> > ext4 we can signal this by checking for the existence of a file in
> > /sys/fs/ext4/features, such as /sys/fs/ext4/features/fast_commit.
> > (Grep for EXT4_ATTR_FEATURE and ATTR_LIST in fs/ext4/sysfs.c; it
> > requires adding two lines to advertise a new ext4 feature.)
> 
> I wonder if this wouldn't be nice to have independent of whether or not
> there is another way to detect it?
> I'm would think that people like to see all new ext4 features listed in
> there. Even if this is technically a generic vfs feature.

My apologies for the confusion.  There's a bit of overloading in terms
of the word "features" as used above.  In some cases, there is a
direct correspondance between bits that show up in the compat,
ro_compat, and incompat fields in the ext4 superblock.  In other cases
it might describe a file system behaviour (e.g., lazy_itable_init) and
this acts as a hint to e2fsprogs about what kind of defaults it should
use when creating a file system --- or to warn the user about whether
a particular file system can be mounted using the current kernel.

For example, currently we give the following warning if you create a
file system with a 64k block size:

% mke2fs -t ext4 -b 65536 /tmp/foo.img 8M
Warning: blocksize 65536 not usable on most systems.
mke2fs 1.46.2 (28-Feb-2021)
Creating regular file /tmp/foo.img
mke2fs: 65536-byte blocks too big for system (max 4096)
Proceed anyway? (y,N)

However, suppose at some point, thanks to advances in the MM layer, it
becomes possible to support file systems where the block size is
greater than the page size.  We might then advertise that via the
existing of a file such as /sys/fs/ext4/feature/big_blocks (for
example), and then newer versions of mke2fs would check for that file
and suppress the warning on a kernel which would actually allow file
systems with 64k blocks to be mounted on an x86_64 system.  This is
also why I designed it via using a file in /sys/fs/ext4/features, so
that I don't have to do a test mount and use a system call fstatfs(2)
to check for the existenace of an ext4 feature/capability (all of the
obvious terms are already overloaded, sigh).

I agree that having some kind of fs-independent way for querying
whether some feature/capability is present for a particular kernel
would be convenient.  This wouldn't necessarily need to be implemented
in the VFS, though, especially if the convention that gets adopted is
via a file in /sys/fs/<fstype>/features/<capability>.

						- Ted