All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bhumit Attarde <bhumitattarde01@gmail.com>
To: Eryu Guan <guan@eryu.me>
Cc: fstests@vger.kernel.org
Subject: Re: [PATCH 1/1] Added FUSE support
Date: Sun, 19 Sep 2021 12:47:48 +0530	[thread overview]
Message-ID: <CANb_4Ap9FUjLGxTQjtBn5LNdEzScV63tC-0G-=_3K0QO6pj+9A@mail.gmail.com> (raw)
In-Reply-To: <YRkg5GrK0XnD1fZZ@desktop>

Apologies for the delayed response, too many things going on.

> I think we could set different 'dev' for TEST_DEV and SCRATCH_DEV, e.g.
>
> TEST_DEV=fuse_test
> SCRATCH_DEV=fuse_scratch
>
> And the "This check doesn't apply to FUSE since both mounts have the
> same device" check could be removed.

We did actually try setting different `dev` for TEST_DEV and
SCRATCH_DEV, but it doesn't work for certain FUSE applications like
KIO-FUSE, which is also the reason FUSE check is added (XFSTESTS
complaints that the device is already mounted and aborts). An
application like KIO-FUSE has the same dev name (`kio-fuse`) for all
FUSE mounts and deliberately setting different dev names not only
doesn't work at all but, in my opinion, also feels a little hacky.

> Set MOUNT_OPTIONS should be fine here.
>
> And we should set "subtype" in options as well, i.e.
>
> MOUNT_OPTIONS="-o source=/home/test/scratch,allow_other, default_permissions,subtype=passthrough_ll"
>
> Otherwise the filesystem type printed by mount of a passthrough_ll mount
> is only "fuse" and doesn't contain the subtype, and _check_mounted_on()
> will fail the check.
>
> Should do the same to _test_mount().

Will make these changes.

> Update DF_PROG definition instead, so all callers of $DF_PROG would
> print fuse mounts correctly.

This for some reason is not producing the same results for us, will
look into this more.

Best,
Bhumit Attarde


On Sun, 15 Aug 2021 at 19:43, Eryu Guan <guan@eryu.me> wrote:
>
> On Thu, Aug 12, 2021 at 10:29:50AM +0530, Bhumit Attarde wrote:
> > This allows any FUSE filesystem that can mounted like
> > `mount -t fuse.$FUSE_SUBTYP`
> > to be tested using XFSTESTS.
> >
> > This patch is an improvment on the patch originally submitted by Miklos
> > Szeredi.
> >
> > Signed-off-by: Bhumit Attarde <bhumit.attarde01@gmail.com>
> > ---
> >  README.fuse   | 29 +++++++++++++++++++++++++++++
> >  check         |  2 ++
> >  common/attr   |  4 ++--
> >  common/config | 10 ++++++++--
> >  common/rc     | 31 +++++++++++++++++++++++--------
> >  5 files changed, 64 insertions(+), 12 deletions(-)
> >  create mode 100644 README.fuse
> >
> > diff --git a/README.fuse b/README.fuse
> > new file mode 100644
> > index 00000000..839db5dd
> > --- /dev/null
> > +++ b/README.fuse
> > @@ -0,0 +1,29 @@
> > +Make sure you've followed instructions from the principal README.
> > +Instructions to test FUSE using the passthrough_ll example filesystem provided in the libfuse source tree:
> > +
> > +git clone git://github.com/libfuse/libfuse.git
> > +cd libfuse
> > +meson build
> > +cd build
> > +ninja
> > +cp example/passthrough_ll /usr/bin
> > +cd
> > +cat << 'EOF' > /sbin/mount.fuse.passthrough_ll
> > +#!/bin/bash
> > +ulimit -n 1048576
> > +exec /usr/bin/passthrough_ll -ofsname="$@"
> > +EOF
> > +chmod +x /sbin/mount.fuse.passthrough_ll
> > +mkdir -p /mnt/test /mnt/scratch /home/test/test /home/test/scratch
> > +
> > +
> > +Use the following config file:
> > +
> > +export TEST_DEV=passthrough_ll
> > +export TEST_DIR=/mnt/test
> > +export SCRATCH_DEV=passthrough_ll
>
> I think we could set different 'dev' for TEST_DEV and SCRATCH_DEV, e.g.
>
> TEST_DEV=fuse_test
> SCRATCH_DEV=fuse_scratch
>
> And the "This check doesn't apply to FUSE since both mounts have the
> same device" check could be removed.
>
> > +export SCRATCH_MNT=/mnt/scratch
> > +export FSTYP=fuse
> > +export FUSE_SUBTYP=.passthrough_ll
> > +export FUSE_MOUNT_OPTIONS="-osource=/home/test/scratch,allow_other,default_permissions"
>
> Set MOUNT_OPTIONS should be fine here.
>
> And we should set "subtype" in options as well, i.e.
>
> MOUNT_OPTIONS="-o source=/home/test/scratch,allow_other, default_permissions,subtype=passthrough_ll"
>
> Otherwise the filesystem type printed by mount of a passthrough_ll mount
> is only "fuse" and doesn't contain the subtype, and _check_mounted_on()
> will fail the check.
>
> > +export TEST_FS_MOUNT_OPTS="-osource=/home/test/test,allow_other,default_permissions"
> > diff --git a/check b/check
> > index a0863121..bae9fddd 100755
> > --- a/check
> > +++ b/check
> > @@ -59,6 +59,7 @@ check options
> >      -glusterfs               test GlusterFS
> >      -cifs            test CIFS
> >      -9p                      test 9p
> > +    -fuse            test fuse
> >      -virtiofs                test virtiofs
> >      -overlay         test overlay
> >      -pvfs2           test PVFS2
> > @@ -281,6 +282,7 @@ while [ $# -gt 0 ]; do
> >       -glusterfs)     FSTYP=glusterfs ;;
> >       -cifs)          FSTYP=cifs ;;
> >       -9p)            FSTYP=9p ;;
> > +     -fuse)          FSTYP=fuse ;;
> >       -virtiofs)      FSTYP=virtiofs ;;
> >       -overlay)       FSTYP=overlay; export OVERLAY=true ;;
> >       -pvfs2)         FSTYP=pvfs2 ;;
> > diff --git a/common/attr b/common/attr
> > index 35682d7c..f3342d25 100644
> > --- a/common/attr
> > +++ b/common/attr
> > @@ -253,7 +253,7 @@ _getfattr()
> >
> >  # set maximum total attr space based on fs type
> >  case "$FSTYP" in
> > -xfs|udf|pvfs2|9p|ceph|nfs)
> > +xfs|udf|pvfs2|9p|ceph|nfs|fuse)
> >       MAX_ATTRS=1000
> >       ;;
> >  ext2|ext3|ext4)
> > @@ -312,7 +312,7 @@ xfs|udf|btrfs)
> >  pvfs2)
> >       MAX_ATTRVAL_SIZE=8192
> >       ;;
> > -9p|ceph|nfs)
> > +9p|ceph|nfs|fuse)
> >       MAX_ATTRVAL_SIZE=65536
> >       ;;
> >  bcachefs)
> > diff --git a/common/config b/common/config
> > index 005fd50a..8203492d 100644
> > --- a/common/config
> > +++ b/common/config
> > @@ -305,6 +305,9 @@ _mount_opts()
> >       9p)
> >               export MOUNT_OPTIONS=$PLAN9_MOUNT_OPTIONS
> >               ;;
> > +     fuse)
> > +             export MOUNT_OPTIONS=$FUSE_MOUNT_OPTIONS
> > +             ;;
> >       xfs)
> >               export MOUNT_OPTIONS=$XFS_MOUNT_OPTIONS
> >               ;;
> > @@ -363,6 +366,9 @@ _test_mount_opts()
> >       9p)
> >               export TEST_FS_MOUNT_OPTS=$PLAN9_MOUNT_OPTIONS
> >               ;;
> > +     fuse)
> > +             export TEST_FS_MOUNT_OPTS=$FUSE_MOUNT_OPTIONS
> > +             ;;
> >       cifs)
> >               export TEST_FS_MOUNT_OPTS=$CIFS_MOUNT_OPTIONS
> >               ;;
> > @@ -501,8 +507,8 @@ _check_device()
> >       fi
> >
> >       case "$FSTYP" in
> > -     9p|tmpfs|virtiofs)
> > -             # 9p and virtiofs mount tags are just plain strings, so anything is allowed
> > +     9p|fuse|tmpfs|virtiofs)
> > +             # 9p, fuse and virtiofs mount tags are just plain strings, so anything is allowed
> >               # tmpfs doesn't use mount source, ignore
> >               ;;
> >       overlay)
> > diff --git a/common/rc b/common/rc
> > index d4b1f21f..1ae0746a 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -127,6 +127,8 @@ case "$FSTYP" in
> >        ;;
> >      9p)
> >        ;;
> > +    fuse)
> > +      ;;
> >      ceph)
> >        . ./common/ceph
> >        ;;
> > @@ -333,7 +335,7 @@ _try_scratch_mount()
> >               _overlay_scratch_mount $*
> >               return $?
> >       fi
> > -     _mount -t $FSTYP `_scratch_mount_options $*`
> > +     _mount -t $FSTYP$FUSE_SUBTYP `_scratch_mount_options $*`
>
> Should do the same to _test_mount().
>
> >  }
> >
> >  # mount scratch device with given options and _fail if mount fails
> > @@ -643,6 +645,9 @@ _test_mkfs()
> >      9p)
> >       # do nothing for 9p
> >       ;;
> > +    fuse)
> > +     # do nothing for fuse
> > +     ;;
> >      virtiofs)
> >       # do nothing for virtiofs
> >       ;;
> > @@ -683,6 +688,9 @@ _mkfs_dev()
> >      9p)
> >       # do nothing for 9p
> >       ;;
> > +    fuse)
> > +     # do nothing for fuse
> > +     ;;
> >      virtiofs)
> >       # do nothing for virtiofs
> >       ;;
> > @@ -750,7 +758,7 @@ _scratch_mkfs()
> >       local mkfs_status
> >
> >       case $FSTYP in
> > -     nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p|virtiofs)
> > +     nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p|fuse|virtiofs)
> >               # unable to re-create this fstyp, just remove all files in
> >               # $SCRATCH_MNT to avoid EEXIST caused by the leftover files
> >               # created in previous runs
> > @@ -1271,7 +1279,7 @@ _df_device()
> >
> >      # Note that we use "==" here so awk doesn't try to interpret an NFS over
> >      # IPv6 server as a regular expression.
> > -    $DF_PROG 2>/dev/null | $AWK_PROG -v what=$1 '
> > +    $DF_PROG -a 2>/dev/null | $AWK_PROG -v what=$1 '
>
> Update DF_PROG definition instead, so all callers of $DF_PROG would
> print fuse mounts correctly.
>
> >          ($1==what) && (NF==1) {
> >              v=$1
> >              getline
> > @@ -1514,14 +1522,15 @@ _check_mounted_on()
> >       [ -n "$mount_rec" ] || return 1 # 1 = not mounted
> >
> >       # if it's mounted, make sure its on $mnt
> > -     if [ "$mount_rec" != "$dev $mnt" ]; then
> > +     # This check doesn't apply to FUSE since both mounts have the same device
> > +     if [[ "$FSTYP" != "fuse" && "$mount_rec" != "$dev $mnt" ]]; then
>
> As stated above, I don't think this check is needed. Any reason to do so?
>
> Thanks,
> Eryu
>
> >               echo "$devname=$dev is mounted but not on $mntname=$mnt - aborting"
> >               echo "Already mounted result:"
> >               echo $mount_rec
> >               return 2 # 2 = mounted on wrong mnt
> >       fi
> >
> > -     if [ -n "$type" -a "`_fs_type $dev`" != "$type" ]; then
> > +     if [ -n "$type" -a "`_fs_type $dev`" != "$type$FUSE_SUBTYP" ]; then
> >               echo "$devname=$dev is mounted but not a type $type filesystem"
> >               # raw $DF_PROG cannot handle NFS/CIFS/overlay correctly
> >               _df_device $dev
> > @@ -1545,7 +1554,7 @@ _require_scratch_nocheck()
> >                       _notrun "this test requires a valid \$SCRATCH_MNT"
> >               fi
> >               ;;
> > -     9p|virtiofs)
> > +     9p|fuse|virtiofs)
> >               if [ -z "$SCRATCH_DEV" ]; then
> >                       _notrun "this test requires a valid \$SCRATCH_DEV"
> >               fi
> > @@ -1701,7 +1710,7 @@ _require_test()
> >                       _notrun "this test requires a valid \$TEST_DIR"
> >               fi
> >               ;;
> > -     9p|virtiofs)
> > +     9p|fuse|virtiofs)
> >               if [ -z "$TEST_DEV" ]; then
> >                       _notrun "this test requires a valid \$TEST_DEV"
> >               fi
> > @@ -2863,7 +2872,7 @@ _mount_or_remount_rw()
> >
> >       if [ $USE_REMOUNT -eq 0 ]; then
> >               if [ "$FSTYP" != "overlay" ]; then
> > -                     _mount -t $FSTYP $mount_opts $device $mountpoint
> > +                     _mount -t $FSTYP$FUSE_SUBTYP $mount_opts $device $mountpoint
> >               else
> >                       _overlay_mount $device $mountpoint
> >               fi
> > @@ -3000,6 +3009,9 @@ _check_test_fs()
> >      9p)
> >       # no way to check consistency for 9p
> >       ;;
> > +    fuse)
> > +     # no way to check consistency for fuse
> > +     ;;
> >      virtiofs)
> >       # no way to check consistency for virtiofs
> >       ;;
> > @@ -3061,6 +3073,9 @@ _check_scratch_fs()
> >      9p)
> >       # no way to check consistency for 9p
> >       ;;
> > +    fuse)
> > +     # no way to check consistency for fuse
> > +     ;;
> >      virtiofs)
> >       # no way to check consistency for virtiofs
> >       ;;
> > --
> > 2.30.1

      reply	other threads:[~2021-09-19  7:18 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-12  4:59 [PATCH 1/1] Added FUSE support Bhumit Attarde
2021-08-15 14:12 ` Eryu Guan
2021-09-19  7:17   ` Bhumit Attarde [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANb_4Ap9FUjLGxTQjtBn5LNdEzScV63tC-0G-=_3K0QO6pj+9A@mail.gmail.com' \
    --to=bhumitattarde01@gmail.com \
    --cc=fstests@vger.kernel.org \
    --cc=guan@eryu.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.