| Message ID | 1467786171-21127-1-git-send-email-wangshilong1991@gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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