On Sun, Jul 25, 2021 at 09:21:03PM +0300, Amir Goldstein wrote:
> 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.
Makes sense to me. Then I'll take 'perm' for now.
>
> 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 ;-)
Yeah, naming is always a problem :)
>
> > 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.
Sure, I'll pick it up in next week's update and fix supported fs and add
copyup group.
Thanks,
Eryu
