diff mbox series

[RFC] common: overlay support tmpfs

Message ID 20220424063751.1067376-1-libaokun1@huawei.com (mailing list archive)
State New, archived
Headers show
Series [RFC] common: overlay support tmpfs | expand

Commit Message

Baokun Li April 24, 2022, 6:37 a.m. UTC
xfstests support overlay+tmpfs

```local.config.example
export FSTYP=tmpfs
export TEST_DEV=tmpfs_test
export TEST_DIR=/tmp/test
export TEST_FS_MOUNT_OPTS="-t tmpfs"
export SCRATCH_DEV=tmpfs_scratch
export SCRATCH_MNT=/tmp/scratch
export MOUNT_OPTIONS="-t tmpfs"
```
Run `./check -overlay tests` to execute test case on overlay+tmpfs.

Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
 common/config | 4 ++--
 common/rc     | 7 ++++++-
 2 files changed, 8 insertions(+), 3 deletions(-)

Comments

Amir Goldstein April 24, 2022, 12:02 p.m. UTC | #1
On Sun, Apr 24, 2022 at 2:21 PM Baokun Li <libaokun1@huawei.com> wrote:
>
> xfstests support overlay+tmpfs

Thanks for this improvement.
Can you please share the results of ./check -overlay -g auto ?

How many tests ran? notran? failed?

Best if you have those numbers compared to
overlay+(already supported base fs)


>
> ```local.config.example
> export FSTYP=tmpfs
> export TEST_DEV=tmpfs_test
> export TEST_DIR=/tmp/test
> export TEST_FS_MOUNT_OPTS="-t tmpfs"
> export SCRATCH_DEV=tmpfs_scratch
> export SCRATCH_MNT=/tmp/scratch
> export MOUNT_OPTIONS="-t tmpfs"

These mount options for tmpfs are very awkward.
Please fix _overlay_base_mount to use -t $OVL_BASE_FSTYP
like _test_mount() and _try_scratch_mount() do


> ```
> Run `./check -overlay tests` to execute test case on overlay+tmpfs.
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
> ---
>  common/config | 4 ++--
>  common/rc     | 7 ++++++-
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/common/config b/common/config
> index 1033b890..3dc047e8 100644
> --- a/common/config
> +++ b/common/config
> @@ -614,7 +614,7 @@ _overlay_config_override()
>         #    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
>         #    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
>         #    overlayfs base and mount dirs inside base fs mount.
> -       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || return 0
> +       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || [ "$FSTYP" == tmpfs ] || return 0
>
>         # Config file may specify base fs type, but we obay -overlay flag
>         [ "$FSTYP" == overlay ] || export OVL_BASE_FSTYP="$FSTYP"

Please move the setting of OVL_BASE_FSTYP to the top of the function and use
[ "$OVL_BASE_FSTYP" == tmpfs ] consistently.

> @@ -634,7 +634,7 @@ _overlay_config_override()
>         export TEST_DIR="$OVL_BASE_TEST_DIR/$OVL_MNT"
>         export MOUNT_OPTIONS="$OVERLAY_MOUNT_OPTIONS"
>
> -       [ -b "$SCRATCH_DEV" ] || [ -c "$SCRATCH_DEV" ] || return 0
> +       [ -b "$SCRATCH_DEV" ] || [ -c "$SCRATCH_DEV" ] || [ "$OVL_BASE_FSTYP" == tmpfs ] || return 0
>
>         # Store original base fs vars
>         export OVL_BASE_SCRATCH_DEV="$SCRATCH_DEV"
> diff --git a/common/rc b/common/rc
> index 553ae350..12498189 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -834,13 +834,18 @@ _scratch_mkfs()
>         local mkfs_status
>
>         case $FSTYP in
> -       nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p|virtiofs)
> +       nfs*|cifs|ceph|glusterfs|pvfs2|9p|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
>                 _scratch_cleanup_files
>                 return $?
>                 ;;
> +       overlay)
> +               [ ! "$OVL_BASE_FSTYP" == tmpfs ] || return 0
> +               _scratch_cleanup_files
> +               return $?
> +               ;;

Why? What's the problem with cleaning the ovl-* layers on tmpfs?


Thanks,
Amir.
Zorro Lang April 24, 2022, 1:23 p.m. UTC | #2
On Sun, Apr 24, 2022 at 03:02:03PM +0300, Amir Goldstein wrote:
> On Sun, Apr 24, 2022 at 2:21 PM Baokun Li <libaokun1@huawei.com> wrote:
> >
> > xfstests support overlay+tmpfs
> 
> Thanks for this improvement.
> Can you please share the results of ./check -overlay -g auto ?
> 
> How many tests ran? notran? failed?
> 
> Best if you have those numbers compared to
> overlay+(already supported base fs)
> 
> 
> >
> > ```local.config.example
> > export FSTYP=tmpfs
> > export TEST_DEV=tmpfs_test
> > export TEST_DIR=/tmp/test
> > export TEST_FS_MOUNT_OPTS="-t tmpfs"
> > export SCRATCH_DEV=tmpfs_scratch
> > export SCRATCH_MNT=/tmp/scratch
> > export MOUNT_OPTIONS="-t tmpfs"
> 
> These mount options for tmpfs are very awkward.
> Please fix _overlay_base_mount to use -t $OVL_BASE_FSTYP
> like _test_mount() and _try_scratch_mount() do
> 
> 
> > ```
> > Run `./check -overlay tests` to execute test case on overlay+tmpfs.
> >
> > Signed-off-by: Baokun Li <libaokun1@huawei.com>
> > ---
> >  common/config | 4 ++--
> >  common/rc     | 7 ++++++-
> >  2 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/common/config b/common/config
> > index 1033b890..3dc047e8 100644
> > --- a/common/config
> > +++ b/common/config
> > @@ -614,7 +614,7 @@ _overlay_config_override()
> >         #    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
> >         #    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
> >         #    overlayfs base and mount dirs inside base fs mount.
> > -       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || return 0
> > +       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || [ "$FSTYP" == tmpfs ] || return 0
> >
> >         # Config file may specify base fs type, but we obay -overlay flag
> >         [ "$FSTYP" == overlay ] || export OVL_BASE_FSTYP="$FSTYP"
> 
> Please move the setting of OVL_BASE_FSTYP to the top of the function and use
> [ "$OVL_BASE_FSTYP" == tmpfs ] consistently.

Actually I'm wondering if we can bring in a parameter to clarify that xfstests need to
build uplying fs base on a underlying fs, don't depend on the "[ -b "$TEST_DEV" ] ||
[ -c "$TEST_DEV" ] || [ "$FSTYP" == tmpfs ]" things. Due to:
1) overlayfs might not only base on localfs, it can over NFS or something likes it. (right?)
   If so, how many judgements we need to add at here?
2) If xfstests can help overlayfs, that means it can help to build other fs (e.g. nfs, cifs,
   ceph, etc) from an underlying fs in one day.

So how about bring in a parameter, maybe USE_UNDERLYING_FS=yes/no(default), or use "BASE_FSTYP"
directly, e.g.
export USE_UNDERLYING_FS=yes
export FSTYP=tmpfs
....
if [ "USE_UNDERLYING_FS" = "yes" ];then build uplying fs from $FSTYP

Or:
export BASE_FSTYP=xfs (BASE_FSTYP=$FSTYP if BASE_FSTYP is empty)
export FSTYP=overlay
export TEST_DEV=/dev/sdb1
export TEST_DIR=/mnt/test
export BASE_MOUNT_OPTIONS=...
export MOUNT_OPTIONS=...
...
if [ "$BASE_FSTYP" != "$FSTYP" ];then build $FSTYP from $BASE_FSTYP

The 2nd method might be complex, and will affect current code logic much. The 1st one is
simple. This idea just flashed my mind, I haven't thought too much. If others feel this
idea is good too, I can try to implement.

Thanks,
Zorro


> 
> > @@ -634,7 +634,7 @@ _overlay_config_override()
> >         export TEST_DIR="$OVL_BASE_TEST_DIR/$OVL_MNT"
> >         export MOUNT_OPTIONS="$OVERLAY_MOUNT_OPTIONS"
> >
> > -       [ -b "$SCRATCH_DEV" ] || [ -c "$SCRATCH_DEV" ] || return 0
> > +       [ -b "$SCRATCH_DEV" ] || [ -c "$SCRATCH_DEV" ] || [ "$OVL_BASE_FSTYP" == tmpfs ] || return 0
> >
> >         # Store original base fs vars
> >         export OVL_BASE_SCRATCH_DEV="$SCRATCH_DEV"
> > diff --git a/common/rc b/common/rc
> > index 553ae350..12498189 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -834,13 +834,18 @@ _scratch_mkfs()
> >         local mkfs_status
> >
> >         case $FSTYP in
> > -       nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p|virtiofs)
> > +       nfs*|cifs|ceph|glusterfs|pvfs2|9p|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
> >                 _scratch_cleanup_files
> >                 return $?
> >                 ;;
> > +       overlay)
> > +               [ ! "$OVL_BASE_FSTYP" == tmpfs ] || return 0
> > +               _scratch_cleanup_files
> > +               return $?
> > +               ;;
> 
> Why? What's the problem with cleaning the ovl-* layers on tmpfs?
> 
> 
> Thanks,
> Amir.
>
Amir Goldstein April 24, 2022, 2:34 p.m. UTC | #3
On Sun, Apr 24, 2022 at 4:24 PM Zorro Lang <zlang@redhat.com> wrote:
>
> On Sun, Apr 24, 2022 at 03:02:03PM +0300, Amir Goldstein wrote:
> > On Sun, Apr 24, 2022 at 2:21 PM Baokun Li <libaokun1@huawei.com> wrote:
> > >
> > > xfstests support overlay+tmpfs
> >
> > Thanks for this improvement.
> > Can you please share the results of ./check -overlay -g auto ?
> >
> > How many tests ran? notran? failed?
> >
> > Best if you have those numbers compared to
> > overlay+(already supported base fs)
> >
> >
> > >
> > > ```local.config.example
> > > export FSTYP=tmpfs
> > > export TEST_DEV=tmpfs_test
> > > export TEST_DIR=/tmp/test
> > > export TEST_FS_MOUNT_OPTS="-t tmpfs"
> > > export SCRATCH_DEV=tmpfs_scratch
> > > export SCRATCH_MNT=/tmp/scratch
> > > export MOUNT_OPTIONS="-t tmpfs"
> >
> > These mount options for tmpfs are very awkward.
> > Please fix _overlay_base_mount to use -t $OVL_BASE_FSTYP
> > like _test_mount() and _try_scratch_mount() do
> >
> >
> > > ```
> > > Run `./check -overlay tests` to execute test case on overlay+tmpfs.
> > >
> > > Signed-off-by: Baokun Li <libaokun1@huawei.com>
> > > ---
> > >  common/config | 4 ++--
> > >  common/rc     | 7 ++++++-
> > >  2 files changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/common/config b/common/config
> > > index 1033b890..3dc047e8 100644
> > > --- a/common/config
> > > +++ b/common/config
> > > @@ -614,7 +614,7 @@ _overlay_config_override()
> > >         #    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
> > >         #    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
> > >         #    overlayfs base and mount dirs inside base fs mount.
> > > -       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || return 0
> > > +       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || [ "$FSTYP" == tmpfs ] || return 0
> > >
> > >         # Config file may specify base fs type, but we obay -overlay flag
> > >         [ "$FSTYP" == overlay ] || export OVL_BASE_FSTYP="$FSTYP"
> >
> > Please move the setting of OVL_BASE_FSTYP to the top of the function and use
> > [ "$OVL_BASE_FSTYP" == tmpfs ] consistently.
>
> Actually I'm wondering if we can bring in a parameter to clarify that xfstests need to
> build uplying fs base on a underlying fs, don't depend on the "[ -b "$TEST_DEV" ] ||
> [ -c "$TEST_DEV" ] || [ "$FSTYP" == tmpfs ]" things. Due to:
> 1) overlayfs might not only base on localfs, it can over NFS or something likes it. (right?)

No it cannot.
The way that xfstests -overlay work is that both upper and lower layers
are created on the base fs, therefore only fs supported as upper fs
can be tested with -overlay.
None of the network fs qualify as valid overlay upper fs.
The only other non-blockdev fs besides tmpfs that could be tested
with -overlay is virtiofs.

>    If so, how many judgements we need to add at here?
> 2) If xfstests can help overlayfs, that means it can help to build other fs (e.g. nfs, cifs,
>    ceph, etc) from an underlying fs in one day.
>
> So how about bring in a parameter, maybe USE_UNDERLYING_FS=yes/no(default), or use "BASE_FSTYP"
> directly, e.g.
> export USE_UNDERLYING_FS=yes
> export FSTYP=tmpfs

This already exists:

export OVERLAY=true
export FSTYP=tmpfs

means exactly that, but is usually set internally by ./check -overlay

I think what you mean is that this should be a helper:

_overlay_is_valid_upper_fs()
{
        local basedev=$1

        case $FSTYP in
        tmpfs)
                return 0
                ;;
        *)
                [ -b "$basedev" ] || [ -c "$basedev" ]
                return $?
                ;;
        esac
}

Then we could whitelist other fs after tmpfs
and also blacklist other fs even if they are blockdev fs.

Thanks,
Amir.
Zorro Lang April 24, 2022, 6:41 p.m. UTC | #4
On Sun, Apr 24, 2022 at 05:34:44PM +0300, Amir Goldstein wrote:
> On Sun, Apr 24, 2022 at 4:24 PM Zorro Lang <zlang@redhat.com> wrote:
> >
> > On Sun, Apr 24, 2022 at 03:02:03PM +0300, Amir Goldstein wrote:
> > > On Sun, Apr 24, 2022 at 2:21 PM Baokun Li <libaokun1@huawei.com> wrote:
> > > >
> > > > xfstests support overlay+tmpfs
> > >
> > > Thanks for this improvement.
> > > Can you please share the results of ./check -overlay -g auto ?
> > >
> > > How many tests ran? notran? failed?
> > >
> > > Best if you have those numbers compared to
> > > overlay+(already supported base fs)
> > >
> > >
> > > >
> > > > ```local.config.example
> > > > export FSTYP=tmpfs
> > > > export TEST_DEV=tmpfs_test
> > > > export TEST_DIR=/tmp/test
> > > > export TEST_FS_MOUNT_OPTS="-t tmpfs"
> > > > export SCRATCH_DEV=tmpfs_scratch
> > > > export SCRATCH_MNT=/tmp/scratch
> > > > export MOUNT_OPTIONS="-t tmpfs"
> > >
> > > These mount options for tmpfs are very awkward.
> > > Please fix _overlay_base_mount to use -t $OVL_BASE_FSTYP
> > > like _test_mount() and _try_scratch_mount() do
> > >
> > >
> > > > ```
> > > > Run `./check -overlay tests` to execute test case on overlay+tmpfs.
> > > >
> > > > Signed-off-by: Baokun Li <libaokun1@huawei.com>
> > > > ---
> > > >  common/config | 4 ++--
> > > >  common/rc     | 7 ++++++-
> > > >  2 files changed, 8 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/common/config b/common/config
> > > > index 1033b890..3dc047e8 100644
> > > > --- a/common/config
> > > > +++ b/common/config
> > > > @@ -614,7 +614,7 @@ _overlay_config_override()
> > > >         #    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
> > > >         #    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
> > > >         #    overlayfs base and mount dirs inside base fs mount.
> > > > -       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || return 0
> > > > +       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || [ "$FSTYP" == tmpfs ] || return 0
> > > >
> > > >         # Config file may specify base fs type, but we obay -overlay flag
> > > >         [ "$FSTYP" == overlay ] || export OVL_BASE_FSTYP="$FSTYP"
> > >
> > > Please move the setting of OVL_BASE_FSTYP to the top of the function and use
> > > [ "$OVL_BASE_FSTYP" == tmpfs ] consistently.
> >
> > Actually I'm wondering if we can bring in a parameter to clarify that xfstests need to
> > build uplying fs base on a underlying fs, don't depend on the "[ -b "$TEST_DEV" ] ||
> > [ -c "$TEST_DEV" ] || [ "$FSTYP" == tmpfs ]" things. Due to:
> > 1) overlayfs might not only base on localfs, it can over NFS or something likes it. (right?)
> 
> No it cannot.
> The way that xfstests -overlay work is that both upper and lower layers
> are created on the base fs, therefore only fs supported as upper fs
> can be tested with -overlay.
> None of the network fs qualify as valid overlay upper fs.
> The only other non-blockdev fs besides tmpfs that could be tested
> with -overlay is virtiofs.

Oh, looks like my memory is a little jumbled, maybe overlayfs can be exported to NFS :)

> 
> >    If so, how many judgements we need to add at here?
> > 2) If xfstests can help overlayfs, that means it can help to build other fs (e.g. nfs, cifs,
> >    ceph, etc) from an underlying fs in one day.

Actually I asked for this ^^ mainly.

> >
> > So how about bring in a parameter, maybe USE_UNDERLYING_FS=yes/no(default), or use "BASE_FSTYP"
> > directly, e.g.
> > export USE_UNDERLYING_FS=yes
> > export FSTYP=tmpfs
> 
> This already exists:
> 
> export OVERLAY=true
> export FSTYP=tmpfs

Yes, it's same with ./check -overlay, so how about

# need override
if [ "$OVERLAY" = "true" -a "$FSTYP" != "overlay" ];then
...

> 
> means exactly that, but is usually set internally by ./check -overlay
> 
> I think what you mean is that this should be a helper:
> 
> _overlay_is_valid_upper_fs()
> {
>         local basedev=$1
> 
>         case $FSTYP in
>         tmpfs)
>                 return 0
>                 ;;
>         *)
>                 [ -b "$basedev" ] || [ -c "$basedev" ]
>                 return $?
>                 ;;
>         esac
> }

Sure, if we don't leave this job to tester (tester makes sure he provide proper TEST_DEV
and SCRATCH_DEV), we help to judge that :)

......

Now uplying fs testing in xfstests supports two ways:
1) Let testers prepare uplying fs TEST_DEV and SCRATCH_DEV, then set FSTYP=$upfs
2) The testers prepare underlying fs TEST_DEV and SCRATCH_DEV, then set FSTYP=$underfs, and
   run ./check -$upfs ...

Until now, only overlayfs supports both 2 ways (if I don't remember wrong), others (likes
nfs, cifs, glusterfs, ceph etc) are only support the 1st way.

So I'm thinking if other fs would like to be supported as overlay, we might can help to
provide:
1) A parameter UPLYING_FS to record if we're testing a fs can be uplying fs, e.g:
        -nfs)           FSTYP=nfs; UPLYING_FS=nfs ;;
        -glusterfs)     FSTYP=glusterfs; UPLYING_FS=glusterfs ;;
        -cifs)          FSTYP=cifs; export UPLYING_FS=cifs ;;
        -overlay)       FSTYP=overlay; export UPLYING_FS=overly ;;

2) A common _config_override() function, and different _${UPLYING_FS}_config_override()
   functions.
3) After source local.config, check if [ "$UPLYING_FS" != "$FSTYP" ]. If they're equal,
   then return, else we need underlying things, then run _${UPLYING_FS}_is_valid() to check
   the TEST_DEV and SCRATCH_DEV are good for ${UPLYING_FS}.
4) Do real override.

As overlayfs is the only one supports the 2nd way, so maybe we can let overlay testing
to be a demonstration. Does that make sense?

Thanks,
Zorro

> 
> Then we could whitelist other fs after tmpfs
> and also blacklist other fs even if they are blockdev fs.
> 
> Thanks,
> Amir.
>
Amir Goldstein April 24, 2022, 8:42 p.m. UTC | #5
On Sun, Apr 24, 2022 at 9:41 PM Zorro Lang <zlang@redhat.com> wrote:
>
> On Sun, Apr 24, 2022 at 05:34:44PM +0300, Amir Goldstein wrote:
> > On Sun, Apr 24, 2022 at 4:24 PM Zorro Lang <zlang@redhat.com> wrote:
> > >
> > > On Sun, Apr 24, 2022 at 03:02:03PM +0300, Amir Goldstein wrote:
> > > > On Sun, Apr 24, 2022 at 2:21 PM Baokun Li <libaokun1@huawei.com> wrote:
> > > > >
> > > > > xfstests support overlay+tmpfs
> > > >
> > > > Thanks for this improvement.
> > > > Can you please share the results of ./check -overlay -g auto ?
> > > >
> > > > How many tests ran? notran? failed?
> > > >
> > > > Best if you have those numbers compared to
> > > > overlay+(already supported base fs)
> > > >
> > > >
> > > > >
> > > > > ```local.config.example
> > > > > export FSTYP=tmpfs
> > > > > export TEST_DEV=tmpfs_test
> > > > > export TEST_DIR=/tmp/test
> > > > > export TEST_FS_MOUNT_OPTS="-t tmpfs"
> > > > > export SCRATCH_DEV=tmpfs_scratch
> > > > > export SCRATCH_MNT=/tmp/scratch
> > > > > export MOUNT_OPTIONS="-t tmpfs"
> > > >
> > > > These mount options for tmpfs are very awkward.
> > > > Please fix _overlay_base_mount to use -t $OVL_BASE_FSTYP
> > > > like _test_mount() and _try_scratch_mount() do
> > > >
> > > >
> > > > > ```
> > > > > Run `./check -overlay tests` to execute test case on overlay+tmpfs.
> > > > >
> > > > > Signed-off-by: Baokun Li <libaokun1@huawei.com>
> > > > > ---
> > > > >  common/config | 4 ++--
> > > > >  common/rc     | 7 ++++++-
> > > > >  2 files changed, 8 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/common/config b/common/config
> > > > > index 1033b890..3dc047e8 100644
> > > > > --- a/common/config
> > > > > +++ b/common/config
> > > > > @@ -614,7 +614,7 @@ _overlay_config_override()
> > > > >         #    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
> > > > >         #    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
> > > > >         #    overlayfs base and mount dirs inside base fs mount.
> > > > > -       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || return 0
> > > > > +       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || [ "$FSTYP" == tmpfs ] || return 0
> > > > >
> > > > >         # Config file may specify base fs type, but we obay -overlay flag
> > > > >         [ "$FSTYP" == overlay ] || export OVL_BASE_FSTYP="$FSTYP"
> > > >
> > > > Please move the setting of OVL_BASE_FSTYP to the top of the function and use
> > > > [ "$OVL_BASE_FSTYP" == tmpfs ] consistently.
> > >
> > > Actually I'm wondering if we can bring in a parameter to clarify that xfstests need to
> > > build uplying fs base on a underlying fs, don't depend on the "[ -b "$TEST_DEV" ] ||
> > > [ -c "$TEST_DEV" ] || [ "$FSTYP" == tmpfs ]" things. Due to:
> > > 1) overlayfs might not only base on localfs, it can over NFS or something likes it. (right?)
> >
> > No it cannot.
> > The way that xfstests -overlay work is that both upper and lower layers
> > are created on the base fs, therefore only fs supported as upper fs
> > can be tested with -overlay.
> > None of the network fs qualify as valid overlay upper fs.
> > The only other non-blockdev fs besides tmpfs that could be tested
> > with -overlay is virtiofs.
>
> Oh, looks like my memory is a little jumbled, maybe overlayfs can be exported to NFS :)
>

Yes it can.

> >
> > >    If so, how many judgements we need to add at here?
> > > 2) If xfstests can help overlayfs, that means it can help to build other fs (e.g. nfs, cifs,
> > >    ceph, etc) from an underlying fs in one day.
>
> Actually I asked for this ^^ mainly.
>

Ahhh I completely misunderstood what you meant.
It's interesting. I also wanted to improve test coverage of fuse by running
fuse passthrough with xfstests.


> > >
> > > So how about bring in a parameter, maybe USE_UNDERLYING_FS=yes/no(default), or use "BASE_FSTYP"
> > > directly, e.g.
> > > export USE_UNDERLYING_FS=yes
> > > export FSTYP=tmpfs
> >
> > This already exists:
> >
> > export OVERLAY=true
> > export FSTYP=tmpfs
>
> Yes, it's same with ./check -overlay, so how about
>
> # need override
> if [ "$OVERLAY" = "true" -a "$FSTYP" != "overlay" ];then
> ...

That could make sense.
TBH, I never test overlay without ./check -overlay.
I only tried to avoid breaking this config in case people are still
using it. If someone is interested in making changes here they
need to test those non-standard configurations and make sure
they are not broken by these changes.

>
> >
> > means exactly that, but is usually set internally by ./check -overlay
> >
> > I think what you mean is that this should be a helper:
> >
> > _overlay_is_valid_upper_fs()
> > {
> >         local basedev=$1
> >
> >         case $FSTYP in
> >         tmpfs)
> >                 return 0
> >                 ;;
> >         *)
> >                 [ -b "$basedev" ] || [ -c "$basedev" ]
> >                 return $?
> >                 ;;
> >         esac
> > }
>
> Sure, if we don't leave this job to tester (tester makes sure he provide proper TEST_DEV
> and SCRATCH_DEV), we help to judge that :)
>
> ......
>
> Now uplying fs testing in xfstests supports two ways:

I am not a native English speaker myself, but I don't think this term
fits so well to describe what you mean.
Maybe "overlayed" fs sounds too overlayfs oriended, but it could describe
other fs.

> 1) Let testers prepare uplying fs TEST_DEV and SCRATCH_DEV, then set FSTYP=$upfs
> 2) The testers prepare underlying fs TEST_DEV and SCRATCH_DEV, then set FSTYP=$underfs, and
>    run ./check -$upfs ...
>
> Until now, only overlayfs supports both 2 ways (if I don't remember wrong), others (likes
> nfs, cifs, glusterfs, ceph etc) are only support the 1st way.
>
> So I'm thinking if other fs would like to be supported as overlay, we might can help to
> provide:
> 1) A parameter UPLYING_FS to record if we're testing a fs can be uplying fs, e.g:
>         -nfs)           FSTYP=nfs; UPLYING_FS=nfs ;;
>         -glusterfs)     FSTYP=glusterfs; UPLYING_FS=glusterfs ;;
>         -cifs)          FSTYP=cifs; export UPLYING_FS=cifs ;;
>         -overlay)       FSTYP=overlay; export UPLYING_FS=overly ;;
>
> 2) A common _config_override() function, and different _${UPLYING_FS}_config_override()
>    functions.
> 3) After source local.config, check if [ "$UPLYING_FS" != "$FSTYP" ]. If they're equal,
>    then return, else we need underlying things, then run _${UPLYING_FS}_is_valid() to check
>    the TEST_DEV and SCRATCH_DEV are good for ${UPLYING_FS}.
> 4) Do real override.
>
> As overlayfs is the only one supports the 2nd way, so maybe we can let overlay testing
> to be a demonstration. Does that make sense?
>

I think it does make sense, but the complications are in the details,
so I'll need
to look closer at some examples and mainly need some developers or testers
of nfs/cifs/glusterfs to care about this.

I may get around to look at fuse passthrough as the first target and see
what benefits this could bring on top of standard fuse support [1]

Thanks,
Amir.

[1] https://lore.kernel.org/fstests/20210812045950.3190-1-bhumit.attarde01@gmail.com/
Amir Goldstein April 25, 2022, 10:42 a.m. UTC | #6
On Mon, Apr 25, 2022 at 12:31 PM libaokun (A) <libaokun1@huawei.com> wrote:
>
> 在 2022/4/24 20:02, Amir Goldstein 写道:
>
> On Sun, Apr 24, 2022 at 2:21 PM Baokun Li <libaokun1@huawei.com> wrote:
>
> xfstests support overlay+tmpfs
>
> Thanks for this improvement.
> Can you please share the results of ./check -overlay -g auto ?
>
> How many tests ran? notran? failed?
>
> Best if you have those numbers compared to
> overlay+(already supported base fs)
>
> All right, I'll share it after the adaptation is completed.
>
> ```local.config.example
> export FSTYP=tmpfs
> export TEST_DEV=tmpfs_test
> export TEST_DIR=/tmp/test
> export TEST_FS_MOUNT_OPTS="-t tmpfs"
> export SCRATCH_DEV=tmpfs_scratch
> export SCRATCH_MNT=/tmp/scratch
> export MOUNT_OPTIONS="-t tmpfs"
>
> These mount options for tmpfs are very awkward.
> Please fix _overlay_base_mount to use -t $OVL_BASE_FSTYP
> like _test_mount() and _try_scratch_mount() do
>
> Could you be more specific?

--- a/common/overlay
+++ b/common/overlay
@@ -81,7 +81,7 @@ _overlay_base_mount()
                return 1
        fi

-       _mount $* $dev $mnt
+       _mount -t $OVL_BASE_FSTYP $* $dev $mnt
        _idmapped_mount $dev $mnt
 }


Untested.

Thanks,
Amir.
Baokun Li April 25, 2022, 12:59 p.m. UTC | #7
在 2022/4/25 18:42, Amir Goldstein 写道:
> On Mon, Apr 25, 2022 at 12:31 PM libaokun (A) <libaokun1@huawei.com> wrote:
>> 在 2022/4/24 20:02, Amir Goldstein 写道:
>>
>> On Sun, Apr 24, 2022 at 2:21 PM Baokun Li <libaokun1@huawei.com> wrote:
>>
>> xfstests support overlay+tmpfs
>>
>> Thanks for this improvement.
>> Can you please share the results of ./check -overlay -g auto ?
>>
>> How many tests ran? notran? failed?
>>
>> Best if you have those numbers compared to
>> overlay+(already supported base fs)
>>
>> All right, I'll share it after the adaptation is completed.
>>
>> ```local.config.example
>> export FSTYP=tmpfs
>> export TEST_DEV=tmpfs_test
>> export TEST_DIR=/tmp/test
>> export TEST_FS_MOUNT_OPTS="-t tmpfs"
>> export SCRATCH_DEV=tmpfs_scratch
>> export SCRATCH_MNT=/tmp/scratch
>> export MOUNT_OPTIONS="-t tmpfs"
>>
>> These mount options for tmpfs are very awkward.
>> Please fix _overlay_base_mount to use -t $OVL_BASE_FSTYP
>> like _test_mount() and _try_scratch_mount() do
>>
>> Could you be more specific?
> --- a/common/overlay
> +++ b/common/overlay
> @@ -81,7 +81,7 @@ _overlay_base_mount()
>                  return 1
>          fi
>
> -       _mount $* $dev $mnt
> +       _mount -t $OVL_BASE_FSTYP $* $dev $mnt
>          _idmapped_mount $dev $mnt
>   }
>
>
> Untested.
>
> Thanks,
> Amir.
> .

Ok, got it! Thank a milion!
diff mbox series

Patch

diff --git a/common/config b/common/config
index 1033b890..3dc047e8 100644
--- a/common/config
+++ b/common/config
@@ -614,7 +614,7 @@  _overlay_config_override()
 	#    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
 	#    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
 	#    overlayfs base and mount dirs inside base fs mount.
-	[ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || return 0
+	[ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || [ "$FSTYP" == tmpfs ] || return 0
 
 	# Config file may specify base fs type, but we obay -overlay flag
 	[ "$FSTYP" == overlay ] || export OVL_BASE_FSTYP="$FSTYP"
@@ -634,7 +634,7 @@  _overlay_config_override()
 	export TEST_DIR="$OVL_BASE_TEST_DIR/$OVL_MNT"
 	export MOUNT_OPTIONS="$OVERLAY_MOUNT_OPTIONS"
 
-	[ -b "$SCRATCH_DEV" ] || [ -c "$SCRATCH_DEV" ] || return 0
+	[ -b "$SCRATCH_DEV" ] || [ -c "$SCRATCH_DEV" ] || [ "$OVL_BASE_FSTYP" == tmpfs ] || return 0
 
 	# Store original base fs vars
 	export OVL_BASE_SCRATCH_DEV="$SCRATCH_DEV"
diff --git a/common/rc b/common/rc
index 553ae350..12498189 100644
--- a/common/rc
+++ b/common/rc
@@ -834,13 +834,18 @@  _scratch_mkfs()
 	local mkfs_status
 
 	case $FSTYP in
-	nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p|virtiofs)
+	nfs*|cifs|ceph|glusterfs|pvfs2|9p|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
 		_scratch_cleanup_files
 		return $?
 		;;
+	overlay)
+		[ ! "$OVL_BASE_FSTYP" == tmpfs ] || return 0
+		_scratch_cleanup_files
+		return $?
+		;;
 	tmpfs)
 		# do nothing for tmpfs
 		return 0