On Sun, Jul 25, 2021 at 7:27 PM Eryu Guan <guan@xxxxxxx> wrote:
>
> On Thu, Jul 22, 2021 at 07:46:34PM +0300, Amir Goldstein wrote:
> > Overlayfs copies up a subset of lower file attributes since kernel
> > commits:
> > 173ff5c9ec37 ("ovl: consistent behavior for immutable/append-only inodes")
> > 2e3f6e87c2b0 ("ovl: copy up sync/noatime fileattr flags")
> >
> > This test verifies this functionality works correctly and that it
> > survives power failure and/or mount cycle.
> >
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>
> Looks good to me overall, just one minor question below.
>
> > ---
> >
> > Eryu,
> >
> > This test is failing on master and passes on overlayfs-next.
> >
> > Thanks,
> > Amir.
> >
> > tests/overlay/078 | 145 ++++++++++++++++++++++++++++++++++++++++++
> > tests/overlay/078.out | 2 +
> > 2 files changed, 147 insertions(+)
> > create mode 100755 tests/overlay/078
> > create mode 100644 tests/overlay/078.out
> >
> > diff --git a/tests/overlay/078 b/tests/overlay/078
> > new file mode 100755
> > index 00000000..b43449d1
> > --- /dev/null
> > +++ b/tests/overlay/078
> > @@ -0,0 +1,145 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2018 Huawei. All Rights Reserved.
> > +# Copyright (C) 2021 CTERA Networks. All Rights Reserved.
> > +#
> > +# FS QA Test 078
> > +#
> > +# Test copy up of lower file attributes.
> > +#
> > +# Overlayfs copies up a subset of lower file attributes since kernel commits:
> > +# 173ff5c9ec37 ("ovl: consistent behavior for immutable/append-only inodes")
> > +# 2e3f6e87c2b0 ("ovl: copy up sync/noatime fileattr flags")
> > +#
> > +# This test is similar and was derived from generic/507, but instead
> > +# of creating new files which are created in upper layer, prepare
> > +# the file with attributes in lower layer and verify that attributes
> > +# are not lost during copy up, (optional) shutdown and mount cycle.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick perms shutdown
>
> I noticed that generic/507 has the same groups defined, but I'm
> wondering if 'perms' is right group, 'attr' seems a better fit to me.
The term "attr" is now very much overloaded in filesystems.
It may refer to info of stat() (i.e. getattr())
it my refer to xattr and it may refer to lsattr/chattr,
which is referred to as fileattr in latest vfs API.
In fstests, most of the tests in the 'attr' group include
./common/attr which refers to ACL xattrs in particular...
so it is not a good fit IMO.
The group 'perm' is already used for overlayfs tests that deal with
immutable files and generic tests that deal with immutable files
don't really have a common group AFAICT.
I have no objection for creating a new group for this
purpose but that would involve marking all related tests and worse...
...finding a name for that group ;-)
> And we could add 'copyup' group as well.
>
Sure.
> > +
> > +# Override the default cleanup function.
> > +_cleanup()
> > +{
> > + cd /
> > + $CHATTR_PROG -ai $lowertestfile &> /dev/null
> > + rm -f $tmp.*
> > +}
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_fs generic
>
> s/generic/overlay/
Oops. I assume you would be fix this typo on commit.
Thanks,
Amir.
