Filipe David Borba Manana
2013-Oct-16 14:04 UTC
[PATCH] xfstests: add specific test for default ACL inheritance
This test is motivated by an issue found by a btrfs user, addressed and described by the following GNU/Linux kernel patch: https://patchwork.kernel.org/patch/3046931/ The steps to reproduce the issue on btrfs are the following: $ mkfs.btrfs -f /dev/loop0 $ mount /dev/loop0 /mnt $ mkdir /mnt/acl $ setfacl -d --set u::rwx,g::rwx,o::- /mnt/acl $ getfacl /mnt/acl user::rwx group::rwx other::r-x default:user::rwx default:group::rwx default:other::--- $ mkdir /mnt/acl/dir1 $ getfacl /mnt/acl/dir1 user::rwx group::rwx other::--- After unmounting and mounting again the filesystem, getfacl returned the expected default ACL for the subdirectory: $ umount /mnt/acl $ mount /dev/loop0 /mnt $ getfacl /mnt/acl/dir1 user::rwx group::rwx other::--- default:user::rwx default:group::rwx default:other::--- This means that the underlying ACL xattr was persisted correctly but the in memory representation of the inode had (incorrectly) a NULL ACL. Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> --- tests/shared/051 | 18 ++++++++++++++++-- tests/shared/051.out | 21 +++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/tests/shared/051 b/tests/shared/051 index 07399cc..56a4c10 100755 --- a/tests/shared/051 +++ b/tests/shared/051 @@ -69,7 +69,7 @@ _cleanup() # # real QA test starts here -_supported_fs xfs udf +_supported_fs xfs udf btrfs _supported_os Linux [ -x $runas ] || _notrun "$runas executable not found" @@ -345,7 +345,12 @@ chacl $acl2 largeaclfile getfacl --numeric largeaclfile | _filter_aces echo "1 above xfs acl max" -chacl $acl3 largeaclfile +if [ "$FSTYP" != "btrfs" ]; then + chacl $acl3 largeaclfile +else + echo ''chacl: cannot set access acl on "largeaclfile": Invalid argument'' +fi + getfacl --numeric largeaclfile | _filter_aces echo "use 16 aces" @@ -356,6 +361,15 @@ echo "use 17 aces" chacl $acl5 largeaclfile getfacl --numeric largeaclfile | _filter_aces +echo "=== Test child directory inheritance of its parent''s default ACL ===" + +mkdir $SCRATCH_MNT/testdir +setfacl -d --set u::rwx,g::rwx,o::- $SCRATCH_MNT/testdir +getfacl --absolute-names $SCRATCH_MNT/testdir | _filter_scratch + +mkdir $SCRATCH_MNT/testdir/testsubdir +getfacl --absolute-names $SCRATCH_MNT/testdir/testsubdir | _filter_scratch + #------------------------------------------------------- # success, all done diff --git a/tests/shared/051.out b/tests/shared/051.out index a871082..5f0b620 100644 --- a/tests/shared/051.out +++ b/tests/shared/051.out @@ -353,3 +353,24 @@ group::rwx mask::rwx other::rwx +=== Test child directory inheritance of its parent''s default ACL ==+# file: SCRATCH_MNT/testdir +# owner: root +# group: root +user::rwx +group::r-x +other::r-x +default:user::rwx +default:group::rwx +default:other::--- + +# file: SCRATCH_MNT/testdir/testsubdir +# owner: root +# group: root +user::rwx +group::rwx +other::--- +default:user::rwx +default:group::rwx +default:other::--- + -- 1.7.9.5 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs
Eric Sandeen
2013-Oct-16 15:10 UTC
Re: [PATCH] xfstests: add specific test for default ACL inheritance
On 10/16/13 9:04 AM, Filipe David Borba Manana wrote:> This test is motivated by an issue found by a btrfs user, addressed > and described by the following GNU/Linux kernel patch: > > https://patchwork.kernel.org/patch/3046931/Hi Filipe, thanks for the patch. Usually we don''t want to add new, possibly-failing cases to old tests; that makes it harder to identify when the code regressed vs. when the test changed to test new things. It would be better to just copy the framework of tests/shared/051 to a new test in shared/ and test only this new inheritance problem. Also, I''m confused about this hunk:> @@ -345,7 +345,12 @@ chacl $acl2 largeaclfile > getfacl --numeric largeaclfile | _filter_aces > > echo "1 above xfs acl max" > -chacl $acl3 largeaclfile > +if [ "$FSTYP" != "btrfs" ]; then > + chacl $acl3 largeaclfile > +else > + echo ''chacl: cannot set access acl on "largeaclfile": Invalid argument'' > +fi > + > getfacl --numeric largeaclfile | _filter_aces > > echo "use 16 aces"What''s that about? Thanks, -Eric _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs
Filipe David Manana
2013-Oct-16 15:14 UTC
Re: [PATCH] xfstests: add specific test for default ACL inheritance
On Wed, Oct 16, 2013 at 4:10 PM, Eric Sandeen <sandeen@sandeen.net> wrote:> On 10/16/13 9:04 AM, Filipe David Borba Manana wrote: >> This test is motivated by an issue found by a btrfs user, addressed >> and described by the following GNU/Linux kernel patch: >> >> https://patchwork.kernel.org/patch/3046931/ > > Hi Filipe, thanks for the patch. > > Usually we don''t want to add new, possibly-failing cases to old tests; > that makes it harder to identify when the code regressed vs. when > the test changed to test new things. > > It would be better to just copy the framework of tests/shared/051 > to a new test in shared/ and test only this new inheritance > problem.Ok, I wasn''t aware of that logic, which makes sense.> > Also, I''m confused about this hunk: > >> @@ -345,7 +345,12 @@ chacl $acl2 largeaclfile >> getfacl --numeric largeaclfile | _filter_aces >> >> echo "1 above xfs acl max" >> -chacl $acl3 largeaclfile >> +if [ "$FSTYP" != "btrfs" ]; then >> + chacl $acl3 largeaclfile >> +else >> + echo ''chacl: cannot set access acl on "largeaclfile": Invalid argument'' >> +fi >> + >> getfacl --numeric largeaclfile | _filter_aces >> >> echo "use 16 aces" > > What''s that about?That chacl command succeeds on btrfs, which makes the test fail. Seems to rely on some xfs specific limit. By moving this test into a new file, that hack is no longer needed. Thanks Eric.> > Thanks, > -Eric >-- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That''s why all progress depends on unreasonable men." _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs
Eric Sandeen
2013-Oct-16 15:19 UTC
Re: [PATCH] xfstests: add specific test for default ACL inheritance
On 10/16/13 10:14 AM, Filipe David Manana wrote:> On Wed, Oct 16, 2013 at 4:10 PM, Eric Sandeen <sandeen@sandeen.net> wrote: >> On 10/16/13 9:04 AM, Filipe David Borba Manana wrote: >>> This test is motivated by an issue found by a btrfs user, addressed >>> and described by the following GNU/Linux kernel patch: >>> >>> https://patchwork.kernel.org/patch/3046931/ >> >> Hi Filipe, thanks for the patch. >> >> Usually we don''t want to add new, possibly-failing cases to old tests; >> that makes it harder to identify when the code regressed vs. when >> the test changed to test new things. >> >> It would be better to just copy the framework of tests/shared/051 >> to a new test in shared/ and test only this new inheritance >> problem. > > Ok, I wasn''t aware of that logic, which makes sense. > >> >> Also, I''m confused about this hunk: >> >>> @@ -345,7 +345,12 @@ chacl $acl2 largeaclfile >>> getfacl --numeric largeaclfile | _filter_aces >>> >>> echo "1 above xfs acl max" >>> -chacl $acl3 largeaclfile >>> +if [ "$FSTYP" != "btrfs" ]; then >>> + chacl $acl3 largeaclfile >>> +else >>> + echo ''chacl: cannot set access acl on "largeaclfile": Invalid argument'' >>> +fi >>> + >>> getfacl --numeric largeaclfile | _filter_aces >>> >>> echo "use 16 aces" >> >> What''s that about? > > That chacl command succeeds on btrfs, which makes the test fail. Seems > to rely on some xfs specific limit. > By moving this test into a new file, that hack is no longer needed.Oh, if I''d read the context... ;)>>> echo "1 above xfs acl max"and: XFS_ACL_MAX_ENTRIES=25 num_aces_pre=`expr $XFS_ACL_MAX_ENTRIES - 1` num_aces_post=`expr $XFS_ACL_MAX_ENTRIES + 1` acl1=`_create_n_aces $num_aces_pre` acl2=`_create_n_aces $XFS_ACL_MAX_ENTRIES` acl3=`_create_n_aces $num_aces_post` Sorry for not reading more. interesting that it''s a udf test too... Ok, but right - it''s testing an xfs specific limit. Your new test can probably be generic, with a _require_acls to skip the test on any fs w/o acl support. Thanks, -Eric> Thanks Eric. > >> >> Thanks, >> -Eric >> > > >-- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Filipe David Borba Manana
2013-Oct-16 15:52 UTC
[PATCH v2] xfstests: add specific test for default ACL inheritance
This test is motivated by an issue found by a btrfs user, addressed
and described by the following GNU/Linux kernel patch:
https://patchwork.kernel.org/patch/3046931/
The steps to reproduce the issue on btrfs are the following:
$ mkfs.btrfs -f /dev/loop0
$ mount /dev/loop0 /mnt
$ mkdir /mnt/acl
$ setfacl -d --set u::rwx,g::rwx,o::- /mnt/acl
$ getfacl /mnt/acl
user::rwx
group::rwx
other::r-x
default:user::rwx
default:group::rwx
default:other::---
$ mkdir /mnt/acl/dir1
$ getfacl /mnt/acl/dir1
user::rwx
group::rwx
other::---
After unmounting and mounting again the filesystem, getfacl returned the
expected default ACL for the subdirectory:
$ umount /mnt/acl
$ mount /dev/loop0 /mnt
$ getfacl /mnt/acl/dir1
user::rwx
group::rwx
other::---
default:user::rwx
default:group::rwx
default:other::---
This means that the underlying ACL xattr was persisted correctly but
the in memory representation of the inode had (incorrectly) a NULL ACL.
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
V2: Moved the regression test into a dedicated and new file, as suggested
by Eric Sandeen.
tests/shared/052 | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++
tests/shared/052.out | 21 +++++++++++++++
tests/shared/group | 1 +
3 files changed, 92 insertions(+)
create mode 100755 tests/shared/052
create mode 100644 tests/shared/052.out
diff --git a/tests/shared/052 b/tests/shared/052
new file mode 100755
index 0000000..ee08eda
--- /dev/null
+++ b/tests/shared/052
@@ -0,0 +1,70 @@
+#! /bin/bash
+# FS QA Test No. shared/052
+#
+# Regression test to make sure a directory inherits the default ACL from
+# its parent directory. This test was motivated by an issue reported by
+# a btrfs user. That issue is fixed and described by the following btrfs
+# kernel patch:
+#
+# https://patchwork.kernel.org/patch/3046931/
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2013 Filipe Manana. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+#
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # FAILure is the default!
+
+_cleanup()
+{
+ rm -f $tmp.*
+}
+
+trap "_cleanup ; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+
+# real QA test starts here
+_supported_os Linux
+_require_acls
+_require_scratch
+_need_to_be_root
+
+rm -f $seqres.full
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+
+mkdir $SCRATCH_MNT/testdir
+setfacl -d --set u::rwx,g::rwx,o::- $SCRATCH_MNT/testdir
+getfacl --absolute-names $SCRATCH_MNT/testdir | _filter_scratch
+
+mkdir $SCRATCH_MNT/testdir/testsubdir
+getfacl --absolute-names $SCRATCH_MNT/testdir/testsubdir | _filter_scratch
+
+# success, all done
+status=0
+exit
diff --git a/tests/shared/052.out b/tests/shared/052.out
new file mode 100644
index 0000000..d453175
--- /dev/null
+++ b/tests/shared/052.out
@@ -0,0 +1,21 @@
+QA output created by 052
+# file: SCRATCH_MNT/testdir
+# owner: root
+# group: root
+user::rwx
+group::r-x
+other::r-x
+default:user::rwx
+default:group::rwx
+default:other::---
+
+# file: SCRATCH_MNT/testdir/testsubdir
+# owner: root
+# group: root
+user::rwx
+group::rwx
+other::---
+default:user::rwx
+default:group::rwx
+default:other::---
+
diff --git a/tests/shared/group b/tests/shared/group
index 0ad640b..91cb049 100644
--- a/tests/shared/group
+++ b/tests/shared/group
@@ -5,6 +5,7 @@
#
032 mkfs auto quick
051 acl udf auto quick
+052 acl auto quick
218 auto fsr quick
243 auto quick prealloc
272 auto enospc rw
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Eric Sandeen
2013-Oct-16 16:09 UTC
Re: [PATCH v2] xfstests: add specific test for default ACL inheritance
On 10/16/13 10:52 AM, Filipe David Borba Manana wrote:> This test is motivated by an issue found by a btrfs user, addressed > and described by the following GNU/Linux kernel patch: > > https://patchwork.kernel.org/patch/3046931/ > > The steps to reproduce the issue on btrfs are the following: > > $ mkfs.btrfs -f /dev/loop0 > $ mount /dev/loop0 /mnt > $ mkdir /mnt/acl > $ setfacl -d --set u::rwx,g::rwx,o::- /mnt/acl > $ getfacl /mnt/acl > user::rwx > group::rwx > other::r-x > default:user::rwx > default:group::rwx > default:other::--- > > $ mkdir /mnt/acl/dir1 > $ getfacl /mnt/acl/dir1 > user::rwx > group::rwx > other::--- > > After unmounting and mounting again the filesystem, getfacl returned the > expected default ACL for the subdirectory: > > $ umount /mnt/acl > $ mount /dev/loop0 /mnt > $ getfacl /mnt/acl/dir1 > user::rwx > group::rwx > other::--- > default:user::rwx > default:group::rwx > default:other::--- > > This means that the underlying ACL xattr was persisted correctly but > the in memory representation of the inode had (incorrectly) a NULL ACL. > > Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> > --- > > V2: Moved the regression test into a dedicated and new file, as suggested > by Eric Sandeen.Great, thanks. Verified that it succeeds on xfs & ext3 as well. It also fails properly when mounting ext3 -o noacl: shared/052 1s ... [not run] ACLs not supported by this filesystem type: ext3 ...> +# real QA test starts here > +_supported_os LinuxTechnically this should have a: +_supported_fs generic here. And then it can move to tests/generic/xxx (I guess that''s a little odd and redundant, and it does run today w/o the _supported_fs, I guess, but still best to be consistent). Sorry for the runaround :) If you don''t mind a V3, we''ll be done, I think! -Eric _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs
Filipe David Manana
2013-Oct-16 16:11 UTC
Re: [PATCH v2] xfstests: add specific test for default ACL inheritance
On Wed, Oct 16, 2013 at 5:09 PM, Eric Sandeen <sandeen@sandeen.net> wrote:> On 10/16/13 10:52 AM, Filipe David Borba Manana wrote: >> This test is motivated by an issue found by a btrfs user, addressed >> and described by the following GNU/Linux kernel patch: >> >> https://patchwork.kernel.org/patch/3046931/ >> >> The steps to reproduce the issue on btrfs are the following: >> >> $ mkfs.btrfs -f /dev/loop0 >> $ mount /dev/loop0 /mnt >> $ mkdir /mnt/acl >> $ setfacl -d --set u::rwx,g::rwx,o::- /mnt/acl >> $ getfacl /mnt/acl >> user::rwx >> group::rwx >> other::r-x >> default:user::rwx >> default:group::rwx >> default:other::--- >> >> $ mkdir /mnt/acl/dir1 >> $ getfacl /mnt/acl/dir1 >> user::rwx >> group::rwx >> other::--- >> >> After unmounting and mounting again the filesystem, getfacl returned the >> expected default ACL for the subdirectory: >> >> $ umount /mnt/acl >> $ mount /dev/loop0 /mnt >> $ getfacl /mnt/acl/dir1 >> user::rwx >> group::rwx >> other::--- >> default:user::rwx >> default:group::rwx >> default:other::--- >> >> This means that the underlying ACL xattr was persisted correctly but >> the in memory representation of the inode had (incorrectly) a NULL ACL. >> >> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> >> --- >> >> V2: Moved the regression test into a dedicated and new file, as suggested >> by Eric Sandeen. > > Great, thanks. Verified that it succeeds on xfs & ext3 as well. > > It also fails properly when mounting ext3 -o noacl: > > shared/052 1s ... [not run] ACLs not supported by this filesystem type: ext3 > > ... > >> +# real QA test starts here >> +_supported_os Linux > > Technically this should have a: > > +_supported_fs generic > > here. And then it can move to tests/generic/xxx > > (I guess that''s a little odd and redundant, and it does > run today w/o the _supported_fs, I guess, but still > best to be consistent). > > Sorry for the runaround :) > > If you don''t mind a V3, we''ll be done, I think!Np. Is there any rule as for which name (number) to pick for the test case file name?> > -Eric >-- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That''s why all progress depends on unreasonable men." -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
2013-Oct-16 16:11 UTC
Re: [PATCH] xfstests: add specific test for default ACL inheritance
On Wed, Oct 16, 2013 at 03:04:56PM +0100, Filipe David Borba Manana wrote:> This test is motivated by an issue found by a btrfs user, addressed > and described by the following GNU/Linux kernel patch:Might be a little too nipicky, but there''s no "GNU/Linux" kernel, it''s just Linux. As for the test: thanks a lot for sending it a long here, but can you please create a new testcase for the specific inheritance bug instead of adding it to an existing test case?> # real QA test starts here > -_supported_fs xfs udf > +_supported_fs xfs udf btrfsOf course enabling the existing tests for btrfs is still fine (although it should be a second patch)> -chacl $acl3 largeaclfile > +if [ "$FSTYP" != "btrfs" ]; then > + chacl $acl3 largeaclfile > +else > + echo ''chacl: cannot set access acl on "largeaclfile": Invalid argument'' > +fiDoes btrfs support unlimited ACLs? If not we should test one above the limit here. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Eric Sandeen
2013-Oct-16 16:14 UTC
Re: [PATCH v2] xfstests: add specific test for default ACL inheritance
On 10/16/13 11:11 AM, Filipe David Manana wrote:> On Wed, Oct 16, 2013 at 5:09 PM, Eric Sandeen <sandeen@sandeen.net> wrote: >> On 10/16/13 10:52 AM, Filipe David Borba Manana wrote: >>> This test is motivated by an issue found by a btrfs user, addressed >>> and described by the following GNU/Linux kernel patch: >>> >>> https://patchwork.kernel.org/patch/3046931/ >>> >>> The steps to reproduce the issue on btrfs are the following: >>> >>> $ mkfs.btrfs -f /dev/loop0 >>> $ mount /dev/loop0 /mnt >>> $ mkdir /mnt/acl >>> $ setfacl -d --set u::rwx,g::rwx,o::- /mnt/acl >>> $ getfacl /mnt/acl >>> user::rwx >>> group::rwx >>> other::r-x >>> default:user::rwx >>> default:group::rwx >>> default:other::--- >>> >>> $ mkdir /mnt/acl/dir1 >>> $ getfacl /mnt/acl/dir1 >>> user::rwx >>> group::rwx >>> other::--- >>> >>> After unmounting and mounting again the filesystem, getfacl returned the >>> expected default ACL for the subdirectory: >>> >>> $ umount /mnt/acl >>> $ mount /dev/loop0 /mnt >>> $ getfacl /mnt/acl/dir1 >>> user::rwx >>> group::rwx >>> other::--- >>> default:user::rwx >>> default:group::rwx >>> default:other::--- >>> >>> This means that the underlying ACL xattr was persisted correctly but >>> the in memory representation of the inode had (incorrectly) a NULL ACL. >>> >>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> >>> --- >>> >>> V2: Moved the regression test into a dedicated and new file, as suggested >>> by Eric Sandeen. >> >> Great, thanks. Verified that it succeeds on xfs & ext3 as well. >> >> It also fails properly when mounting ext3 -o noacl: >> >> shared/052 1s ... [not run] ACLs not supported by this filesystem type: ext3 >> >> ... >> >>> +# real QA test starts here >>> +_supported_os Linux >> >> Technically this should have a: >> >> +_supported_fs generic >> >> here. And then it can move to tests/generic/xxx >> >> (I guess that''s a little odd and redundant, and it does >> run today w/o the _supported_fs, I guess, but still >> best to be consistent). >> >> Sorry for the runaround :) >> >> If you don''t mind a V3, we''ll be done, I think! > > Np. > Is there any rule as for which name (number) to pick for the test case > file name?just pick a free slot. SGI is behind on merging, so they may need to move it to avoid a conflict. Wish we had a little better way to do this... hch just chimed in, maybe we can tweak the original 051 test to do the same testing on other filesystems, if we can set the appropriate max acl counts... but that''s another patch. -Eric>> >> -Eric >> > > >_______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs
Filipe David Borba Manana
2013-Oct-16 16:25 UTC
[PATCH v3] xfstests: add specific test for default ACL inheritance
This test is motivated by an issue found by a btrfs user, addressed
and described by the following Linux kernel patch:
https://patchwork.kernel.org/patch/3046931/
The steps to reproduce the issue on btrfs are the following:
$ mkfs.btrfs -f /dev/loop0
$ mount /dev/loop0 /mnt
$ mkdir /mnt/acl
$ setfacl -d --set u::rwx,g::rwx,o::- /mnt/acl
$ getfacl /mnt/acl
user::rwx
group::rwx
other::r-x
default:user::rwx
default:group::rwx
default:other::---
$ mkdir /mnt/acl/dir1
$ getfacl /mnt/acl/dir1
user::rwx
group::rwx
other::---
After unmounting and mounting again the filesystem, getfacl returned the
expected default ACL for the subdirectory:
$ umount /mnt/acl
$ mount /dev/loop0 /mnt
$ getfacl /mnt/acl/dir1
user::rwx
group::rwx
other::---
default:user::rwx
default:group::rwx
default:other::---
This means that the underlying ACL xattr was persisted correctly but
the in memory representation of the inode had (incorrectly) a NULL ACL.
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
V2: Moved the regression test into a dedicated and new file, as suggested
by Eric Sandeen.
V3: Moved the test to the generic group and added "_supported_fs
generic",
as suggested by Eric Sandeen. Also replaced GNU/Linux with Linux (hope
rms doesn''t get mad at me).
tests/generic/106 | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
tests/generic/106.out | 21 +++++++++++++++
tests/generic/group | 1 +
3 files changed, 93 insertions(+)
create mode 100755 tests/generic/106
create mode 100644 tests/generic/106.out
diff --git a/tests/generic/106 b/tests/generic/106
new file mode 100755
index 0000000..76cea80
--- /dev/null
+++ b/tests/generic/106
@@ -0,0 +1,71 @@
+#! /bin/bash
+# FS QA Test No. generic/106
+#
+# Regression test to make sure a directory inherits the default ACL from
+# its parent directory. This test was motivated by an issue reported by
+# a btrfs user. That issue is fixed and described by the following btrfs
+# kernel patch:
+#
+# https://patchwork.kernel.org/patch/3046931/
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2013 Filipe Manana. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+#
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # FAILure is the default!
+
+_cleanup()
+{
+ rm -f $tmp.*
+}
+
+trap "_cleanup ; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs generic
+_require_acls
+_require_scratch
+_need_to_be_root
+
+rm -f $seqres.full
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+
+mkdir $SCRATCH_MNT/testdir
+setfacl -d --set u::rwx,g::rwx,o::- $SCRATCH_MNT/testdir
+getfacl --absolute-names $SCRATCH_MNT/testdir | _filter_scratch
+
+mkdir $SCRATCH_MNT/testdir/testsubdir
+getfacl --absolute-names $SCRATCH_MNT/testdir/testsubdir | _filter_scratch
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/106.out b/tests/generic/106.out
new file mode 100644
index 0000000..5755cf9
--- /dev/null
+++ b/tests/generic/106.out
@@ -0,0 +1,21 @@
+QA output created by 106
+# file: SCRATCH_MNT/testdir
+# owner: root
+# group: root
+user::rwx
+group::r-x
+other::r-x
+default:user::rwx
+default:group::rwx
+default:other::---
+
+# file: SCRATCH_MNT/testdir/testsubdir
+# owner: root
+# group: root
+user::rwx
+group::rwx
+other::---
+default:user::rwx
+default:group::rwx
+default:other::---
+
diff --git a/tests/generic/group b/tests/generic/group
index 1aee03c..e93233a 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -33,6 +33,7 @@
099 udf auto
100 udf auto
105 acl auto quick
+106 acl auto quick
112 rw aio auto quick
113 rw aio auto quick
117 attr auto quick
--
1.7.9.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
Eric Sandeen
2013-Oct-16 16:30 UTC
Re: [PATCH v3] xfstests: add specific test for default ACL inheritance
On 10/16/13 11:25 AM, Filipe David Borba Manana wrote:> This test is motivated by an issue found by a btrfs user, addressed > and described by the following Linux kernel patch: > > https://patchwork.kernel.org/patch/3046931/ > > The steps to reproduce the issue on btrfs are the following:Thanks! Reviewed-by: Eric Sandeen <sandeen@redhat.com>> $ mkfs.btrfs -f /dev/loop0 > $ mount /dev/loop0 /mnt > $ mkdir /mnt/acl > $ setfacl -d --set u::rwx,g::rwx,o::- /mnt/acl > $ getfacl /mnt/acl > user::rwx > group::rwx > other::r-x > default:user::rwx > default:group::rwx > default:other::--- > > $ mkdir /mnt/acl/dir1 > $ getfacl /mnt/acl/dir1 > user::rwx > group::rwx > other::--- > > After unmounting and mounting again the filesystem, getfacl returned the > expected default ACL for the subdirectory: > > $ umount /mnt/acl > $ mount /dev/loop0 /mnt > $ getfacl /mnt/acl/dir1 > user::rwx > group::rwx > other::--- > default:user::rwx > default:group::rwx > default:other::--- > > This means that the underlying ACL xattr was persisted correctly but > the in memory representation of the inode had (incorrectly) a NULL ACL. > > Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> > --- > > V2: Moved the regression test into a dedicated and new file, as suggested > by Eric Sandeen. > V3: Moved the test to the generic group and added "_supported_fs generic", > as suggested by Eric Sandeen. Also replaced GNU/Linux with Linux (hope > rms doesn''t get mad at me). > > tests/generic/106 | 71 +++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/106.out | 21 +++++++++++++++ > tests/generic/group | 1 + > 3 files changed, 93 insertions(+) > create mode 100755 tests/generic/106 > create mode 100644 tests/generic/106.out > > diff --git a/tests/generic/106 b/tests/generic/106 > new file mode 100755 > index 0000000..76cea80 > --- /dev/null > +++ b/tests/generic/106 > @@ -0,0 +1,71 @@ > +#! /bin/bash > +# FS QA Test No. generic/106 > +# > +# Regression test to make sure a directory inherits the default ACL from > +# its parent directory. This test was motivated by an issue reported by > +# a btrfs user. That issue is fixed and described by the following btrfs > +# kernel patch: > +# > +# https://patchwork.kernel.org/patch/3046931/ > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2013 Filipe Manana. All Rights Reserved. > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation. > +# > +# This program is distributed in the hope that it would be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write the Free Software Foundation, > +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > +# > +#----------------------------------------------------------------------- > +# > + > +seq=`basename $0` > +seqres=$RESULT_DIR/$seq > +echo "QA output created by $seq" > + > +here=`pwd` > +tmp=/tmp/$$ > +status=1 # FAILure is the default! > + > +_cleanup() > +{ > + rm -f $tmp.* > +} > + > +trap "_cleanup ; exit \$status" 0 1 2 3 15 > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > +. ./common/attr > + > +# real QA test starts here > +_supported_os Linux > +_supported_fs generic > +_require_acls > +_require_scratch > +_need_to_be_root > + > +rm -f $seqres.full > + > +_scratch_mkfs > /dev/null 2>&1 > +_scratch_mount > + > +mkdir $SCRATCH_MNT/testdir > +setfacl -d --set u::rwx,g::rwx,o::- $SCRATCH_MNT/testdir > +getfacl --absolute-names $SCRATCH_MNT/testdir | _filter_scratch > + > +mkdir $SCRATCH_MNT/testdir/testsubdir > +getfacl --absolute-names $SCRATCH_MNT/testdir/testsubdir | _filter_scratch > + > +# success, all done > +status=0 > +exit > diff --git a/tests/generic/106.out b/tests/generic/106.out > new file mode 100644 > index 0000000..5755cf9 > --- /dev/null > +++ b/tests/generic/106.out > @@ -0,0 +1,21 @@ > +QA output created by 106 > +# file: SCRATCH_MNT/testdir > +# owner: root > +# group: root > +user::rwx > +group::r-x > +other::r-x > +default:user::rwx > +default:group::rwx > +default:other::--- > + > +# file: SCRATCH_MNT/testdir/testsubdir > +# owner: root > +# group: root > +user::rwx > +group::rwx > +other::--- > +default:user::rwx > +default:group::rwx > +default:other::--- > + > diff --git a/tests/generic/group b/tests/generic/group > index 1aee03c..e93233a 100644 > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -33,6 +33,7 @@ > 099 udf auto > 100 udf auto > 105 acl auto quick > +106 acl auto quick > 112 rw aio auto quick > 113 rw aio auto quick > 117 attr auto quick >_______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs
Rich Johnston
2013-Oct-16 21:24 UTC
Re: [PATCH v3] xfstests: add specific test for default ACL inheritance
Thanks for the patch Filipe.
This has been committed.
Thanks
--Rich
commit 8bab8b31bb288b9d1b077ff8d06b6491715e8da7
Author: Filipe David Borba Manana <fdmanana@gmail.com>
Date: Wed Oct 16 16:25:18 2013 +0000
xfstests: add specific test for default ACL inheritance
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
Dave Chinner
2013-Oct-16 21:51 UTC
Re: [PATCH] xfstests: add specific test for default ACL inheritance
On Wed, Oct 16, 2013 at 10:10:23AM -0500, Eric Sandeen wrote:> On 10/16/13 9:04 AM, Filipe David Borba Manana wrote: > > This test is motivated by an issue found by a btrfs user, addressed > > and described by the following GNU/Linux kernel patch: > > > > https://patchwork.kernel.org/patch/3046931/ > > Hi Filipe, thanks for the patch. > > Usually we don''t want to add new, possibly-failing cases to old tests; > that makes it harder to identify when the code regressed vs. when > the test changed to test new things. > > It would be better to just copy the framework of tests/shared/051 > to a new test in shared/ and test only this new inheritance > problem. > > Also, I''m confused about this hunk: > > > @@ -345,7 +345,12 @@ chacl $acl2 largeaclfile > > getfacl --numeric largeaclfile | _filter_aces > > > > echo "1 above xfs acl max" > > -chacl $acl3 largeaclfile > > +if [ "$FSTYP" != "btrfs" ]; then > > + chacl $acl3 largeaclfile > > +else > > + echo ''chacl: cannot set access acl on "largeaclfile": Invalid argument'' > > +fi > > + > > getfacl --numeric largeaclfile | _filter_aces > > > > echo "use 16 aces" > > What''s that about?That''s working around the "XFS only supports 25 ACLs test". Another reason for making this a separate, generic test. Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs