[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251107070820.6mil3ptmkkyauyts@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com>
Date: Fri, 7 Nov 2025 15:08:20 +0800
From: Zorro Lang <zlang@...hat.com>
To: "Darrick J. Wong" <djwong@...nel.org>,
Amir Goldstein <amir73il@...il.com>
Cc: neal@...pa.dev, fstests@...r.kernel.org, linux-ext4@...r.kernel.org,
linux-fsdevel@...r.kernel.org, joannelkoong@...il.com,
bernd@...ernd.com
Subject: Re: [PATCH 01/33] misc: adapt tests to handle the fuse ext[234]
drivers
On Wed, Nov 05, 2025 at 02:53:55PM -0800, Darrick J. Wong wrote:
> On Thu, Oct 30, 2025 at 10:51:06AM +0100, Amir Goldstein wrote:
> > On Wed, Oct 29, 2025 at 2:22 AM Darrick J. Wong <djwong@...nel.org> wrote:
> > >
> > > From: Darrick J. Wong <djwong@...nel.org>
> > >
> > > It would be useful to be able to run fstests against the userspace
> > > ext[234] driver program fuse2fs. A convention (at least on Debian)
> > > seems to be to install fuse drivers as /sbin/mount.fuse.XXX so that
> > > users can run "mount -t fuse.XXX" to start a fuse driver for a
> > > disk-based filesystem type XXX.
> > >
> > > Therefore, we'll adopt the practice of setting FSTYP=fuse.ext4 to
> > > test ext4 with fuse2fs. Change all the library code as needed to handle
> > > this new type alongside all the existing ext[234] checks, which seems a
> > > little cleaner than FSTYP=fuse FUSE_SUBTYPE=ext4, which also would
> > > require even more treewide cleanups to work properly because most
> > > fstests code switches on $FSTYP alone.
Thanks Darrick, a big patchset again :)
> > >
> >
> > I agree that FSTYP=fuse.ext4 is cleaner than
> > FSTYP=fuse FUSE_SUBTYPE=ext4
> > but it is not extendable to future (e.g. fuse.xfs)
> > and it is still a bit ugly.
> >
> > Consider:
> > FSTYP=fuse.ext4
> > MKFSTYP=ext4
No matter this ^^^, or ...
> >
> > I think this is the correct abstraction -
> > fuse2fs/ext4 are formatted that same and mounted differently
> >
> > See how some of your patch looks nicer and naturally extends to
> > the imaginary fuse.xfs...
>
> Maybe I'd rather do it the other way around for fuse4fs:
>
> FSTYP=ext4
> MOUNT_FSTYP=fuse.ext4
... this ^^^, I think this discussion brings in a topic:
If the same on-disk fstype can be mounted with different drivers, what does
original $FSTYP stand for? Is it stand for the running fs driver, or the the
on-disk fs type? Besides the code looks ugly or not, the basic parameters must
be clear in meaning I think.
If the same on-disk fstype can be mounted with different drivers, we might
need two parameters to avoid this confusion. Due to we sometimes depends on
the on-disk type, e.g. mkfs, fsck, xfs_db, metadump and so on, sometimes
depend on running-time fs driver, e.g. mount, quota and so on.
We can let FSTYP stands for the running-time fs driver, as it's mostly treated
as that I think. And set MKFSTYP=$FSTYP by default, if MKFSTYP isn't specified.
Then change those "FSTYP judgement" code to MKFSTYP, if they need ondisk fstype
actually.
I don't mind the parameter name, but the meaning should not be confused. What
do you think? Feel free to correct me if I miss anything.
Thanks,
Zorro
>
> (obviously, MOUNT_FSTYP=$FSTYP if the test runner hasn't overridden it)
>
> Where $MOUNT_FSTYP is what you pass to mount -t and what you'd see in
> /proc/mounts. The only weirdness with that is that some of the helpers
> will end up with code like:
>
> case $FSTYP in
> ext4)
> # do ext4 stuff
> ;;
> esac
>
> case $MOUNT_FSTYP in
> fuse.ext4)
> # do fuse4fs stuff that overrides ext4
> ;;
> esac
>
> which would be a little weird.
>
> _scratch_mount would end up with:
>
> $MOUNT_PROG -t $MOUNT_FSTYP ...
>
> and detecting it would be
>
> grep -q -w $MOUNT_FSTYP /proc/mounts || _fail "booooo"
>
> Hrm?
>
> --D
>
> >
> > > Signed-off-by: "Darrick J. Wong" <djwong@...nel.org>
> > > ---
> > > check | 24 +++++++++++++++++-------
> > > common/casefold | 4 ++++
> > > common/config | 11 ++++++++---
> > > common/defrag | 2 +-
> > > common/encrypt | 16 ++++++++--------
> > > common/log | 10 +++++-----
> > > common/populate | 14 +++++++-------
> > > common/quota | 9 +++++++++
> > > common/rc | 50 +++++++++++++++++++++++++++++---------------------
> > > common/report | 2 +-
> > > common/verity | 8 ++++----
> > > tests/generic/020 | 2 +-
> > > tests/generic/067 | 2 +-
> > > tests/generic/441 | 2 +-
> > > tests/generic/496 | 2 +-
> > > tests/generic/621 | 2 +-
> > > tests/generic/740 | 2 +-
> > > tests/generic/746 | 4 ++--
> > > tests/generic/765 | 4 ++--
> > > 19 files changed, 103 insertions(+), 67 deletions(-)
> > >
> > >
> > > diff --git a/check b/check
> > > index 9bb80a22440f97..81cd03f73ce155 100755
> > > --- a/check
> > > +++ b/check
> > > @@ -140,12 +140,25 @@ get_sub_group_list()
> > > echo $grpl
> > > }
> > >
> > > +get_group_dirs()
> > > +{
> > > + local fsgroup="$FSTYP"
> > > +
> > > + case "$FSTYP" in
> > > + ext2|ext3|fuse.ext[234])
> > > + fsgroup=ext4
> > > + ;;
> > > + esac
> > > +
> > > + echo $SRC_GROUPS
> > > + echo $fsgroup
> > > +}
> > > +
> > > get_group_list()
> > > {
> > > local grp=$1
> > > local grpl=""
> > > local sub=$(dirname $grp)
> > > - local fsgroup="$FSTYP"
> > >
> > > if [ -n "$sub" -a "$sub" != "." -a -d "$SRC_DIR/$sub" ]; then
> > > # group is given as <subdir>/<group> (e.g. xfs/quick)
> > > @@ -154,10 +167,7 @@ get_group_list()
> > > return
> > > fi
> > >
> > > - if [ "$FSTYP" = ext2 -o "$FSTYP" = ext3 ]; then
> > > - fsgroup=ext4
> > > - fi
> > > - for d in $SRC_GROUPS $fsgroup; do
> > > + for d in $(get_group_dirs); do
> > > if ! test -d "$SRC_DIR/$d" ; then
> > > continue
> > > fi
> > > @@ -171,7 +181,7 @@ get_group_list()
> > > get_all_tests()
> > > {
> > > touch $tmp.list
> > > - for d in $SRC_GROUPS $FSTYP; do
> > > + for d in $(get_group_dirs); do
> > > if ! test -d "$SRC_DIR/$d" ; then
> > > continue
> > > fi
> > > @@ -387,7 +397,7 @@ if [ -n "$FUZZ_REWRITE_DURATION" ]; then
> > > fi
> > >
> > > if [ -n "$subdir_xfile" ]; then
> > > - for d in $SRC_GROUPS $FSTYP; do
> > > + for d in $(get_group_dirs); do
> > > [ -f $SRC_DIR/$d/$subdir_xfile ] || continue
> > > for f in `sed "s/#.*$//" $SRC_DIR/$d/$subdir_xfile`; do
> > > exclude_tests+=($d/$f)
> > > diff --git a/common/casefold b/common/casefold
> > > index 2aae5e5e6c8925..fcdb4d210028ac 100644
> > > --- a/common/casefold
> > > +++ b/common/casefold
> > > @@ -6,6 +6,10 @@
> > > _has_casefold_kernel_support()
> > > {
> > > case $FSTYP in
> > > + fuse.ext[234])
> > > + # fuse2fs does not support casefolding
> > > + false
> > > + ;;
> >
> > This would not be needed
> >
> > > ext4)
> > > test -f '/sys/fs/ext4/features/casefold'
> > > ;;
> > > diff --git a/common/config b/common/config
> > > index 7fa97319d7d0ca..0cd2b33c4ade40 100644
> > > --- a/common/config
> > > +++ b/common/config
> > > @@ -386,6 +386,11 @@ _common_mount_opts()
> > > overlay)
> > > echo $OVERLAY_MOUNT_OPTIONS
> > > ;;
> > > + fuse.ext[234])
> > > + # fuse sets up secure defaults, so we must explicitly tell
> > > + # fuse2fs to use the more relaxed kernel access behaviors.
> > > + echo "-o kernel $EXT_MOUNT_OPTIONS"
> > > + ;;
> > > ext2|ext3|ext4)
> > > # acls & xattrs aren't turned on by default on ext$FOO
> > > echo "-o acl,user_xattr $EXT_MOUNT_OPTIONS"
> > > @@ -472,7 +477,7 @@ _mkfs_opts()
> > > _fsck_opts()
> > > {
> > > case $FSTYP in
> >
> > This would obviously be $MKFSTYP with no further changes
> >
> > > - ext2|ext3|ext4)
> > > + ext2|ext3|fuse.ext[234]|ext4)
> > > export FSCK_OPTIONS="-nf"
> > > ;;
> > > reiser*)
> > > @@ -514,11 +519,11 @@ _source_specific_fs()
> > >
> > > . ./common/btrfs
> > > ;;
> > > - ext4)
> > > + fuse.ext4|ext4)
> > > [ "$MKFS_EXT4_PROG" = "" ] && _fatal "mkfs.ext4 not found"
> > > . ./common/ext4
> > > ;;
> > > - ext2|ext3)
> > > + ext2|ext3|fuse.ext[23])
> > > . ./common/ext4
> >
> > same here
> >
> > > ;;
> > > f2fs)
> > > diff --git a/common/defrag b/common/defrag
> > > index 055d0d0e9182c5..c054e62bde6f4d 100644
> > > --- a/common/defrag
> > > +++ b/common/defrag
> > > @@ -12,7 +12,7 @@ _require_defrag()
> > > _require_xfs_io_command "falloc"
> > > DEFRAG_PROG="$XFS_FSR_PROG"
> > > ;;
> > > - ext4)
> > > + fuse.ext4|ext4)
> > > testfile="$TEST_DIR/$$-test.defrag"
> > > donorfile="$TEST_DIR/$$-donor.defrag"
> > > bsize=`_get_block_size $TEST_DIR`
> >
> > and here
> >
> > > diff --git a/common/encrypt b/common/encrypt
> > > index f2687631b214cf..4fa7b6853fd461 100644
> > > --- a/common/encrypt
> > > +++ b/common/encrypt
> > > @@ -191,7 +191,7 @@ _require_hw_wrapped_key_support()
> > > _scratch_mkfs_encrypted()
> > > {
> > > case $FSTYP in
> > > - ext4|f2fs)
> > > + fuse.ext4|ext4|f2fs)
> > > _scratch_mkfs -O encrypt
> > > ;;
> >
> > and here
> >
> > > ubifs)
> > > @@ -210,7 +210,7 @@ _scratch_mkfs_encrypted()
> > > _scratch_mkfs_sized_encrypted()
> > > {
> > > case $FSTYP in
> > > - ext4|f2fs)
> > > + fuse.ext4|ext4|f2fs)
> > > MKFS_OPTIONS="$MKFS_OPTIONS -O encrypt" _scratch_mkfs_sized $*
> > > ;;
> >
> > and here... I think you got my point.
> >
> > Thanks,
> > Amir.
> >
>
Powered by blists - more mailing lists