Re: [PATCH v2 0/5] fstests: overlay: initial support for aufs and

Re: [PATCH v2 0/5] fstests: overlay: initial support for aufs and

[Amir Goldstein]

+CC: <linux-unionfs@vger.kernel.org>

On Fri, Feb 14, 2020 at 5:18 PM Mauricio Faria de Oliveira
<mfo@canonical.com> wrote:
>
> This patchset allows the existing support for overlay to be used with
> aufs and fuse-overlayfs, so the increase the coverage/test tools that
> are available for these filesystems.
>
> Initial numbers on v5.4-based Ubuntu kernel on Ubuntu Eoan/19.10
> (fuse-overlay installed from distro package), few tests excluded:
>
>  OVL_FSTYP=aufs
>  - Ran: 645 tests
>  - Not run: 483 tests
>  - Failures: 22 tests
>
>  OVL_FSTYP=fuse.fuse-overlayfs
>  - Ran: 530
>  - Not run: 395
>  - Failures: 29
>

It'd be interesting to know the baseline - what are those numbers for
OVL_FSTYP=overlay with same kernel?

Thanks,
Amir.

> Thanks to Amir Goldstein for review/improvements/suggestions.
>
> Changes:
>  - v2:
>    - fix tests/overlay that hardcode the overlay fs type
>    - add support to fuse-overlayfs with +3 other patches
>  - v1:
>    - [PATCH] common/overlay,rc: introduce OVL_ALT_FSTYP for testing aufs
>
> Mauricio Faria de Oliveira (5):
>   common/overlay,rc,config: introduce OVL_FSTYP variable and aufs
>   tests/overlay: mount: replace overlay hardcode with OVL_FSTYP variable
>   common/rc: introduce new helper function _fs_type_dev_dir()
>   common/rc: add quirks for fuse-overlayfs device/mount point
>   common/overlay: silence some mount messages for fuse-overlayfs
>
>  README.overlay    |  5 ++++
>  common/config     |  2 ++
>  common/overlay    | 29 +++++++++++++++++++---
>  common/rc         | 61 ++++++++++++++++++++++++++++++++++++++++-------
>  tests/overlay/011 |  2 +-
>  tests/overlay/035 |  2 +-
>  tests/overlay/052 |  4 ++--
>  tests/overlay/053 |  4 ++--
>  tests/overlay/062 |  2 +-
>  9 files changed, 92 insertions(+), 19 deletions(-)
>
> --
> 2.20.1
>

Re: [PATCH v2 0/5] fstests: overlay: initial support for aufs and

[Mauricio Faria de Oliveira]

On Fri, Feb 14, 2020 at 3:45 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> +CC: <linux-unionfs@vger.kernel.org>
>
> On Fri, Feb 14, 2020 at 5:18 PM Mauricio Faria de Oliveira
> <mfo@canonical.com> wrote:
> >
> > This patchset allows the existing support for overlay to be used with
> > aufs and fuse-overlayfs, so the increase the coverage/test tools that
> > are available for these filesystems.
> >
> > Initial numbers on v5.4-based Ubuntu kernel on Ubuntu Eoan/19.10
> > (fuse-overlay installed from distro package), few tests excluded:
> >
> >  OVL_FSTYP=aufs
> >  - Ran: 645 tests
> >  - Not run: 483 tests
> >  - Failures: 22 tests
> >
> >  OVL_FSTYP=fuse.fuse-overlayfs
> >  - Ran: 530
> >  - Not run: 395
> >  - Failures: 29
> >
>
> It'd be interesting to know the baseline - what are those numbers for
> OVL_FSTYP=overlay with same kernel?
>

Oh, indeed.
Here's the numbers for the 3 filesystems again (re-runs as I noticed
some tests skipped in fuse-overlayfs previously.)

OVL_FSTYP=overlay (unset/default)
- Ran: 654
- Not run: 391
- Failures: 5

OVL_FSTYP=fuse.fuse-overlayfs
- Ran: 653
- Not run: 472
- Failures: 54

OVL_FSTYP=aufs
- Ran: 645
- Not run: 483
- Failures: 22

BTW, thanks for reviewing/commenting on the individual patches.

I'll review them in more detail, and think I can send out a v3 next week.

cheers,
Mauricio


> Thanks,
> Amir.
>
> > Thanks to Amir Goldstein for review/improvements/suggestions.
> >
> > Changes:
> >  - v2:
> >    - fix tests/overlay that hardcode the overlay fs type
> >    - add support to fuse-overlayfs with +3 other patches
> >  - v1:
> >    - [PATCH] common/overlay,rc: introduce OVL_ALT_FSTYP for testing aufs
> >
> > Mauricio Faria de Oliveira (5):
> >   common/overlay,rc,config: introduce OVL_FSTYP variable and aufs
> >   tests/overlay: mount: replace overlay hardcode with OVL_FSTYP variable
> >   common/rc: introduce new helper function _fs_type_dev_dir()
> >   common/rc: add quirks for fuse-overlayfs device/mount point
> >   common/overlay: silence some mount messages for fuse-overlayfs
> >
> >  README.overlay    |  5 ++++
> >  common/config     |  2 ++
> >  common/overlay    | 29 +++++++++++++++++++---
> >  common/rc         | 61 ++++++++++++++++++++++++++++++++++++++++-------
> >  tests/overlay/011 |  2 +-
> >  tests/overlay/035 |  2 +-
> >  tests/overlay/052 |  4 ++--
> >  tests/overlay/053 |  4 ++--
> >  tests/overlay/062 |  2 +-
> >  9 files changed, 92 insertions(+), 19 deletions(-)
> >
> > --
> > 2.20.1
> >



--
Mauricio Faria de Oliveira

Re: [PATCH v2 1/5] common/overlay,rc,config: introduce OVL_FSTYP variable and aufs

[Amir Goldstein]

On Fri, Feb 14, 2020 at 5:18 PM Mauricio Faria de Oliveira
<mfo@canonical.com> wrote:
>
> Recently I was looking for an aufs test suite, and reached out to
> Okajima, but 'There is no public test suite specific to aufs.' [1],
> and it looks like 'xfstests/tests/generic' should be enough [1, 2].
>
> Thus, building on top existing xfstests support for overlay just
> introduce the OVL_FSTYP variable, and the default value "overlay"
> can be changed to "aufs" (uses overlay's upperdir as a rw-branch
> and lowerdir as a ro-branch; workdir is not used.)
>
> This is indeed a workaround^W simple change that does the job vs.
> creating a new FSTYP "aufs" and mechanically changing the number
> of places that check for "overlay" to just handle "aufs" as well.
> (so the effort is still small as aufs has no specific tests now.)
>
> This also allows testing fuse-overlayfs with the next patches.
>
> The changes are minimal -- just translate overlay mount options
> and use $OVL_FSTYP as filesystem type for checking/mount/umount;
> then report it in log headers and document it in README.overlay.
>
> Currently, running './check -overlay' tests (excluding a few [3]
> which either hang or keep looping) the numbers for aufs on loop
> devices on v5.4-based Ubuntu kernel are:
>
>   - Ran: 645 tests
>   - Not run: 483 tests
>   - Failures: 22 tests
>
> So, hopefully this may help with a starting point for an public
> test suite for aufs.
>
> Thanks to Amir Goldstein for feedback/improvements and pointers
> to support fuse-overlayfs as well [v2].
>
> [1] https://sourceforge.net/p/aufs/mailman/message/36918721/
> [2] https://sourceforge.net/p/aufs/mailman/message/36918932/
> [3] Steps:
>
>   $ export OVL_FSTYP=aufs
>   $ export FSTYP=ext4
>   $ export TEST_DEV=/dev/loop0
>   $ export TEST_DIR=/mnt/test
>   $ export SCRATCH_DEV=/dev/loop1
>   $ export SCRATCH_MNT=/mnt/scratch
>
>   $ sudo mkfs.$FSTYP -F $TEST_DEV
>   $ sudo mkfs.$FSTYP -F $SCRATCH_DEV
>   $ sudo mkdir $TEST_DIR $SCRATCH_MNT
>
>   $ cat <<EOF >/tmp/exclude-tests
>   generic/013
>   generic/070
>   generic/075
>   generic/112
>   generic/127
>   generic/461
>   generic/476
>   generic/522
>   generic/530
>   overlay/019
>   EOF
>
>   $ sudo -E ./check -overlay -E /tmp/exclude-tests
>
> Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
> ---
>  README.overlay |  4 ++++
>  common/config  |  2 ++
>  common/overlay | 11 ++++++++---
>  common/rc      |  6 ++++++
>  4 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/README.overlay b/README.overlay
> index 30b5ddb2d1c3..08a39b8830c9 100644
> --- a/README.overlay
> +++ b/README.overlay
> @@ -50,3 +50,7 @@ In the example above, MOUNT_OPTIONS will be used to mount the base scratch fs,
>  TEST_FS_MOUNT_OPTS will be used to mount the base test fs,
>  OVERLAY_MOUNT_OPTIONS will be used to mount both test and scratch overlay and
>  OVERLAY_FSCK_OPTIONS will be used to check both test and scratch overlay.
> +
> +To test other filesystem types (experimental) configure the OVL_FSTYP variable:
> +
> + OVL_FSTYP=aufs
> diff --git a/common/config b/common/config
> index 9a9c77602b54..d92a78003295 100644
> --- a/common/config
> +++ b/common/config
> @@ -71,6 +71,8 @@ export OVL_LOWER="ovl-lower"
>  export OVL_WORK="ovl-work"
>  # overlay mount point parent must be the base fs root
>  export OVL_MNT="ovl-mnt"
> +# overlay mount filesystem type (for testing other fs)
> +export OVL_FSTYP=${OVL_FSTYP:-overlay}
>
>  # From e2fsprogs/e2fsck/e2fsck.h:
>  # Exit code used by fsck-type programs
> diff --git a/common/overlay b/common/overlay
> index 65c639e9c6d8..a1076926c23f 100644
> --- a/common/overlay
> +++ b/common/overlay
> @@ -18,10 +18,15 @@ _overlay_mount_dirs()
>         local lowerdir=$1
>         local upperdir=$2
>         local workdir=$3
> +       local options
>         shift 3
>
> -       $MOUNT_PROG -t overlay -o lowerdir=$lowerdir -o upperdir=$upperdir \
> -                   -o workdir=$workdir `_common_dev_mount_options $*`
> +       options="-o lowerdir=$lowerdir -o upperdir=$upperdir -o workdir=$workdir"
> +       if [ "$OVL_FSTYP" = "aufs" ]; then
> +               options="-o br=$upperdir=rw -o br=$lowerdir=ro"
> +       fi
> +
> +       $MOUNT_PROG -t $OVL_FSTYP $options `_common_dev_mount_options $*`
>  }
>
>  # Mount with same options/mnt/dev of scratch mount, but optionally
> @@ -302,7 +307,7 @@ _overlay_check_fs()
>                 _overlay_base_mount $*
>         else
>                 # Check and umount overlay for dir check
> -               ovl_mounted=`_is_dir_mountpoint $ovl_mnt`
> +               ovl_mounted=`_is_dir_mountpoint $ovl_mnt $OVL_FSTYP`
>                 [ -z "$ovl_mounted" ] || $UMOUNT_PROG $ovl_mnt
>         fi
>
> diff --git a/common/rc b/common/rc
> index b4a77a2187f4..1feae1a94f9e 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1471,6 +1471,10 @@ _check_mounted_on()
>                 return 2 # 2 = mounted on wrong mnt
>         fi
>
> +       if [ -n "$type" -a "$type" = "overlay" ]; then
> +               type="$OVL_FSTYP"
> +       fi
> +

Hmm. I found 2 other instances of _fs_type in common/rc.
I think it would be safer to let _fs_type return "overlay" in
case the mounted fs is of type $OVL_FSTYP.
This will be simple to do by extending the sed expression -
no need for special cases and conditions.

Other than that, patch looks good.

Thanks,
Amir.

>         if [ -n "$type" -a "`_fs_type $dev`" != "$type" ]; then
>                 echo "$devname=$dev is mounted but not a type $type filesystem"
>                 # raw $DF_PROG cannot handle NFS/CIFS/overlay correctly
> @@ -2841,6 +2845,8 @@ _full_fstyp_details()
>                 FSTYP="$FSTYP (non-debug)"
>             fi
>         fi
> +     elif [ $FSTYP = "overlay" -a "$OVL_FSTYP" != "overlay" ]; then
> +       FSTYP="$FSTYP ($OVL_FSTYP)"
>       fi
>       echo $FSTYP
>  }
> --
> 2.20.1
>

Re: [PATCH v2 3/5] common/rc: introduce new helper function _fs_type_dev_dir()

[Amir Goldstein]

On Fri, Feb 14, 2020 at 5:18 PM Mauricio Faria de Oliveira
<mfo@canonical.com> wrote:
>
> In order to determine the fs type on fuse-overlayfs (coming)
> we need to search for the mount point/directory; the device
> is not enough. (details in the next patch.)
>
> Thus the _fs_type() function is insufficient to determine
> the filesystem type, as it only searches for mount device.
>
> So, introduce the _fs_type_dev_dir() function, which also
> searches for the mountpoint/dir in addition to the device.
>
> The fs type fix-up sed script goes into a common function.
>
> P.S.: there might be other sites that need similar changes,
> since the mount device is also checked elsewhere, but just
> with this bit tests can run, so it is good enough for now.)
>
> Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>

Looks ok.
You may add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  common/rc | 36 ++++++++++++++++++++++++++++--------
>  1 file changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 1feae1a94f9e..5711eca2a1d2 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1262,6 +1262,19 @@ _used()
>      _df_device $1 | $AWK_PROG '{ sub("%", "") ; print $6 }'
>  }
>
> +# fix filesystem type up
> +#
> +_fix_fs_type()
> +{
> +    #
> +    # The Linux kernel shows NFSv4 filesystems in df output as
> +    # filesystem type nfs4, although we mounted it as nfs earlier.
> +    # Fix the filesystem type up here so that the callers don't
> +    # have to bother with this quirk.
> +    #
> +    sed -e 's/nfs4/nfs/' -e 's/fuse.glusterfs/glusterfs/'
> +}
> +
>  # return the FS type of a mounted device
>  #
>  _fs_type()
> @@ -1272,14 +1285,21 @@ _fs_type()
>         exit 1
>      fi
>
> -    #
> -    # The Linux kernel shows NFSv4 filesystems in df output as
> -    # filesystem type nfs4, although we mounted it as nfs earlier.
> -    # Fix the filesystem type up here so that the callers don't
> -    # have to bother with this quirk.
> -    #
> -    _df_device $1 | $AWK_PROG '{ print $2 }' | \
> -        sed -e 's/nfs4/nfs/' -e 's/fuse.glusterfs/glusterfs/'
> +    _df_device $1 | $AWK_PROG '{ print $2 }' | _fix_fs_type
> +}
> +
> +# return the FS type of a mounted device
> +# on a mount point directory (check both)
> +#
> +_fs_type_dev_dir()
> +{
> +    if [ $# -ne 2 ]
> +    then
> +       echo "Usage: _fs_type_dev_dir device directory" 1>&2
> +       exit 1
> +    fi
> +
> +    _df_dir $2 | $AWK_PROG -v what=$1 '($1==what) { print $2 }' | _fix_fs_type
>  }
>
>  # return the FS mount options of a mounted device
> --
> 2.20.1
>

Re: [PATCH v2 4/5] common/rc: add quirks for fuse-overlayfs device/mount point

[Amir Goldstein]

On Fri, Feb 14, 2020 at 5:18 PM Mauricio Faria de Oliveira
<mfo@canonical.com> wrote:
>
> On fuse-overlayfs the mount device is always reported as
> a "fuse-overlayfs" string, instead of the parent directory
> of the mount point (the string expected by overlay/scripts).
>
> Unfortunately, it seems that the fuse mount option 'fsname'
> doesn't set the filesystem source/device on fuse-overlayfs,
> so the easy fix of just adding it to mount options is gone.
>
> So two quirks are used to check for a fuse-overlayfs mount,
> that checks the mount point/directory in _check_mounted_on()
> and init_rc(); latter with the new helper _fs_type_dev_dir().
>
> With this, fuse-overlayfs can now keep going through tests!
>
> Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
> ---
>  README.overlay |  1 +
>  common/rc      | 19 ++++++++++++++++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/README.overlay b/README.overlay
> index 08a39b8830c9..7ef07ae6bbab 100644
> --- a/README.overlay
> +++ b/README.overlay
> @@ -53,4 +53,5 @@ OVERLAY_FSCK_OPTIONS will be used to check both test and scratch overlay.
>
>  To test other filesystem types (experimental) configure the OVL_FSTYP variable:
>
> + OVL_FSTYP=fuse.fuse-overlayfs
>   OVL_FSTYP=aufs
> diff --git a/common/rc b/common/rc
> index 5711eca2a1d2..6a4cb9b6d604 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1481,6 +1481,14 @@ _check_mounted_on()
>
>         # find $dev as the source, and print result in "$dev $mnt" format
>         local mount_rec=`findmnt -rncv -S $dev -o SOURCE,TARGET`
> +
> +       # fuse-overlayfs dev is not $dev, check via $mnt.
> +       if [ -z "$mount_rec" -a "$FSTYP" = "overlay" -a \
> +               "$OVL_FSTYP" = "fuse.fuse-overlayfs" ]; then
> +               dev="fuse-overlayfs"
> +               mount_rec=`findmnt -rncv -S $dev -T $mnt -o SOURCE,TARGET`
> +       fi
> +

I guess if we have to do the quirk then we better do it before the
first findmnt.
I don't see the point of trying the first time.

But I wonder, how come this is not also problem for virtiofs?
Maybe there is a way to make fuse fsname mount option work
instead of these quirks?

>         [ -n "$mount_rec" ] || return 1 # 1 = not mounted
>
>         # if it's mounted, make sure its on $mnt
> @@ -3788,8 +3796,17 @@ init_rc()
>         fi
>
>         # if $TEST_DEV is not mounted, mount it now as XFS
> -       if [ -z "`_fs_type $TEST_DEV`" ]
> +       if [ -n "`_fs_type $TEST_DEV`" ]
>         then
> +               # $TEST_DEV is mounted
> +               true
> +       elif [ "$FSTYP" = "overlay" -a "$OVL_FSTYP" = "fuse.fuse-overlayfs" -a \
> +               -n "`_fs_type_dev_dir fuse-overlayfs $TEST_DEV/$OVL_MNT`" ]

Somewhat simplified:
       elif [ "$OVL_FSTYP" = "fuse.fuse-overlayfs" -a \
               -n "`_fs_type_dev_dir fuse-overlayfs $TEST_DIR`" ]


Thanks,
Amir.

Re: [PATCH v2 5/5] common/overlay: silence some mount messages for fuse-overlayfs

[Amir Goldstein]

On Fri, Feb 14, 2020 at 5:19 PM Mauricio Faria de Oliveira
<mfo@canonical.com> wrote:
>
> When mouting fuse-overlayfs there are some messages that make
> the tests report failures due to output mismatch; ignore them:
>
>   uid=unchanged
>   uid=unchanged
>   upperdir=/mnt/test/ovl-upper
>   workdir=/mnt/test/ovl-work
>   lowerdir=/mnt/test/ovl-lower
>   mountpoint=/mnt/test/ovl-mnt
>
> For other filesystem types (e.g., overlay and aufs) make sure to
> only print non-null output, to avoid blank lines output mismatch.
>
> And return the status of the mount command, not other commands.
>
> Currently, running './check -overlay' tests (excluding generic/062)
> the numbers for fuse-overlayfs on loop devices on v5.4-based Ubuntu
> kernel with the fuse-overlayfs package from Ubuntu Eoan/19.10 are:
>
>  - Ran: 530
>  - Not run: 395
>  - Failures: 29
>
> And hopefully this helps with testing for fuse-overlayfs too.
>
> Steps:
>
>   $ export OVL_FSTYP=fuse.fuse-overlayfs
>   $ export FSTYP=ext4
>   $ export TEST_DEV=/dev/loop0
>   $ export TEST_DIR=/mnt/test
>   $ export SCRATCH_DEV=/dev/loop1
>   $ export SCRATCH_MNT=/mnt/scratch
>
>   $ sudo mkfs.$FSTYP -F $TEST_DEV
>   $ sudo mkfs.$FSTYP -F $SCRATCH_DEV
>   $ sudo mkdir $TEST_DIR $SCRATCH_MNT
>
>   $ cat <<EOF >/tmp/exclude-tests
>   generic/062
>   EOF
>
>   $ sudo -E ./check -overlay -E /tmp/exclude-tests
>
> Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
> ---
>  common/overlay | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/common/overlay b/common/overlay
> index a1076926c23f..27f3c08252ee 100644
> --- a/common/overlay
> +++ b/common/overlay
> @@ -19,6 +19,8 @@ _overlay_mount_dirs()
>         local upperdir=$2
>         local workdir=$3
>         local options
> +       local output
> +       local rc
>         shift 3
>
>         options="-o lowerdir=$lowerdir -o upperdir=$upperdir -o workdir=$workdir"
> @@ -26,7 +28,23 @@ _overlay_mount_dirs()
>                 options="-o br=$upperdir=rw -o br=$lowerdir=ro"
>         fi
>
> -       $MOUNT_PROG -t $OVL_FSTYP $options `_common_dev_mount_options $*`
> +       options="$options `_common_dev_mount_options $*`"
> +       output="`$MOUNT_PROG -t $OVL_FSTYP $options 2>&1`"
> +       rc=$?
> +
> +       if [ "$OVL_FSTYP" = "fuse.fuse-overlayfs" ]; then
> +               # Less verbosity to avoid output mismatch.
> +               echo "$output" | grep -v \
> +                       -e "^uid=" \
> +                       -e "^upperdir=" \
> +                       -e "^lowerdir=" \
> +                       -e "^workdir=" \
> +                       -e "^mountpoint="
> +       elif [ -n "$output" ]; then
> +               echo "$output"
> +       fi
> +
> +       return $rc

rc will always be 0 because it holds the return code of the assignment
expression 'output=...', not the return code of mount.

Does fuse mount verbose output go to stdout or to stderr?
I don't think we ever care about stdout of this mount command
on golden output - it is always assumed to be silent and
only a silent or noisy stderr is verified in golden output.
So if I am not mistaken and if fuse verbose output it to stdout,
then maybe would be enough to redirect stdout to /dev/null?

Thanks,
Amir.