[PATCH] xfstests: add specific test for default ACL inheritance

[PATCH] xfstests: add specific test for default ACL inheritance

[Filipe David Borba Manana]

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

Re: [PATCH] xfstests: add specific test for default ACL inheritance

[Eric Sandeen]

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

Re: [PATCH] xfstests: add specific test for default ACL inheritance

[Filipe David Manana]

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."

Re: [PATCH] xfstests: add specific test for default ACL inheritance

[Eric Sandeen]

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
>>
> 
> 
> 

Re: [PATCH] xfstests: add specific test for default ACL inheritance

[Dave Chinner]

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

[PATCH v2] xfstests: add specific test for default ACL inheritance

[Filipe David Borba Manana]

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

Re: [PATCH v2] xfstests: add specific test for default ACL inheritance

[Eric Sandeen]

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!

-Eric

Re: [PATCH v2] xfstests: add specific test for default ACL inheritance

[Filipe David Manana]

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."

Re: [PATCH v2] xfstests: add specific test for default ACL inheritance

[Eric Sandeen]

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
>>
> 
> 
> 

Re: [PATCH] xfstests: add specific test for default ACL inheritance

[Christoph Hellwig]

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 btrfs

Of 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'
> +fi

Does btrfs support unlimited ACLs?  If not we should test one above the
limit here.

[PATCH v3] xfstests: add specific test for default ACL inheritance

[Filipe David Borba Manana]

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

Re: [PATCH v3] xfstests: add specific test for default ACL inheritance

[Eric Sandeen]

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
> 

Re: [PATCH v3] xfstests: add specific test for default ACL inheritance

[Rich Johnston]

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