Re: [RFC] common: overlay support tmpfs

[Amir Goldstein <amir73il@gmail.com>]

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/