[PATCH 0/3] Fix various small quota tests issues for ext[234]

[PATCH 0/3] Fix various small quota tests issues for ext[234]

[Jan Kara]

Hello,

these patches fix various smaller issues with quota tests with older kernels
and other not so common configurations.

								Honza

[PATCH 1/3] common/quota: Make project quota handling work for ext2 & ext3

[Jan Kara]

Project quota can work for ext2 & ext3 the same way as for ext4. Handle
them properly as currently the tests just fail because of unknown
prjquota mount option. After this patch, tests will be "not run" because
ext4 driver refuses to use project quota mount options for "old" fs
types.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 common/quota | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/common/quota b/common/quota
index f8e037995ad4..2b99521df43d 100644
--- a/common/quota
+++ b/common/quota
@@ -90,9 +90,9 @@ _require_xfs_quota_foreign()
 _require_prjquota()
 {
     [ -n "$1" ] && _dev="$1" || _dev="$TEST_DEV"
-    if [ "$FSTYP" == "ext4" ]; then
+    if [[ "$FSTYP" == ext[234] ]]; then
 	dumpe2fs -h $_dev 2>&1 | grep -qw project || \
-		_notrun "Project quota not available on this ext4"
+		_notrun "Project quota not available on this $FSTYP"
     fi
     src/feature -P $_dev
     [ $? -ne 0 ] && _notrun "Installed kernel does not support project quotas"
@@ -116,7 +116,7 @@ _require_getnextquota()
 # prior to mount.  This is a relatively new feature ...
 _scratch_enable_pquota()
 {
-	[ "$FSTYP" != "ext4" ] && return
+	[[ "$FSTYP" != ext[234] ]] && return
 
 	tune2fs -O quota,project $SCRATCH_DEV >>$seqres.full 2>&1
 	_try_scratch_mount >/dev/null 2>&1 \
@@ -242,7 +242,7 @@ _qmount_option()
 
 	# ext4 doesn't _do_ "-o pquota/prjquota" because reasons
 	# Switch it to "quota" to enable mkfs-time pquota
-	if [ "$FSTYP" == "ext4" ]; then
+	if [[ "$FSTYP" == ext[234] ]]; then
 		OPTS=`echo $OPTS \
 		| sed	-e 's/\bpquota/quota/g'	\
 			-e 's/prjquota/quota/g'`
-- 
2.13.6

[PATCH 2/3] generic/382: Fix test when report uses /etc/passwd

[Jan Kara]

On older kernels xfs_quota uses /etc/passwd to report quota entries. In
such case all-zero lines disturb its output and the test fails. Fix the
problem by ignoring all-zero lines in quota report.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 common/filter     | 8 ++++++++
 tests/generic/382 | 6 +++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/common/filter b/common/filter
index 53874a08f5e3..037799e5cd19 100644
--- a/common/filter
+++ b/common/filter
@@ -344,6 +344,14 @@ _filter_project_quota()
 	_filter_quota | grep -v "^\#0 \|^(null) "
 }
 
+_filter_quota_report()
+{
+	# When xfs_quota uses /etc/passwd for reporting, it may report lines
+	# full of zeros. Ignore them.
+	grep -v -E '^[^ ]+ +0 +0 +0 +00 +\[--------\] +0 +0 +0 +00 +\[--------\]' |
+	grep -v -E '^[^ ]+ +0 +0 +0 +00 +\[--------\]$'
+}
+
 # Account for different "ln" failure messages
 _filter_ln()
 {
diff --git a/tests/generic/382 b/tests/generic/382
index 33326ffa4d1b..02b98a9ef027 100755
--- a/tests/generic/382
+++ b/tests/generic/382
@@ -79,12 +79,12 @@ do_test()
 	$XFS_QUOTA_PROG -x -c "limit bsoft=20M bhard=20M isoft=20 ihard=20 $type -d" $SCRATCH_MNT
 	$XFS_QUOTA_PROG -x -c "limit bsoft=40M bhard=40M isoft=40 ihard=40 $type fsgqa" $SCRATCH_MNT
 	echo "$qname blocks and inode limit"
-	$XFS_QUOTA_PROG -x -c "report $type -N -bi" $SCRATCH_MNT | grep -v ^root | _filter_spaces
+	$XFS_QUOTA_PROG -x -c "report $type -N -bi" $SCRATCH_MNT | grep -v ^root | _filter_quota_report | _filter_spaces
 
 	## blocks default quota test ##
 	_user_do "$XFS_IO_PROG -f -c \"pwrite 0 30M\" -c \"fsync\" $SCRATCH_MNT/data" | _filter_xfs_io
 	echo "$qname blocks quota after write 30M data"
-	$XFS_QUOTA_PROG -x -c "report $type -N -b" $SCRATCH_MNT | grep -v ^root | _filter_spaces
+	$XFS_QUOTA_PROG -x -c "report $type -N -b" $SCRATCH_MNT | grep -v ^root | _filter_quota_report | _filter_spaces
 
 	rm -f ${SCRATCH_MNT}/* >/dev/null 2>&1
 
@@ -95,7 +95,7 @@ do_test()
 	sync
 
 	echo "$qname inode quota after creating 30 inodes"
-	$XFS_QUOTA_PROG -x -c "report $type -N -i" $SCRATCH_MNT | grep -v ^root | _filter_spaces
+	$XFS_QUOTA_PROG -x -c "report $type -N -i" $SCRATCH_MNT | grep -v ^root | _filter_quota_report | _filter_spaces
 
 	rm -f ${SCRATCH_MNT}/* >/dev/null 2>&1
 }
-- 
2.13.6

Re: [PATCH 2/3] generic/382: Fix test when report uses /etc/passwd

[Eryu Guan]

On Mon, May 14, 2018 at 07:03:50PM +0200, Jan Kara wrote:
> On older kernels xfs_quota uses /etc/passwd to report quota entries. In
> such case all-zero lines disturb its output and the test fails. Fix the
> problem by ignoring all-zero lines in quota report.

I notice that generic/381 does quota report in a similar way as in
generic/382, is 381 affected by the same problem? If only generic/382 is
affected, we could move the filter from common/filter to the test
itself.

BTW, I'm curious what kernel behaves in this way, what distribution are
you using that reproduces this failure? I want to try it myself too.

Thanks,
Eryu

> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  common/filter     | 8 ++++++++
>  tests/generic/382 | 6 +++---
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/common/filter b/common/filter
> index 53874a08f5e3..037799e5cd19 100644
> --- a/common/filter
> +++ b/common/filter
> @@ -344,6 +344,14 @@ _filter_project_quota()
>  	_filter_quota | grep -v "^\#0 \|^(null) "
>  }
>  
> +_filter_quota_report()
> +{
> +	# When xfs_quota uses /etc/passwd for reporting, it may report lines
> +	# full of zeros. Ignore them.
> +	grep -v -E '^[^ ]+ +0 +0 +0 +00 +\[--------\] +0 +0 +0 +00 +\[--------\]' |
> +	grep -v -E '^[^ ]+ +0 +0 +0 +00 +\[--------\]$'
> +}
> +
>  # Account for different "ln" failure messages
>  _filter_ln()
>  {
> diff --git a/tests/generic/382 b/tests/generic/382
> index 33326ffa4d1b..02b98a9ef027 100755
> --- a/tests/generic/382
> +++ b/tests/generic/382
> @@ -79,12 +79,12 @@ do_test()
>  	$XFS_QUOTA_PROG -x -c "limit bsoft=20M bhard=20M isoft=20 ihard=20 $type -d" $SCRATCH_MNT
>  	$XFS_QUOTA_PROG -x -c "limit bsoft=40M bhard=40M isoft=40 ihard=40 $type fsgqa" $SCRATCH_MNT
>  	echo "$qname blocks and inode limit"
> -	$XFS_QUOTA_PROG -x -c "report $type -N -bi" $SCRATCH_MNT | grep -v ^root | _filter_spaces
> +	$XFS_QUOTA_PROG -x -c "report $type -N -bi" $SCRATCH_MNT | grep -v ^root | _filter_quota_report | _filter_spaces
>  
>  	## blocks default quota test ##
>  	_user_do "$XFS_IO_PROG -f -c \"pwrite 0 30M\" -c \"fsync\" $SCRATCH_MNT/data" | _filter_xfs_io
>  	echo "$qname blocks quota after write 30M data"
> -	$XFS_QUOTA_PROG -x -c "report $type -N -b" $SCRATCH_MNT | grep -v ^root | _filter_spaces
> +	$XFS_QUOTA_PROG -x -c "report $type -N -b" $SCRATCH_MNT | grep -v ^root | _filter_quota_report | _filter_spaces
>  
>  	rm -f ${SCRATCH_MNT}/* >/dev/null 2>&1
>  
> @@ -95,7 +95,7 @@ do_test()
>  	sync
>  
>  	echo "$qname inode quota after creating 30 inodes"
> -	$XFS_QUOTA_PROG -x -c "report $type -N -i" $SCRATCH_MNT | grep -v ^root | _filter_spaces
> +	$XFS_QUOTA_PROG -x -c "report $type -N -i" $SCRATCH_MNT | grep -v ^root | _filter_quota_report | _filter_spaces
>  
>  	rm -f ${SCRATCH_MNT}/* >/dev/null 2>&1
>  }
> -- 
> 2.13.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] generic/382: Fix test when report uses /etc/passwd

[Jan Kara]

On Thu 17-05-18 10:42:50, Eryu Guan wrote:
> On Mon, May 14, 2018 at 07:03:50PM +0200, Jan Kara wrote:
> > On older kernels xfs_quota uses /etc/passwd to report quota entries. In
> > such case all-zero lines disturb its output and the test fails. Fix the
> > problem by ignoring all-zero lines in quota report.
> 
> I notice that generic/381 does quota report in a similar way as in
> generic/382, is 381 affected by the same problem? If only generic/382 is
> affected, we could move the filter from common/filter to the test
> itself.

Yes, that test is affected as well. I just didn't notice because on my
system I don't have user 123456-fsgqa (SLE/openSUSE don't allow you to
create user with such name and I didn't bother to force-create it by
manually editting passwd). I'll send a patch to update that test to use the
filter as well if you're otherwise fine with this approach.

> BTW, I'm curious what kernel behaves in this way, what distribution are
> you using that reproduces this failure? I want to try it myself too.

I was doing regression testing for SLE12 SP3 kernel which is 4.4 based
after backporting some fixes. Generally any kernel prior commit 926132c025
"quota: add new quotactl Q_GETNEXTQUOTA" which landed in v4.6 will have
this issue.

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

Re: [PATCH 2/3] generic/382: Fix test when report uses /etc/passwd

[Jan Kara]

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

On Thu 17-05-18 11:18:36, Jan Kara wrote:
> On Thu 17-05-18 10:42:50, Eryu Guan wrote:
> > On Mon, May 14, 2018 at 07:03:50PM +0200, Jan Kara wrote:
> > > On older kernels xfs_quota uses /etc/passwd to report quota entries. In
> > > such case all-zero lines disturb its output and the test fails. Fix the
> > > problem by ignoring all-zero lines in quota report.
> > 
> > I notice that generic/381 does quota report in a similar way as in
> > generic/382, is 381 affected by the same problem? If only generic/382 is
> > affected, we could move the filter from common/filter to the test
> > itself.
> 
> Yes, that test is affected as well. I just didn't notice because on my
> system I don't have user 123456-fsgqa (SLE/openSUSE don't allow you to
> create user with such name and I didn't bother to force-create it by
> manually editting passwd). I'll send a patch to update that test to use the
> filter as well if you're otherwise fine with this approach.

Attached is a patch for generic/381.

								Honza

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

[-- Attachment #2: 0001-generic-381-Fix-test-when-report-uses-etc-passwd.patch --]
[-- Type: text/x-patch, Size: 1710 bytes --]

>From b3ee82ef8a81ed4ab41ad840ff55acd458654b58 Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Thu, 17 May 2018 11:27:21 +0200
Subject: [PATCH] generic/381: Fix test when report uses /etc/passwd

On older kernels xfs_quota uses /etc/passwd to report quota entries. In
such case all-zero lines disturb its output and the test fails. Fix the
problem by ignoring all-zero lines in quota report.

Reported-by: Eryu Guan <guaneryu@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 tests/generic/381 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/generic/381 b/tests/generic/381
index 006f0d879638..72adbee66848 100755
--- a/tests/generic/381
+++ b/tests/generic/381
@@ -67,7 +67,7 @@ $XFS_QUOTA_PROG -x -c "limit -u bsoft=100m bhard=200m 123456-fsgqa" $SCRATCH_MNT
 echo "=== quota command output ==="
 $XFS_QUOTA_PROG -c "quota -u -b -N -v 123456-fsgqa" $SCRATCH_MNT | _filter_quota
 echo "=== report command output ==="
-$XFS_QUOTA_PROG -x -c "report -u -b -N" $SCRATCH_MNT | grep -v "^root " | _filter_quota
+$XFS_QUOTA_PROG -x -c "report -u -b -N" $SCRATCH_MNT | grep -v "^root " | _filter_quota_report | _filter_quota
 
 # group test
 echo "== group test =="
@@ -75,7 +75,7 @@ $XFS_QUOTA_PROG -x -c "limit -g bsoft=100m bhard=200m 123456-fsgqa" $SCRATCH_MNT
 echo "=== quota command output ==="
 $XFS_QUOTA_PROG -c "quota -g -b -N -v 123456-fsgqa" $SCRATCH_MNT | _filter_quota
 echo "=== report command output ==="
-$XFS_QUOTA_PROG -x -c "report -u -b -N" $SCRATCH_MNT | grep -v "^root " | _filter_quota
+$XFS_QUOTA_PROG -x -c "report -u -b -N" $SCRATCH_MNT | grep -v "^root " | _filter_quota_report | _filter_quota
 
 # success, all done
 status=0
-- 
2.13.6

Re: [PATCH 2/3] generic/382: Fix test when report uses /etc/passwd

[Eryu Guan]

On Thu, May 17, 2018 at 12:06:11PM +0200, Jan Kara wrote:
> On Thu 17-05-18 11:18:36, Jan Kara wrote:
> > On Thu 17-05-18 10:42:50, Eryu Guan wrote:
> > > On Mon, May 14, 2018 at 07:03:50PM +0200, Jan Kara wrote:
> > > > On older kernels xfs_quota uses /etc/passwd to report quota entries. In
> > > > such case all-zero lines disturb its output and the test fails. Fix the
> > > > problem by ignoring all-zero lines in quota report.
> > > 
> > > I notice that generic/381 does quota report in a similar way as in
> > > generic/382, is 381 affected by the same problem? If only generic/382 is
> > > affected, we could move the filter from common/filter to the test
> > > itself.
> > 
> > Yes, that test is affected as well. I just didn't notice because on my
> > system I don't have user 123456-fsgqa (SLE/openSUSE don't allow you to
> > create user with such name and I didn't bother to force-create it by
> > manually editting passwd). I'll send a patch to update that test to use the
> > filter as well if you're otherwise fine with this approach.
> 
> Attached is a patch for generic/381.
> 
> 								Honza

Thanks for the update! I probably will test the problem more in next
week (I'm traveling this weekend) and fold it into the generic/382 fix.
I'm taking the other two patches for this week's update.

Thanks,
Eryu

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

> From b3ee82ef8a81ed4ab41ad840ff55acd458654b58 Mon Sep 17 00:00:00 2001
> From: Jan Kara <jack@suse.cz>
> Date: Thu, 17 May 2018 11:27:21 +0200
> Subject: [PATCH] generic/381: Fix test when report uses /etc/passwd
> 
> On older kernels xfs_quota uses /etc/passwd to report quota entries. In
> such case all-zero lines disturb its output and the test fails. Fix the
> problem by ignoring all-zero lines in quota report.
> 
> Reported-by: Eryu Guan <guaneryu@gmail.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  tests/generic/381 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/generic/381 b/tests/generic/381
> index 006f0d879638..72adbee66848 100755
> --- a/tests/generic/381
> +++ b/tests/generic/381
> @@ -67,7 +67,7 @@ $XFS_QUOTA_PROG -x -c "limit -u bsoft=100m bhard=200m 123456-fsgqa" $SCRATCH_MNT
>  echo "=== quota command output ==="
>  $XFS_QUOTA_PROG -c "quota -u -b -N -v 123456-fsgqa" $SCRATCH_MNT | _filter_quota
>  echo "=== report command output ==="
> -$XFS_QUOTA_PROG -x -c "report -u -b -N" $SCRATCH_MNT | grep -v "^root " | _filter_quota
> +$XFS_QUOTA_PROG -x -c "report -u -b -N" $SCRATCH_MNT | grep -v "^root " | _filter_quota_report | _filter_quota
>  
>  # group test
>  echo "== group test =="
> @@ -75,7 +75,7 @@ $XFS_QUOTA_PROG -x -c "limit -g bsoft=100m bhard=200m 123456-fsgqa" $SCRATCH_MNT
>  echo "=== quota command output ==="
>  $XFS_QUOTA_PROG -c "quota -g -b -N -v 123456-fsgqa" $SCRATCH_MNT | _filter_quota
>  echo "=== report command output ==="
> -$XFS_QUOTA_PROG -x -c "report -u -b -N" $SCRATCH_MNT | grep -v "^root " | _filter_quota
> +$XFS_QUOTA_PROG -x -c "report -u -b -N" $SCRATCH_MNT | grep -v "^root " | _filter_quota_report | _filter_quota
>  
>  # success, all done
>  status=0
> -- 
> 2.13.6
> 

Re: [PATCH 2/3] generic/382: Fix test when report uses /etc/passwd

[Dave Chinner]

On Thu, May 17, 2018 at 11:18:36AM +0200, Jan Kara wrote:
> On Thu 17-05-18 10:42:50, Eryu Guan wrote:
> > On Mon, May 14, 2018 at 07:03:50PM +0200, Jan Kara wrote:
> > > On older kernels xfs_quota uses /etc/passwd to report quota entries. In
> > > such case all-zero lines disturb its output and the test fails. Fix the
> > > problem by ignoring all-zero lines in quota report.
> > 
> > I notice that generic/381 does quota report in a similar way as in
> > generic/382, is 381 affected by the same problem? If only generic/382 is
> > affected, we could move the filter from common/filter to the test
> > itself.
> 
> Yes, that test is affected as well. I just didn't notice because on my
> system I don't have user 123456-fsgqa (SLE/openSUSE don't allow you to
> create user with such name and I didn't bother to force-create it by
> manually editting passwd). I'll send a patch to update that test to use the
> filter as well if you're otherwise fine with this approach.

Perhaps it would also be worthwhile to send a patch to allow an
alternate username for quota testing (e.g. fstests-user) and probe
which one to use at startup?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 2/3] generic/382: Fix test when report uses /etc/passwd

[Jan Kara]

On Fri 18-05-18 14:02:02, Dave Chinner wrote:
> On Thu, May 17, 2018 at 11:18:36AM +0200, Jan Kara wrote:
> > On Thu 17-05-18 10:42:50, Eryu Guan wrote:
> > > On Mon, May 14, 2018 at 07:03:50PM +0200, Jan Kara wrote:
> > > > On older kernels xfs_quota uses /etc/passwd to report quota entries. In
> > > > such case all-zero lines disturb its output and the test fails. Fix the
> > > > problem by ignoring all-zero lines in quota report.
> > > 
> > > I notice that generic/381 does quota report in a similar way as in
> > > generic/382, is 381 affected by the same problem? If only generic/382 is
> > > affected, we could move the filter from common/filter to the test
> > > itself.
> > 
> > Yes, that test is affected as well. I just didn't notice because on my
> > system I don't have user 123456-fsgqa (SLE/openSUSE don't allow you to
> > create user with such name and I didn't bother to force-create it by
> > manually editting passwd). I'll send a patch to update that test to use the
> > filter as well if you're otherwise fine with this approach.
> 
> Perhaps it would also be worthwhile to send a patch to allow an
> alternate username for quota testing (e.g. fstests-user) and probe
> which one to use at startup?

Well, we usually use the fsgqa user and that is fine. Just this specific
test wants a username starting with digits and that's why it uses such a
weird name.

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

[PATCH 3/3] common/quota: Fix _qmount_options for ext4 journalled quotas

[Jan Kara]

_qmount_options didn't properly replace ext[34] journalled quotas mount
options. As such the mount option string got garbled and the test
(e.g. generic/379) failed.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 common/quota | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/quota b/common/quota
index 2b99521df43d..f01bd068383b 100644
--- a/common/quota
+++ b/common/quota
@@ -229,8 +229,10 @@ _qmount_option()
 	export MOUNT_OPTIONS=`echo $MOUNT_OPTIONS \
 	| sed   -e 's/uquota/QUOTA/g'      \
 		-e 's/usrquota/QUOTA/g'    \
+		-e 's/usrjquota=[^, ]/QUOTA/g' \
 		-e 's/gquota/QUOTA/g'      \
 		-e 's/grpquota/QUOTA/g'    \
+		-e 's/grpjquota=[^, ]/QUOTA/g' \
 		-e 's/\bpquota/QUOTA/g'    \
 		-e 's/prjquota/QUOTA/g'    \
 		-e 's/quota/QUOTA/g'       \
-- 
2.13.6