[PATCH v2] xfstests, generic: add project quota attribute tests

[PATCH v2] xfstests, generic: add project quota attribute tests

[Wang Shilong]

From: Wang Shilong <wshilong@ddn.com>

lsattr/chattr support both of ext4 and xfs, add
a test to cover both of them.

 1. ioctl with project quota.
 2. project inherit attribute.
 3. Link accross project should fail
 4. change project ignores quota

Signed-off-by: Wang Shilong <wshilong@ddn.com>
---
 tests/generic/362     | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/362.out |   8 ++++
 tests/generic/group   |   1 +
 3 files changed, 139 insertions(+)
 create mode 100755 tests/generic/362
 create mode 100644 tests/generic/362.out

diff --git a/tests/generic/362 b/tests/generic/362
new file mode 100755
index 0000000..f763bc5
--- /dev/null
+++ b/tests/generic/362
@@ -0,0 +1,130 @@
+#! /bin/bash
+# FS QA Test No. 030
+#
+# Test Project quota attr function
+#
+#-----------------------------------------------------------------------
+# Copyright 2016 (C) Wang Shilong <wshilong@ddn.com>
+#
+# 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
+#
+#-----------------------------------------------------------------------
+#
+
+seqfull=$0
+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
+. ./common/quota
+
+# real QA test starts here
+_supported_fs ext4 xfs
+_supported_os Linux
+
+_require_scratch
+_require_chattr
+_require_test_lsattr
+_require_quota
+
+rm -f $seqres.full
+MKFSOPTIONS=""
+MOUNTOPTIONS=""
+
+function setup_quota_options()
+{
+    case $FSTYP in
+    xfs)
+	#quotaon rely on this.
+	MOUNTOPTIONS="-o pquota"
+	;;
+    ext4)
+	#project quota is disabled in default.
+	MKFSOPTIONS="-O quota,project"
+	;;
+    *)
+        ;;
+    esac
+
+}
+
+function set_inherit_attribute()
+{
+    case $FSTYP in
+    xfs)
+	$XFS_IO_PROG -r -c 'chattr +P' $*
+	;;
+    ext4)
+	chattr +P $*
+	;;
+    *)
+        ;;
+    esac
+}
+
+setup_quota_options
+
+echo "+ create scratch fs"
+_scratch_mkfs $MKFSOPTIONS > /dev/null 2>&1
+
+echo "+ mount fs image"
+_scratch_mount $MOUNTOPTIONS
+
+function attr_test()
+{
+	#default project without inherit
+	mkdir $SCRATCH_MNT/dir
+	chattr -p 123456 $SCRATCH_MNT/dir | _filter_scratch | _filter_spaces
+	lsattr -p $SCRATCH_MNT/dir | _filter_scratch | $AWK_PROG '{print $1" "$3}'
+	touch $SCRATCH_MNT/dir/foo
+	lsattr -p $SCRATCH_MNT/dir/foo | _filter_scratch | $AWK_PROG '{print $1" "$3}'
+
+	#test project inherit with inherit attribute
+	set_inherit_attribute $SCRATCH_MNT/dir
+	touch $SCRATCH_MNT/dir/foo1
+	lsattr -p $SCRATCH_MNT/dir/foo1 | _filter_scratch | $AWK_PROG '{print $1" "$3}'
+
+	#Link accross project should fail
+	mkdir $SCRATCH_MNT/dir1
+	set_inherit_attribute $SCRATCH_MNT/dir1
+	chattr -p 654321 $SCRATCH_MNT/dir1 | _filter_scratch | _filter_spaces
+	ln $SCRATCH_MNT/dir/foo1 $SCRATCH_MNT/dir1/foo1 2>&1 \
+		| _filter_scratch | _filter_spaces
+
+	#mv accross different projects should work
+	mv $SCRATCH_MNT/dir/foo1 $SCRATCH_MNT/dir1/foo1
+	lsattr -p $SCRATCH_MNT/dir1/foo1 | _filter_scratch | $AWK_PROG '{print $1" "$3}'
+
+	#change project ignores quota
+	quotaon $SCRATCH_MNT >/dev/null 2>&1
+	setquota -P 654321 0 0 0 1 $SCRATCH_MNT/
+	chattr -p 123456 $SCRATCH_MNT/dir1/foo1 | _filter_scratch | _filter_spaces
+	lsattr -p $SCRATCH_MNT/dir1/foo1 | _filter_scratch | $AWK_PROG '{print $1" "$3}'
+}
+attr_test
diff --git a/tests/generic/362.out b/tests/generic/362.out
new file mode 100644
index 0000000..31db991
--- /dev/null
+++ b/tests/generic/362.out
@@ -0,0 +1,8 @@
+QA output created by 362
++ create scratch fs
++ mount fs image
+0 SCRATCH_MNT/dir/foo
+123456 SCRATCH_MNT/dir/foo1
+ln: failed to create hard link 'SCRATCH_MNT/dir1/foo1' => 'SCRATCH_MNT/dir/foo1': Invalid cross-device link
+654321 SCRATCH_MNT/dir1/foo1
+123456 SCRATCH_MNT/dir1/foo1
diff --git a/tests/generic/group b/tests/generic/group
index 7491282..e834762 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -364,3 +364,4 @@
 359 auto quick clone
 360 auto quick metadata
 361 auto quick
+362 auto quick
-- 
2.7.4

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Eric Sandeen]

On 7/6/16 1:22 AM, Wang Shilong wrote:
> From: Wang Shilong <wshilong@ddn.com>
> 
> lsattr/chattr support both of ext4 and xfs, add
> a test to cover both of them.

Thanks for making this generic; some comments below.
 
>  1. ioctl with project quota.
>  2. project inherit attribute.
>  3. Link accross project should fail

s/accross/across/ FWIW :)

>  4. change project ignores quota
> 
> Signed-off-by: Wang Shilong <wshilong@ddn.com>
> ---
>  tests/generic/362     | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/362.out |   8 ++++
>  tests/generic/group   |   1 +
>  3 files changed, 139 insertions(+)
>  create mode 100755 tests/generic/362
>  create mode 100644 tests/generic/362.out
> 
> diff --git a/tests/generic/362 b/tests/generic/362
> new file mode 100755
> index 0000000..f763bc5
> --- /dev/null
> +++ b/tests/generic/362
> @@ -0,0 +1,130 @@
> +#! /bin/bash
> +# FS QA Test No. 030
> +#
> +# Test Project quota attr function
> +#
> +#-----------------------------------------------------------------------
> +# Copyright 2016 (C) Wang Shilong <wshilong@ddn.com>
> +#
> +# 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
> +#
> +#-----------------------------------------------------------------------
> +#
> +
> +seqfull=$0
> +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
> +. ./common/quota
> +
> +# real QA test starts here
> +_supported_fs ext4 xfs
> +_supported_os Linux
> +
> +_require_scratch
> +_require_chattr
> +_require_test_lsattr
> +_require_quota

We need a _require test to see if the generic quota tools
support project quota, or this will fail due to lack of
userspace support:

+setquota: invalid option -- 'P'
+setquota: Usage:
+  setquota [-u|-g] [-rm] [-F quotaformat] <user|group>
...

same for chattr & lsattr:

+Usage: chattr [-RVf] [-+=AacDdeijsSu] [-v version] files...
...
+lsattr: invalid option -- 'p'

If any of these don't work, the test should _notrun with
a short explanation about the requirement; see more below.

> +
> +rm -f $seqres.full
> +MKFSOPTIONS=""
> +MOUNTOPTIONS=""
> +
> +function setup_quota_options()
> +{
> +    case $FSTYP in
> +    xfs)
> +	#quotaon rely on this.
> +	MOUNTOPTIONS="-o pquota"
> +	;;
> +    ext4)
> +	#project quota is disabled in default.
> +	MKFSOPTIONS="-O quota,project"
> +	;;

Ok, that explains that mystery, I wasn't sure how to enable
project quota on ext4.

(but I'm curious, why doesn't ext4 have a pquota mount option to
match its grpquota and usrquota mount options?  Seems like strange
asymmetry)

But this will also _require a check to be sure mke2fs understands
the "-O project" option, and _notrun if it doesn't.

I think this could all be wrapped up in a "_require_vfs_project_quota"
test, which tests:

1) linux-quota userspace support
2) e2fsprogs userspace support
3) kernel support for the filesystem being tested.

(if the filesystem doesn't support it in kernelspace, we get stuff like
+mount: wrong fs type, bad option, bad superblock on /dev/sdb2,
+       missing codepage or helper program, or other error
+       In some cases useful info is found in syslog - try
+       dmesg | tail  or so
+
+chattr: Inappropriate ioctl for device while setting project on /mnt/scratch/dir
+lsattr: Inappropriate ioctl for device While reading project on /mnt/scratch/dir/foo)

> +    *)
> +        ;;
> +    esac
> +
> +}
> +
> +function set_inherit_attribute()

I don't think this is really needed, either xfs_io or chattr
should work, assuming it's new enough.

Just using chattr should suffice for all filesystems; the test
uses lsattr directly later, so using chattr directly as well makes
more sense to me.

> +{
> +    case $FSTYP in
> +    xfs)
> +	$XFS_IO_PROG -r -c 'chattr +P' $*

(why "-r" ?)

> +	;;
> +    ext4)
> +	chattr +P $*
> +	;;
> +    *)
> +        ;;
> +    esac
> +}
> +
> +setup_quota_options
> +
> +echo "+ create scratch fs"
> +_scratch_mkfs $MKFSOPTIONS > /dev/null 2>&1
> +
> +echo "+ mount fs image"
> +_scratch_mount $MOUNTOPTIONS
> +
> +function attr_test()
> +{
> +	#default project without inherit
> +	mkdir $SCRATCH_MNT/dir
> +	chattr -p 123456 $SCRATCH_MNT/dir | _filter_scratch | _filter_spaces
> +	lsattr -p $SCRATCH_MNT/dir | _filter_scratch | $AWK_PROG '{print $1" "$3}'
> +	touch $SCRATCH_MNT/dir/foo
> +	lsattr -p $SCRATCH_MNT/dir/foo | _filter_scratch | $AWK_PROG '{print $1" "$3}'
> +
> +	#test project inherit with inherit attribute
> +	set_inherit_attribute $SCRATCH_MNT/dir
> +	touch $SCRATCH_MNT/dir/foo1
> +	lsattr -p $SCRATCH_MNT/dir/foo1 | _filter_scratch | $AWK_PROG '{print $1" "$3}'
> +
> +	#Link accross project should fail

s/accross/across :)

> +	mkdir $SCRATCH_MNT/dir1
> +	set_inherit_attribute $SCRATCH_MNT/dir1
> +	chattr -p 654321 $SCRATCH_MNT/dir1 | _filter_scratch | _filter_spaces
> +	ln $SCRATCH_MNT/dir/foo1 $SCRATCH_MNT/dir1/foo1 2>&1 \
> +		| _filter_scratch | _filter_spaces

ln output seems to have changed at some point, so there needs to be some filtering
or replacement on the ln error:

    -ln: failed to create hard link 'SCRATCH_MNT/dir1/foo1' => 'SCRATCH_MNT/dir/foo1': Invalid cross-device link
    +ln: creating hard link 'SCRATCH_MNT/dir1/foo1' => 'SCRATCH_MNT/dir/foo1': Invalid cross-device link

> +
> +	#mv accross different projects should work

s/accross/across :)

> +	mv $SCRATCH_MNT/dir/foo1 $SCRATCH_MNT/dir1/foo1
> +	lsattr -p $SCRATCH_MNT/dir1/foo1 | _filter_scratch | $AWK_PROG '{print $1" "$3}'
> +
> +	#change project ignores quota
> +	quotaon $SCRATCH_MNT >/dev/null 2>&1
> +	setquota -P 654321 0 0 0 1 $SCRATCH_MNT/
> +	chattr -p 123456 $SCRATCH_MNT/dir1/foo1 | _filter_scratch | _filter_spaces
> +	lsattr -p $SCRATCH_MNT/dir1/foo1 | _filter_scratch | $AWK_PROG '{print $1" "$3}'
> +}
> +attr_test
> diff --git a/tests/generic/362.out b/tests/generic/362.out
> new file mode 100644
> index 0000000..31db991
> --- /dev/null
> +++ b/tests/generic/362.out
> @@ -0,0 +1,8 @@
> +QA output created by 362
> ++ create scratch fs
> ++ mount fs image
> +0 SCRATCH_MNT/dir/foo
> +123456 SCRATCH_MNT/dir/foo1
> +ln: failed to create hard link 'SCRATCH_MNT/dir1/foo1' => 'SCRATCH_MNT/dir/foo1': Invalid cross-device link
> +654321 SCRATCH_MNT/dir1/foo1
> +123456 SCRATCH_MNT/dir1/foo1
> diff --git a/tests/generic/group b/tests/generic/group
> index 7491282..e834762 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -364,3 +364,4 @@
>  359 auto quick clone
>  360 auto quick metadata
>  361 auto quick
> +362 auto quick
> 

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Dave Chinner]

On Wed, Jul 06, 2016 at 03:22:51PM +0900, Wang Shilong wrote:
> From: Wang Shilong <wshilong@ddn.com>
> 
> lsattr/chattr support both of ext4 and xfs, add
> a test to cover both of them.
> 
>  1. ioctl with project quota.
>  2. project inherit attribute.
>  3. Link accross project should fail
>  4. change project ignores quota
> 
> Signed-off-by: Wang Shilong <wshilong@ddn.com>

FWIW, this test already points out a few problems with the ext4
project quota implementation....

> +#
> +# Test Project quota attr function
> +#
> +#-----------------------------------------------------------------------
> +# Copyright 2016 (C) Wang Shilong <wshilong@ddn.com>

Ambiguous copyright statement. Is it your personal copyright, or
should it be DDN, the company you work for? If it's a personal
copyright, please use your personal email address, not the email
address supllied by the company you work for. If it's DDN that
owns the copyright, then please put DDN's name here along with your
DDN email address.


> +# real QA test starts here
> +_supported_fs ext4 xfs
> +_supported_os Linux

Generic tests shouldn't need a _supported_fs line - the filesystems
it runs on should be selected by the _requires*  functions below.

> +_require_scratch
> +_require_chattr
> +_require_test_lsattr
> +_require_quota

needs  _require_prjquota, and that function needs to be modified to
detect for both XFS and ext4 support.

> +
> +rm -f $seqres.full
> +MKFSOPTIONS=""
> +MOUNTOPTIONS=""

This doesn't seem appropriate. We really want to test project quotas
under all the configurations that come in from the environment. e.g.
for different block sizes.

> +function setup_quota_options()
> +{
> +    case $FSTYP in
> +    xfs)
> +	#quotaon rely on this.
> +	MOUNTOPTIONS="-o pquota"
> +	;;
> +    ext4)
> +	#project quota is disabled in default.
> +	MKFSOPTIONS="-O quota,project"
> +	;;
> +    *)
> +        ;;
> +    esac
> +
> +}

Oh, I see now. Really, ext4 needs to support the pquota mount
option, just like all the other filesystem quotas are configured.
Please fix that ext4 kernel bug, not hack around it in the test
harnesses.

> +function set_inherit_attribute()
> +{
> +    case $FSTYP in
> +    xfs)
> +	$XFS_IO_PROG -r -c 'chattr +P' $*
> +	;;
> +    ext4)
> +	chattr +P $*
> +	;;
> +    *)
> +        ;;
> +    esac
> +}

Don't the two programs use exactly the same ioctl interface to set
the project attribute?  If not, then that's a deficiency in the ext4
project quota implementation that needs fixing. If there's some
other reason for xfs_io not working on ext4, then that needs fixing.
Again, don't work around implementation problems in the test harness.

> +setup_quota_options
> +
> +echo "+ create scratch fs"
> +_scratch_mkfs $MKFSOPTIONS > /dev/null 2>&1
> +
> +echo "+ mount fs image"
> +_scratch_mount $MOUNTOPTIONS

Once everything is done via mount option, then we can use the
standard _qmount_option function for setting the quota type, and
_qmount for mounting with quotas enabled according to
_qmount_option.


> +function attr_test()
> +{
> +	#default project without inherit
> +	mkdir $SCRATCH_MNT/dir
> +	chattr -p 123456 $SCRATCH_MNT/dir | _filter_scratch | _filter_spaces
> +	lsattr -p $SCRATCH_MNT/dir | _filter_scratch | $AWK_PROG '{print $1" "$3}'
> +	touch $SCRATCH_MNT/dir/foo
> +	lsattr -p $SCRATCH_MNT/dir/foo | _filter_scratch | $AWK_PROG '{print $1" "$3}'
> +
> +	#test project inherit with inherit attribute
> +	set_inherit_attribute $SCRATCH_MNT/dir
> +	touch $SCRATCH_MNT/dir/foo1
> +	lsattr -p $SCRATCH_MNT/dir/foo1 | _filter_scratch | $AWK_PROG '{print $1" "$3}'
> +
> +	#Link accross project should fail
> +	mkdir $SCRATCH_MNT/dir1
> +	set_inherit_attribute $SCRATCH_MNT/dir1
> +	chattr -p 654321 $SCRATCH_MNT/dir1 | _filter_scratch | _filter_spaces
> +	ln $SCRATCH_MNT/dir/foo1 $SCRATCH_MNT/dir1/foo1 2>&1 \
> +		| _filter_scratch | _filter_spaces
> +
> +	#mv accross different projects should work
> +	mv $SCRATCH_MNT/dir/foo1 $SCRATCH_MNT/dir1/foo1
> +	lsattr -p $SCRATCH_MNT/dir1/foo1 | _filter_scratch | $AWK_PROG '{print $1" "$3}'
> +
> +	#change project ignores quota
> +	quotaon $SCRATCH_MNT >/dev/null 2>&1
> +	setquota -P 654321 0 0 0 1 $SCRATCH_MNT/
> +	chattr -p 123456 $SCRATCH_MNT/dir1/foo1 | _filter_scratch | _filter_spaces
> +	lsattr -p $SCRATCH_MNT/dir1/foo1 | _filter_scratch | $AWK_PROG '{print $1" "$3}'
> +}
> +attr_test

We don't need this encapsulated in a function.

Also, there is nothing to set status=0 before exit, so this test
will always fail due to the exit code being 1.

> diff --git a/tests/generic/362.out b/tests/generic/362.out
> new file mode 100644
> index 0000000..31db991
> --- /dev/null
> +++ b/tests/generic/362.out
> @@ -0,0 +1,8 @@
> +QA output created by 362
> ++ create scratch fs
> ++ mount fs image
> +0 SCRATCH_MNT/dir/foo
> +123456 SCRATCH_MNT/dir/foo1
> +ln: failed to create hard link 'SCRATCH_MNT/dir1/foo1' => 'SCRATCH_MNT/dir/foo1': Invalid cross-device link
> +654321 SCRATCH_MNT/dir1/foo1
> +123456 SCRATCH_MNT/dir1/foo1
> diff --git a/tests/generic/group b/tests/generic/group
> index 7491282..e834762 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -364,3 +364,4 @@
>  359 auto quick clone
>  360 auto quick metadata
>  361 auto quick
> +362 auto quick

and quota.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Eric Sandeen]

On 7/6/16 6:35 PM, Dave Chinner wrote:

...

>> +_require_scratch
>> +_require_chattr
>> +_require_test_lsattr
>> +_require_quota
> 
> needs  _require_prjquota, and that function needs to be modified to
> detect for both XFS and ext4 support.

I think that if there is desire to test both xfs and non-xfs userspace
with project quota, then we need to differentiate between "e2fsprogs
and linux-quota and the kernel all support it" and "xfsprogs and
the kernel both support it" don't we?

IOWs if the test uses setquota/repquota, chattr, mkfs, and fsck to
work with project quota, then that's a different set of requirements
from a test using xfs_io, xfs_quota, etc.

-Eric

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Dave Chinner]

On Wed, Jul 06, 2016 at 09:47:28PM -0500, Eric Sandeen wrote:
> On 7/6/16 6:35 PM, Dave Chinner wrote:
> 
> ...
> 
> >> +_require_scratch
> >> +_require_chattr
> >> +_require_test_lsattr
> >> +_require_quota
> > 
> > needs  _require_prjquota, and that function needs to be modified to
> > detect for both XFS and ext4 support.
> 
> I think that if there is desire to test both xfs and non-xfs userspace
> with project quota, then we need to differentiate between "e2fsprogs
> and linux-quota and the kernel all support it" and "xfsprogs and
> the kernel both support it" don't we?

Well, it should be just "linux-quota and kernel". ext4 needs to
have the same mount option behaviour for project quota as it does
for all other types of quota, not be dependent on mkfs....

> IOWs if the test uses setquota/repquota, chattr, mkfs, and fsck to
> work with project quota, then that's a different set of requirements
> from a test using xfs_io, xfs_quota, etc.

_require_linux_prjquota
_require_xfs_prjquota

But that said, both ext4 and xfs need to work for both
configurations, and they should all be using the common xfstests
quota infrastructure....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Theodore Ts'o]

On Fri, Jul 08, 2016 at 10:51:27AM +1000, Dave Chinner wrote:
> On Wed, Jul 06, 2016 at 09:47:28PM -0500, Eric Sandeen wrote:
> > On 7/6/16 6:35 PM, Dave Chinner wrote:
> > 
> > ...
> > 
> > >> +_require_scratch
> > >> +_require_chattr
> > >> +_require_test_lsattr
> > >> +_require_quota
> > > 
> > > needs  _require_prjquota, and that function needs to be modified to
> > > detect for both XFS and ext4 support.
> > 
> > I think that if there is desire to test both xfs and non-xfs userspace
> > with project quota, then we need to differentiate between "e2fsprogs
> > and linux-quota and the kernel all support it" and "xfsprogs and
> > the kernel both support it" don't we?
> 
> Well, it should be just "linux-quota and kernel". ext4 needs to
> have the same mount option behaviour for project quota as it does
> for all other types of quota, not be dependent on mkfs....

Project quota for ext4 is an optional thing, and if nothing else, we
need to have a separate feature flag for legacy file systems that were
created before we started supporting project quota.  So if you want to
support project quota you *will* need to have a version of e2fsck that
understands project quota, and a version of mke2fs that knows how to
request that project quota be enabled, etc., etc.

So while it might be *nice* if ext4 could support project quota
without being dependent on having a specific version of mke2fs and
e2fsck installed, it's just simply not possible....

> > IOWs if the test uses setquota/repquota, chattr, mkfs, and fsck to
> > work with project quota, then that's a different set of requirements
> > from a test using xfs_io, xfs_quota, etc.
> 
> _require_linux_prjquota
> _require_xfs_prjquota
> 
> But that said, both ext4 and xfs need to work for both
> configurations, and they should all be using the common xfstests
> quota infrastructure....

Agreed, but we want xfstests to be able to support systems where
linux-quota (aka quotatools) and/or e2fsprogs and/or the kernel
haven't been upgraded to support project quota, don't we?  If for no
other reason than to be kind to the poor souls who have to support
RHEL 6.  :-)

						- Ted

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Eric Sandeen]

On 7/7/16 9:46 PM, Theodore Ts'o wrote:
> On Fri, Jul 08, 2016 at 10:51:27AM +1000, Dave Chinner wrote:
>> On Wed, Jul 06, 2016 at 09:47:28PM -0500, Eric Sandeen wrote:
>>> On 7/6/16 6:35 PM, Dave Chinner wrote:
>>>
>>> ...
>>>
>>>>> +_require_scratch
>>>>> +_require_chattr
>>>>> +_require_test_lsattr
>>>>> +_require_quota
>>>>
>>>> needs  _require_prjquota, and that function needs to be modified to
>>>> detect for both XFS and ext4 support.
>>>
>>> I think that if there is desire to test both xfs and non-xfs userspace
>>> with project quota, then we need to differentiate between "e2fsprogs
>>> and linux-quota and the kernel all support it" and "xfsprogs and
>>> the kernel both support it" don't we?
>>
>> Well, it should be just "linux-quota and kernel". ext4 needs to
>> have the same mount option behaviour for project quota as it does
>> for all other types of quota, not be dependent on mkfs....
> 
> Project quota for ext4 is an optional thing, and if nothing else, we
> need to have a separate feature flag for legacy file systems that were
> created before we started supporting project quota.  So if you want to
> support project quota you *will* need to have a version of e2fsck that
> understands project quota, and a version of mke2fs that knows how to
> request that project quota be enabled, etc., etc.
> 
> So while it might be *nice* if ext4 could support project quota
> without being dependent on having a specific version of mke2fs and
> e2fsck installed, it's just simply not possible....
> 
>>> IOWs if the test uses setquota/repquota, chattr, mkfs, and fsck to
>>> work with project quota, then that's a different set of requirements
>>> from a test using xfs_io, xfs_quota, etc.
>>
>> _require_linux_prjquota
>> _require_xfs_prjquota
>>
>> But that said, both ext4 and xfs need to work for both
>> configurations, and they should all be using the common xfstests
>> quota infrastructure....
> 
> Agreed, but we want xfstests to be able to support systems where
> linux-quota (aka quotatools) and/or e2fsprogs and/or the kernel
> haven't been upgraded to support project quota, don't we?  If for no
> other reason than to be kind to the poor souls who have to support
> RHEL 6.  :-)

It's unlikely that ext4 project quota will find its way to RHEL6. ;)

But the point I keep trying to make - and failing, apparently - 
is that we will / should have two sets of tests for userspace
functionality at least; one using standard quota tools, and one
using xfs_quota.  Both should test the same kernel paths, but
if we want to know that userspace is working we need to test both.

And to test one or the other, we need to know that *it* supports
project quota before proceeding.

I don't know how we got to the point where we have 2 parallel
quota infrastructures, it's an unfortunate mess IMHO.  :(

But if we want to test xfs_quota tests on ext4, we still
need to know that e2fsprogs is pquota-capable.

If we want to test standard quota tools on ext4, we need to know
that *those* binaries are capable, as well as e2fsprogs.  etc...

-Eric

> 						- Ted
> 

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Dave Chinner]

On Thu, Jul 07, 2016 at 10:19:27PM -0500, Eric Sandeen wrote:
> 
> 
> On 7/7/16 9:46 PM, Theodore Ts'o wrote:
> > On Fri, Jul 08, 2016 at 10:51:27AM +1000, Dave Chinner wrote:
> >> On Wed, Jul 06, 2016 at 09:47:28PM -0500, Eric Sandeen wrote:
> >>> On 7/6/16 6:35 PM, Dave Chinner wrote:
> >>>
> >>> ...
> >>>
> >>>>> +_require_scratch
> >>>>> +_require_chattr
> >>>>> +_require_test_lsattr
> >>>>> +_require_quota
> >>>>
> >>>> needs  _require_prjquota, and that function needs to be modified to
> >>>> detect for both XFS and ext4 support.
> >>>
> >>> I think that if there is desire to test both xfs and non-xfs userspace
> >>> with project quota, then we need to differentiate between "e2fsprogs
> >>> and linux-quota and the kernel all support it" and "xfsprogs and
> >>> the kernel both support it" don't we?
> >>
> >> Well, it should be just "linux-quota and kernel". ext4 needs to
> >> have the same mount option behaviour for project quota as it does
> >> for all other types of quota, not be dependent on mkfs....
> > 
> > Project quota for ext4 is an optional thing, and if nothing else, we
> > need to have a separate feature flag for legacy file systems that were
> > created before we started supporting project quota.

No shit, Sherlock.

But we don't need the feature bit set by mkfs to support project
quota - it only needs to be set when the project quota file is
created by quotacheck. Or by the first mount -o pquota operation
after quotacheck has created the quotafile.

Either way, project quota can be dynamically turned on and be
protected against legacy filesystems, and it still can't be turned
on without a userspace or kernel that understands project quotas
being installed.

> > So if you want to
> > support project quota you *will* need to have a version of e2fsck that
> > understands project quota, and a version of mke2fs that knows how to
> > request that project quota be enabled, etc., etc.
> > So while it might be *nice* if ext4 could support project quota
> > without being dependent on having a specific version of mke2fs and
> > e2fsck installed, it's just simply not possible....

We can test for that in _requires_linux_prjquota() and _notrun the
test is it isn't present. We do this to test for all optional
features in different filesystems - how is doing this for
ext4 project quota support in any way difficult or so special it's
not possible to implement such checks?

> >>> IOWs if the test uses setquota/repquota, chattr, mkfs, and fsck to
> >>> work with project quota, then that's a different set of requirements
> >>> from a test using xfs_io, xfs_quota, etc.
> >>
> >> _require_linux_prjquota
> >> _require_xfs_prjquota
> >>
> >> But that said, both ext4 and xfs need to work for both
> >> configurations, and they should all be using the common xfstests
> >> quota infrastructure....
> > 
> > Agreed, but we want xfstests to be able to support systems where
> > linux-quota (aka quotatools) and/or e2fsprogs and/or the kernel
> > haven't been upgraded to support project quota, don't we?  If for no
> > other reason than to be kind to the poor souls who have to support
> > RHEL 6.  :-)
> 
> It's unlikely that ext4 project quota will find its way to RHEL6. ;)
> 
> But the point I keep trying to make - and failing, apparently - 
> is that we will / should have two sets of tests for userspace
> functionality at least; one using standard quota tools, and one
> using xfs_quota.  Both should test the same kernel paths, but
> if we want to know that userspace is working we need to test both.

That's what I've been trying to say.

i.e.  the only difference between two project quota tests should be
the binaries run to set project quota flags, limits and get reports.
Otherwise the tests should be the same, similar to how we already
abstract quota setup and mounting, or like we abstract the fallocate
vs XFS preallocation/punch/zero commands in xfs_io in
_test_generic_punch().

i.e. test code is common between two tests, only the setup is
different. And, most importantly, they should give identical
accounting results.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Theodore Ts'o]

On Thu, Jul 07, 2016 at 10:19:27PM -0500, Eric Sandeen wrote:
> 
> But the point I keep trying to make - and failing, apparently - 
> is that we will / should have two sets of tests for userspace
> functionality at least; one using standard quota tools, and one
> using xfs_quota.  Both should test the same kernel paths, but
> if we want to know that userspace is working we need to test both.

I agree completely that we should test both userspace stacks.  Where I
disagree is whether this has anything to do with using the usrquota
and grpquota mount options, and which ext4 mkfs options are used to
set up quota or project quota support.  That's a completely orthogonal
issue.

> I don't know how we got to the point where we have 2 parallel
> quota infrastructures, it's an unfortunate mess IMHO.  :(

Actually, I've been staring at the quotatools source code and it's
even more complicated than that.  There are newer quotactl
subcommands, such as Q_XGETQSTAT and Q_XGETQSTATV, which currently
quotatools only tries using if it thinks the quota format (which in
this sense seems to be system API, not the actual quota file format
--- these two concepts seem to have been overloaded at some point) is
"xfs".

Currently quotatools only assumes the "xfs" quota format should be
used for "xfs" and "gfs" --- but it works for other file systems,
including ext4 as well.  As a result, there's certain information,
such as whether ext4 is doing limits enforcement as well as quota
tracking, which is *not* being exposed to the user.  I suspect one of
the reasons for this is the tests in quotatools for which kernel
interfaces are present are fairly primitive, and in fact there are
some comments in quotasys.c which makes references to behaviours of
certain specific Red Hat kernel versions to decide which interfaces
are available.  :-(

And if we just did the simple thing to enable use of the "new" (aka
"xfs", although this is ***massive*** misnomer) quota format in
quotatools, it would break if the latest quota tools were ever
compiled on older Enterprise Linux systems.

Ugh.  I suspect one of the reasons why things have gotten into such a
mess is that quota is mostly an enterprise feature, and most community
distros and community users/kernel developers don't really use it.  As
a result, no one has bothered to put in clean ways of doing interface
versioning, as I suspect a lot of work happens right before the
Enterprise Linux cutoff date, and there isn't much incentive to clean
things up afterwards.

> But if we want to test xfs_quota tests on ext4, we still
> need to know that e2fsprogs is pquota-capable.
> 
> If we want to test standard quota tools on ext4, we need to know
> that *those* binaries are capable, as well as e2fsprogs.  etc...

Completely agree.

						- Ted

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Jan Kara]

On Fri 08-07-16 01:02:28, Ted Tso wrote:
> On Thu, Jul 07, 2016 at 10:19:27PM -0500, Eric Sandeen wrote:
> > I don't know how we got to the point where we have 2 parallel
> > quota infrastructures, it's an unfortunate mess IMHO.  :(
> 
> Actually, I've been staring at the quotatools source code and it's
> even more complicated than that.  There are newer quotactl
> subcommands, such as Q_XGETQSTAT and Q_XGETQSTATV, which currently
> quotatools only tries using if it thinks the quota format (which in
> this sense seems to be system API, not the actual quota file format
> --- these two concepts seem to have been overloaded at some point) is
> "xfs".

quota-tools currently assume that certain quotactl(8) calls are fine only
for certain quota formats. And this has been long true - since its
beginning, XFS and VFS quota stacks were pretty independent including
supported quotactl calls. Only recently (about year and half ago) I have
added necessary plumbing so that "XFS quotactls" and "VFS quotactls" can be
used independently of the underlying filesystem. The distinction still
remained in quota-tools because there was no real need in unifying them and
we still have to keep knowing about the distinction so that quota-tools can
work with older kernels.

> Currently quotatools only assumes the "xfs" quota format should be
> used for "xfs" and "gfs" --- but it works for other file systems,
> including ext4 as well.  As a result, there's certain information,
> such as whether ext4 is doing limits enforcement as well as quota
> tracking, which is *not* being exposed to the user.  I suspect one of
> the reasons for this is the tests in quotatools for which kernel
> interfaces are present are fairly primitive, and in fact there are
> some comments in quotasys.c which makes references to behaviours of
> certain specific Red Hat kernel versions to decide which interfaces
> are available.  :-(

No, the reason is that until recently there was no kernel interface
to convey this information to userspace for VFS based quotas and nobody
complained :). If there is some functionality you miss in quota-tools, then
I can have a look at implementing it. E.g. reporting whether only tracking
or also enforcement is enabled is relatively simple to add.
 
> And if we just did the simple thing to enable use of the "new" (aka
> "xfs", although this is ***massive*** misnomer) quota format in
> quotatools, it would break if the latest quota tools were ever
> compiled on older Enterprise Linux systems.

What would you like to achieve with this? There is 'QF_META' format which
is different from 'QF_XFS' format basically only in the set of quotactls
used. As I said above it might be nice to separate kernel-api from the
underlying-quota-format but in reality these two were bound together in
older kernels so they are not really independent.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Theodore Ts'o]

On Mon, Jul 11, 2016 at 06:15:56PM +0200, Jan Kara wrote:
> 
> What would you like to achieve with this? There is 'QF_META' format which
> is different from 'QF_XFS' format basically only in the set of quotactls
> used. As I said above it might be nice to separate kernel-api from the
> underlying-quota-format but in reality these two were bound together in
> older kernels so they are not really independent.

The main reason why I noticed is with the new (err, "latest") ext4
quota (enabled via mke2fs -t ext4 -O quota) implementation, we enable
quota tracking at mount time.  (This may be true with journalled quota
as well, actually).  But we don't actually enable quota *enforcement*
until quotaon is given.

The problem is that quotaon -p prints the status of whether or not
quota *tracking* is enabled, and with the new ext4 quota, quota
tracking is *always* enabled.  So quota -p doesn't report anything
useful for new ext4 quota systems, and when I started to look at how
to change things, that's when I noticed that we weren't using the new
quotactl commands with ext4 even though they worked, and that the new
quotactl implementation had more functionality than the older ones.

BTW, I've seriously been thinking about changing the default so that
if you use mke2fs -O quota, quota enforcement is also enabled by
default at mount time, and we use a mount option to disable quota
enforcement.  If we then added a way of selectively enabling and
disabling quota enforcement via quota-tools, then we would be bringing
behaivour of how ext4 quota works to like how xfs treats quota.  The
question I have is how to do this in a way that isn't surprising for
people who are used to the old behaviour --- but mke2fs -O quota is
still relatively new, so maybe we could get away with it without
having to add more superblock flags.

							- Ted

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Jan Kara]

On Mon 11-07-16 13:12:42, Ted Tso wrote:
> On Mon, Jul 11, 2016 at 06:15:56PM +0200, Jan Kara wrote:
> > 
> > What would you like to achieve with this? There is 'QF_META' format which
> > is different from 'QF_XFS' format basically only in the set of quotactls
> > used. As I said above it might be nice to separate kernel-api from the
> > underlying-quota-format but in reality these two were bound together in
> > older kernels so they are not really independent.
> 
> The main reason why I noticed is with the new (err, "latest") ext4
> quota (enabled via mke2fs -t ext4 -O quota) implementation, we enable
> quota tracking at mount time.  (This may be true with journalled quota
> as well, actually).  But we don't actually enable quota *enforcement*
> until quotaon is given.

Yes.

> The problem is that quotaon -p prints the status of whether or not
> quota *tracking* is enabled, and with the new ext4 quota, quota
> tracking is *always* enabled.  So quota -p doesn't report anything
> useful for new ext4 quota systems, and when I started to look at how
> to change things, that's when I noticed that we weren't using the new
> quotactl commands with ext4 even though they worked, and that the new
> quotactl implementation had more functionality than the older ones.

OK. But with XFS you'd notice that quotaon -p also returns 'on' whenever
the accounting is turned on. So ext4 and xfs behave in the same way.
Arguably it would be more useful if quotaon -p reported 'off', 'accounting',
'enforcement'. Maybe I'll do that.

> BTW, I've seriously been thinking about changing the default so that
> if you use mke2fs -O quota, quota enforcement is also enabled by
> default at mount time, and we use a mount option to disable quota
> enforcement.  If we then added a way of selectively enabling and
> disabling quota enforcement via quota-tools, then we would be bringing
> behaivour of how ext4 quota works to like how xfs treats quota.  The
> question I have is how to do this in a way that isn't surprising for
> people who are used to the old behaviour --- but mke2fs -O quota is
> still relatively new, so maybe we could get away with it without
> having to add more superblock flags.

So currently if you use quotaon(8) it will turn on/off only enforcement in
ext4 (accounting is always on when the feature is enabled) - don't get
confused with 'quotaon -p' output - that tests something different.

Speaking of automatic enabling of quota enforcement: I wanted to keep the
old behavior where no enforcement happens until you run quotaon(8) which is
how things traditionally worked for ext2/3/4. That's why things default to
having enforcement off. If we want to make things more consistent with XFS,
one option I can see is that when e.g. 'usrquota' mount option is set, then
user quota enforcement will be turned on. That is essentially how XFS
works (including the mount option name). What do you think?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Jan Kara]

On Tue 12-07-16 12:59:08, Jan Kara wrote:
> On Mon 11-07-16 13:12:42, Ted Tso wrote:
> > On Mon, Jul 11, 2016 at 06:15:56PM +0200, Jan Kara wrote:
> > > 
> > > What would you like to achieve with this? There is 'QF_META' format which
> > > is different from 'QF_XFS' format basically only in the set of quotactls
> > > used. As I said above it might be nice to separate kernel-api from the
> > > underlying-quota-format but in reality these two were bound together in
> > > older kernels so they are not really independent.
> > 
> > The main reason why I noticed is with the new (err, "latest") ext4
> > quota (enabled via mke2fs -t ext4 -O quota) implementation, we enable
> > quota tracking at mount time.  (This may be true with journalled quota
> > as well, actually).  But we don't actually enable quota *enforcement*
> > until quotaon is given.
> 
> Yes.
> 
> > The problem is that quotaon -p prints the status of whether or not
> > quota *tracking* is enabled, and with the new ext4 quota, quota
> > tracking is *always* enabled.  So quota -p doesn't report anything
> > useful for new ext4 quota systems, and when I started to look at how
> > to change things, that's when I noticed that we weren't using the new
> > quotactl commands with ext4 even though they worked, and that the new
> > quotactl implementation had more functionality than the older ones.
> 
> OK. But with XFS you'd notice that quotaon -p also returns 'on' whenever
> the accounting is turned on. So ext4 and xfs behave in the same way.
> Arguably it would be more useful if quotaon -p reported 'off', 'accounting',
> 'enforcement'. Maybe I'll do that.

This is now done and push out to quota-tools repository. When XGETSTAT
quotactl is available, 'quotaon -pva' will report whether the quota is
enabled only for accounting or also enforced.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Theodore Ts'o]

On Tue, Jul 12, 2016 at 12:59:08PM +0200, Jan Kara wrote:
> OK. But with XFS you'd notice that quotaon -p also returns 'on' whenever
> the accounting is turned on. So ext4 and xfs behave in the same way.
> Arguably it would be more useful if quotaon -p reported 'off', 'accounting',
> 'enforcement'. Maybe I'll do that.

That would be good, since at the moment to determine whether or not
quota enforcement is enabled or not.

> Speaking of automatic enabling of quota enforcement: I wanted to keep the
> old behavior where no enforcement happens until you run quotaon(8) which is
> how things traditionally worked for ext2/3/4. That's why things default to
> having enforcement off. If we want to make things more consistent with XFS,
> one option I can see is that when e.g. 'usrquota' mount option is set, then
> user quota enforcement will be turned on. That is essentially how XFS
> works (including the mount option name). What do you think?

That seems reasonable.  The only concern is that it might be confusing
for people who are using older, legacy quota setups, but given that
usrquota would cause enforcing plus accounting to be enabled, it
shouldn't cause a problem.

I think aligning with XFS so that the user experience for quota should
be as identical as possible regardless of which file system is being
used makes a lot of sense, especially since you've been adding a bunch
of the plumbing for quotactl to make this possible.

On the kernel side this means that teaching ext4 so that if the
usrquota monut option is specified, quota enforcing will be enabled.
We should make the necessary changes in kernel and possily quota-tools
so that quotaoff can also disable quota enforcing (just like with
XFS).

Does that sound like a plan?

          				- Ted

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Jan Kara]

[-- Attachment #1: Type: text/plain, Size: 2675 bytes --]

On Tue 12-07-16 12:15:47, Ted Tso wrote:
> On Tue, Jul 12, 2016 at 12:59:08PM +0200, Jan Kara wrote:
> > OK. But with XFS you'd notice that quotaon -p also returns 'on' whenever
> > the accounting is turned on. So ext4 and xfs behave in the same way.
> > Arguably it would be more useful if quotaon -p reported 'off', 'accounting',
> > 'enforcement'. Maybe I'll do that.
> 
> That would be good, since at the moment to determine whether or not
> quota enforcement is enabled or not.

This is now done.

> > Speaking of automatic enabling of quota enforcement: I wanted to keep the
> > old behavior where no enforcement happens until you run quotaon(8) which is
> > how things traditionally worked for ext2/3/4. That's why things default to
> > having enforcement off. If we want to make things more consistent with XFS,
> > one option I can see is that when e.g. 'usrquota' mount option is set, then
> > user quota enforcement will be turned on. That is essentially how XFS
> > works (including the mount option name). What do you think?
> 
> That seems reasonable.  The only concern is that it might be confusing
> for people who are using older, legacy quota setups, but given that
> usrquota would cause enforcing plus accounting to be enabled, it
> shouldn't cause a problem.

Yes, my thinking about this was that when you transfer from a legacy quota
setup to the latest one with hidden quota files, you have to somewhat
update your mindset anyway (e.g. quota information is now created using
tune2fs / mke2fs and checked using e2fsck instead of quotacheck(8)). So the
fact that you won't need quotaon(8) is only a small addition to that.

> I think aligning with XFS so that the user experience for quota should
> be as identical as possible regardless of which file system is being
> used makes a lot of sense, especially since you've been adding a bunch
> of the plumbing for quotactl to make this possible.
> 
> On the kernel side this means that teaching ext4 so that if the
> usrquota monut option is specified, quota enforcing will be enabled.
> We should make the necessary changes in kernel and possily quota-tools
> so that quotaoff can also disable quota enforcing (just like with
> XFS).
> 
> Does that sound like a plan?

Yes. quotaoff will already disable only enforcement for ext4 with hidden
quota files so there's no need to change anything there. We just need to
add kernel support for enabling quota enforcement based on mount option. 
Attached patch should do that - I still need to test whether it doesn't
break anything so don't apply it yet, just have a look whether it looks
sane to you.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[-- Attachment #2: 0001-ext4-Enable-quota-enforcement-based-on-mount-options.patch --]
[-- Type: text/x-patch, Size: 5071 bytes --]

>From 80653a01d0a7ec10598ad5d1eee55612d5a355a5 Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Thu, 14 Jul 2016 15:08:24 +0200
Subject: [PATCH] ext4: Enable quota enforcement based on mount options

When quota information is stored in quota files, we enable only quota
accounting on mount and enforcement is enabled only in response to
Q_QUOTAON quotactl. To make ext4 behavior consistent with XFS, we add a
possibility to enable quota enforcement on mount by specifying
corresponding quota mount option (usrquota, grpquota, prjquota).

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/ext4.h  | 12 +++++++++---
 fs/ext4/super.c | 34 ++++++++++++++++++++++++----------
 2 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index b84aa1ca480a..224cb832e2dc 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1129,9 +1129,15 @@ struct ext4_inode_info {
 #define EXT4_MOUNT_POSIX_ACL		0x08000	/* POSIX Access Control Lists */
 #define EXT4_MOUNT_NO_AUTO_DA_ALLOC	0x10000	/* No auto delalloc mapping */
 #define EXT4_MOUNT_BARRIER		0x20000 /* Use block barriers */
-#define EXT4_MOUNT_QUOTA		0x80000 /* Some quota option set */
-#define EXT4_MOUNT_USRQUOTA		0x100000 /* "old" user quota */
-#define EXT4_MOUNT_GRPQUOTA		0x200000 /* "old" group quota */
+#define EXT4_MOUNT_QUOTA		0x40000 /* Some quota option set */
+#define EXT4_MOUNT_USRQUOTA		0x80000 /* "old" user quota,
+						 * enable enforcement for hidden
+						 * quota files */
+#define EXT4_MOUNT_GRPQUOTA		0x100000 /* "old" group quota, enable
+						  * enforcement for hidden quota
+						  * files */
+#define EXT4_MOUNT_PRJQUOTA		0x200000 /* Enable project quota
+						  * enforcement */
 #define EXT4_MOUNT_DIOREAD_NOLOCK	0x400000 /* Enable support for dio read nolocking */
 #define EXT4_MOUNT_JOURNAL_CHECKSUM	0x800000 /* Journal checksums */
 #define EXT4_MOUNT_JOURNAL_ASYNC_COMMIT	0x1000000 /* Journal Async Commit */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 3822a5aedc61..a03232634cd5 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1187,7 +1187,7 @@ enum {
 	Opt_usrjquota, Opt_grpjquota, Opt_offusrjquota, Opt_offgrpjquota,
 	Opt_jqfmt_vfsold, Opt_jqfmt_vfsv0, Opt_jqfmt_vfsv1, Opt_quota,
 	Opt_noquota, Opt_barrier, Opt_nobarrier, Opt_err,
-	Opt_usrquota, Opt_grpquota, Opt_i_version, Opt_dax,
+	Opt_usrquota, Opt_grpquota, Opt_prjquota, Opt_i_version, Opt_dax,
 	Opt_stripe, Opt_delalloc, Opt_nodelalloc, Opt_mblk_io_submit,
 	Opt_lazytime, Opt_nolazytime,
 	Opt_nomblk_io_submit, Opt_block_validity, Opt_noblock_validity,
@@ -1247,6 +1247,7 @@ static const match_table_t tokens = {
 	{Opt_noquota, "noquota"},
 	{Opt_quota, "quota"},
 	{Opt_usrquota, "usrquota"},
+	{Opt_prjquota, "prjquota"},
 	{Opt_barrier, "barrier=%u"},
 	{Opt_barrier, "barrier"},
 	{Opt_nobarrier, "nobarrier"},
@@ -1466,8 +1467,11 @@ static const struct mount_opts {
 							MOPT_SET | MOPT_Q},
 	{Opt_grpquota, EXT4_MOUNT_QUOTA | EXT4_MOUNT_GRPQUOTA,
 							MOPT_SET | MOPT_Q},
+	{Opt_prjquota, EXT4_MOUNT_QUOTA | EXT4_MOUNT_PRJQUOTA,
+							MOPT_SET | MOPT_Q},
 	{Opt_noquota, (EXT4_MOUNT_QUOTA | EXT4_MOUNT_USRQUOTA |
-		       EXT4_MOUNT_GRPQUOTA), MOPT_CLEAR | MOPT_Q},
+		       EXT4_MOUNT_GRPQUOTA | EXT4_MOUNT_PRJQUOTA),
+							MOPT_CLEAR | MOPT_Q},
 	{Opt_usrjquota, 0, MOPT_Q},
 	{Opt_grpjquota, 0, MOPT_Q},
 	{Opt_offusrjquota, 0, MOPT_Q},
@@ -1756,13 +1760,17 @@ static int parse_options(char *options, struct super_block *sb,
 			return 0;
 	}
 #ifdef CONFIG_QUOTA
-	if (ext4_has_feature_quota(sb) &&
-	    (test_opt(sb, USRQUOTA) || test_opt(sb, GRPQUOTA))) {
-		ext4_msg(sb, KERN_INFO, "Quota feature enabled, usrquota and grpquota "
-			 "mount options ignored.");
-		clear_opt(sb, USRQUOTA);
-		clear_opt(sb, GRPQUOTA);
-	} else if (sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
+	/*
+	 * We do the test below only for project quotas. 'usrquota' and
+	 * 'grpquota' mount options are allowed even without quota feature
+	 * to support legacy quotas in quota files.
+	 */
+	if (test_opt(sb, PRJQUOTA) && !ext4_has_feature_project(sb)) {
+		ext4_msg(sb, KERN_ERR, "Project quota feature not enabled. "
+			 "Cannot enable project quota enforcement.");
+		return 0;
+	}
+	if (sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
 		if (test_opt(sb, USRQUOTA) && sbi->s_qf_names[USRQUOTA])
 			clear_opt(sb, USRQUOTA);
 
@@ -5129,12 +5137,18 @@ static int ext4_enable_quotas(struct super_block *sb)
 		le32_to_cpu(EXT4_SB(sb)->s_es->s_grp_quota_inum),
 		le32_to_cpu(EXT4_SB(sb)->s_es->s_prj_quota_inum)
 	};
+	bool quota_mopt[EXT4_MAXQUOTAS] = {
+		test_opt(sb, USRQUOTA),
+		test_opt(sb, GRPQUOTA),
+		test_opt(sb, PRJQUOTA),
+	};
 
 	sb_dqopt(sb)->flags |= DQUOT_QUOTA_SYS_FILE;
 	for (type = 0; type < EXT4_MAXQUOTAS; type++) {
 		if (qf_inums[type]) {
 			err = ext4_quota_enable(sb, type, QFMT_VFS_V1,
-						DQUOT_USAGE_ENABLED);
+				DQUOT_USAGE_ENABLED |
+				(quota_mopt[type] ? DQUOT_LIMITS_ENABLED : 0));
 			if (err) {
 				ext4_warning(sb,
 					"Failed to enable quota tracking "
-- 
2.6.6

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Wang Shilong]

On Thu, Jul 14, 2016 at 10:13 PM, Jan Kara <jack@suse.cz> wrote:
> On Tue 12-07-16 12:15:47, Ted Tso wrote:

>> Does that sound like a plan?
>
> Yes. quotaoff will already disable only enforcement for ext4 with hidden
> quota files so there's no need to change anything there. We just need to
> add kernel support for enabling quota enforcement based on mount option.
> Attached patch should do that - I still need to test whether it doesn't
> break anything so don't apply it yet, just have a look whether it looks
> sane to you.

I sent a similar patch weeks ago.
http://marc.info/?l=linux-ext4&m=146787897927183&w=2

and also quota tools to support it:

http://marc.info/?l=linux-ext4&m=146787899027184&w=2


>
>                                                                 Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Jan Kara]

On Fri 15-07-16 10:15:31, Wang Shilong wrote:
> On Thu, Jul 14, 2016 at 10:13 PM, Jan Kara <jack@suse.cz> wrote:
> > On Tue 12-07-16 12:15:47, Ted Tso wrote:
> 
> >> Does that sound like a plan?
> >
> > Yes. quotaoff will already disable only enforcement for ext4 with hidden
> > quota files so there's no need to change anything there. We just need to
> > add kernel support for enabling quota enforcement based on mount option.
> > Attached patch should do that - I still need to test whether it doesn't
> > break anything so don't apply it yet, just have a look whether it looks
> > sane to you.
> 
> I sent a similar patch weeks ago.
> http://marc.info/?l=linux-ext4&m=146787897927183&w=2

Your patch was actually quite different. It did implement prjquota option
but the option did something else than in my patch... In your patch you
implemented prjquota option to match the behavior of original quota in
normal files. In my patch I've implemented behavior of prjquota option to
behave similarly as in XFS - i.e. enable enforcement of project quotas when
project quota feature is enabled.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v2] xfstests, generic: add project quota attribute tests

[Wang Shilong]

On Mon, Jul 18, 2016 at 6:20 PM, Jan Kara <jack@suse.cz> wrote:
> On Fri 15-07-16 10:15:31, Wang Shilong wrote:
>> On Thu, Jul 14, 2016 at 10:13 PM, Jan Kara <jack@suse.cz> wrote:
>> > On Tue 12-07-16 12:15:47, Ted Tso wrote:
>>
>> >> Does that sound like a plan?
>> >
>> > Yes. quotaoff will already disable only enforcement for ext4 with hidden
>> > quota files so there's no need to change anything there. We just need to
>> > add kernel support for enabling quota enforcement based on mount option.
>> > Attached patch should do that - I still need to test whether it doesn't
>> > break anything so don't apply it yet, just have a look whether it looks
>> > sane to you.
>>
>> I sent a similar patch weeks ago.
>> http://marc.info/?l=linux-ext4&m=146787897927183&w=2
>
> Your patch was actually quite different. It did implement prjquota option
> but the option did something else than in my patch... In your patch you
> implemented prjquota option to match the behavior of original quota in
> normal files. In my patch I've implemented behavior of prjquota option to
> behave similarly as in XFS - i.e. enable enforcement of project quotas when
> project quota feature is enabled.

I see, ok, that makes sense for me.

Thank you,
Shilong

>
>                                                                 Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR