[PATCH 0/8] fstests: fix quota failures on xfs

[PATCH 0/8] fstests: fix quota failures on xfs

[Darrick J. Wong]

Hi all,

Several months ago I tried testing xfs with quotas enabled and was
surprised at the number of failures.  Since then I've been on a bug hunt
to triage the mess, and the following series fixes some of the low
hanging fruit.  Some of these are minor behavioral variations when quota
are enabled, and other fixes upgrade old v4 tests to work on v5
filesystems.

To reproduce the results, run xfstests with
'MOUNT_OPTIONS="-o usrquota,grpquota,prjquota"' on a v5 filesystem.
Strike the ',grpquota' on a v4 filesystem.

The first patch fixes the sporadic quota count failures when testing XFS
with quotas by replacing the xfs_spaceman call that cleans out (only)
speculative preallocations with a umount/mount pair that forcibly cleans
out /all/ preallocations and delayed allocations.

The second patch fixes bugs in a couple of tests that use error
injection to test log recovery paths but don't succeed in directly
causing the error to trip.

The third patch works around the fact that mounting xfs with quotas
requires a writable block device even for a ro mount, which leads to
a slightly different error message from the mount call.

Patch 4 and 5 fix the dump and restore tests to handle the case that
quota inodes get sent along with dumps when quotas are enabled.

Patch 6 updates xfs/205 to work with v5 filesystems.

Patch 7 changes tests that require specific quota mount options to clear
out the background quota mount options so that we test with /exactly/
the set of mount options.

Patch 8 filters v5 mount options out of tests that format v4
filesystems on the scratch device.

--D

[PATCH 1/8] quota: clean out speculative preallocations when checking quota usage

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

On XFS, the only reliable way to clean out speculative post-eof
preallocations, delayed allocations, and speculative cow preallocations
is to cycle the filesystem mount.  Since we're comparing the post-test
quota counts against a freshly quotacheck to look for leaks, it's fine
to cycle the mount.  This eliminates sporadic quota count failures when
running xfstests with quotas enabled.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/quota |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)


diff --git a/common/quota b/common/quota
index 993ff5de..9309e786 100644
--- a/common/quota
+++ b/common/quota
@@ -260,10 +260,10 @@ _check_quota_usage()
 		quotaon -f -u -g $SCRATCH_MNT 2>/dev/null
 		;;
 	xfs)
-		# Clear out speculative preallocations to eliminate them
-		# as a source of intermittent orig/checked differences.
-		test -x "$XFS_SPACEMAN_PROG" && \
-			"$XFS_SPACEMAN_PROG" -c 'prealloc -s' $SCRATCH_MNT
+		# Only way to make this reliable with cow/delalloc/speculative
+		# preallocations is to unmount and remount the whole mess...
+		_scratch_unmount
+		_scratch_mount "-o usrquota,grpquota"
 		;;
 	*)
 		;;

[PATCH 2/8] xfs: force unlink metadata updates to disk

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

In xfs/318 and xfs/325 we inject errors on extent freeing then delete
some files to see if we can make the filesystem go offline.  However,
with the advent of deferred inode inactivation, sync won't guarantee
that unlinked inodes removal is actually pushed to disk since the "inode
needs to be deallocated" state is persisted to disk.  Freeze achieves
this, however, so inject a freeze/thaw cycle to make sure we hit the
injected error.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/318 |    1 +
 tests/xfs/325 |    1 +
 2 files changed, 2 insertions(+)


diff --git a/tests/xfs/318 b/tests/xfs/318
index 2f2a34b0..83b858ea 100755
--- a/tests/xfs/318
+++ b/tests/xfs/318
@@ -59,6 +59,7 @@ _scratch_inject_error "free_extent"
 echo "Remove files"
 rm -rf $SCRATCH_MNT/file1
 sync
+$XFS_IO_PROG -x -c 'freeze' -c 'thaw' $SCRATCH_MNT >> $seqres.full 2>&1
 
 echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
diff --git a/tests/xfs/325 b/tests/xfs/325
index 67959015..eafb4ae0 100755
--- a/tests/xfs/325
+++ b/tests/xfs/325
@@ -62,6 +62,7 @@ _scratch_inject_error "free_extent"
 
 rm $SCRATCH_MNT/file1
 sync
+$XFS_IO_PROG -x -c 'freeze' -c 'thaw' $SCRATCH_MNT >> $seqres.full 2>&1
 
 echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch

[PATCH 3/8] generic/050: fix ro blockdev mount of xfs with quota

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

In XFS, mounting with quota always require a writable device.  If the
block device is read only, the mount fails, which fails this test.
Since this is expected, work around this.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/generic/050 |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)


diff --git a/tests/generic/050 b/tests/generic/050
index 90e924db..33da228d 100755
--- a/tests/generic/050
+++ b/tests/generic/050
@@ -36,6 +36,21 @@ _require_norecovery
 
 _scratch_mkfs >/dev/null 2>&1
 
+filter_ro_mount() {
+	local arg=""
+
+	# Mounting with quota on XFS requires a writable fs, which means
+	# we fail the ro blockdev test with with EPERM.
+	if [ "$FSTYP" = "xfs" ] && echo "$MOUNT_OPTIONS" | grep -q quota ; then
+		arg="s|mount: $SCRATCH_MNT: permission denied|mount: device write-protected, mounting read-only|g"
+	fi
+	sed -e "$arg" | _filter_ro_mount
+}
+
+filter_umount() {
+	sed -e "\|umount: $SCRATCH_DEV: not mounted.|d" | _filter_scratch
+}
+
 #
 # Mark the device read-only
 #
@@ -46,7 +61,7 @@ blockdev --setro $SCRATCH_DEV
 # Mount it, and make sure we can't write to it, and we can unmount it again
 #
 echo "mounting read-only block device:"
-_try_scratch_mount 2>&1 | _filter_ro_mount
+_try_scratch_mount 2>&1 | filter_ro_mount
 
 echo "touching file on read-only filesystem (should fail)"
 touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
@@ -56,7 +71,7 @@ touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
 #	http://oss.sgi.com/bugzilla/show_bug.cgi?id=807
 #
 echo "unmounting read-only filesystem"
-_scratch_unmount 2>&1 | _filter_scratch
+_scratch_unmount 2>&1 | filter_umount
 
 echo "setting device read-write"
 blockdev --setrw $SCRATCH_DEV
@@ -92,10 +107,10 @@ _scratch_unmount 2>&1 | _filter_scratch | _filter_ending_dot
 # data recovery hack.
 #
 echo "mounting filesystem with -o norecovery on a read-only device:"
-_try_scratch_mount -o norecovery 2>&1 | _filter_ro_mount
+_try_scratch_mount -o norecovery 2>&1 | filter_ro_mount
 
 echo "unmounting read-only filesystem"
-_scratch_unmount 2>&1 | _filter_scratch
+_scratch_unmount 2>&1 | filter_umount
 
 echo "setting device read-write"
 blockdev --setrw $SCRATCH_DEV

[PATCH 4/8] dump: don't fail if multi-file dumps don't all contain quota info

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

When xfsdump is making multi-file dumps, the quota information are
written into the last dump file.  However, the multi-file restore script
(as written) expects to see quota information restored by every dump
file and fails.  This is silly, so fix it.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/dump |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)


diff --git a/common/dump b/common/dump
index b6b7fb46..c45b8a5a 100644
--- a/common/dump
+++ b/common/dump
@@ -1432,16 +1432,19 @@ _check_quota()
 			    print "Found extra:", $0
 			}
 			next
+	}
+	$0 ~ "Restore Status: INCOMPLETE" {
+			incomplete = 1
 	}
 			{ print }
 	END {
-		if (uquota && !found_uquota) {
+		if (uquota && !found_uquota && !incomplete) {
 		    print "Missing user quota msg:", usermsg
 		}
-		if (gquota && !found_gquota) {
+		if (gquota && !found_gquota && !incomplete) {
 		    print "Missing group quota msg:", groupmsg
 		}
-		if (pquota && !found_pquota) {
+		if (pquota && !found_pquota && !incomplete) {
 		    print "Missing project quota msg:", projectmsg
 		}
 	}

[PATCH 5/8] xfs/266: fix restore summary counter when quotas are enabled

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

When quotas are enabled, an incremental backup has to record the quota
file updates too.  Inside a dump the quota files are stored as special
files under the root dump directory.  xfsrestore reports the dump dir
in its restore summary counts even if the dump dir was only there for
the sake of the quota updates, which makes the test fail.  Fix this by
massaging the output when quotas are turned on.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/266 |   17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)


diff --git a/tests/xfs/266 b/tests/xfs/266
index c4c3e8d9..9084f5b9 100755
--- a/tests/xfs/266
+++ b/tests/xfs/266
@@ -26,6 +26,21 @@ _add_and_append_dumpdir_fill()
     _append_dumpdir_fill
 }
 
+# Quota files are stored as special files in the dumpdir of the incremental
+# backup.  This throws off the directory/file count reported because xfsrestore
+# includes the dumpdir in the restore summary counts.
+filter_cumulative_quota_updates() {
+    $here/src/feature -U $SCRATCH_DEV && quota=1
+    $here/src/feature -G $SCRATCH_DEV && quota=1
+    $here/src/feature -P $SCRATCH_DEV && quota=1
+
+    $AWK_PROG -v quota=$quota '
+	/entries processed/ {
+		if (quota) {$2--; $5--;}
+	}
+	{print}'
+}
+
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/dump
@@ -47,7 +62,7 @@ _add_and_append_dumpdir_fill
 _do_dump_file -f $tmp.df.1 -l 1 -D
 _prepare_restore_dir
 _do_restore_file_cum -f $tmp.df.0
-_do_restore_file_cum -f $tmp.df.1
+_do_restore_file_cum -f $tmp.df.1 | filter_cumulative_quota_updates
 _ls_compare_sub
 _diff_compare
 

[PATCH 6/8] xfs/205: update for v5 filesystems

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Update this test to work with v5 filesystems, which have a minimum block
size of 1k.  Adapt the script to sense the amount of free space reported
so that we can get close to ENOSPC instead of hardcoding.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/205 |   21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)


diff --git a/tests/xfs/205 b/tests/xfs/205
index ed07d165..9cc9b599 100755
--- a/tests/xfs/205
+++ b/tests/xfs/205
@@ -27,14 +27,15 @@ _require_scratch_nocheck
 
 rm -f $seqres.full
 
-_scratch_mkfs_xfs -d size=16m -b size=512 -l size=4944b >> $seqres.full 2>&1
+fsblksz=1024
+_scratch_mkfs_xfs -d size=$((32768*fsblksz)) -b size=$fsblksz >> $seqres.full 2>&1
 _scratch_mount
 
 # fix the reserve block pool to a known size so that the enospc calculations
 # work out correctly.
 _scratch_resvblks 1024 >> $seqres.full 2>&1
 
-# on a 16MB filesystem, there's 32768x512byte blocks. used is:
+# on a 16MB filesystem, there's 32768x$fsblkszbyte blocks. used is:
 #	- 4944 in the log,
 #	- 32+1 for the root inode cluster
 #	- 4 for the AG header
@@ -50,19 +51,25 @@ _scratch_resvblks 1024 >> $seqres.full 2>&1
 # Filesystem           1K-blocks      Used Available Use% Mounted on
 # /dev/ubdc                13912     13908         4 100% /mnt/scratch
 #
-dd if=/dev/zero of=$SCRATCH_MNT/fred bs=512 count=25000 2>&1 | _filter_dd
-dd if=/dev/zero of=$SCRATCH_MNT/fred2 bs=512 count=500 2>&1 | _filter_dd
-dd if=/dev/zero of=$SCRATCH_MNT/fred3 bs=512 count=245 2>&1 | _filter_dd
+blks=$(( ($(_get_available_space $SCRATCH_MNT) / $fsblksz) - 15 ))
+test $blks -gt 1100 || _notrun "not enough space"
+b1=$(echo $blks | sed -e 's/[0-9][0-9][0-9]$/000/g')
+b2=$(echo $blks | sed -e 's/^[0-9][0-9]//g')
+echo "blks: $blks b1: $b1 b2: $b2" >> $seqres.full
+dd if=/dev/zero of=$SCRATCH_MNT/fred bs=$fsblksz count=$((b1 - 1000)) 2>&1 | _filter_dd
+dd if=/dev/zero of=$SCRATCH_MNT/fred2 bs=$fsblksz count=$b2 2>&1 | _filter_dd
+dd if=/dev/zero of=$SCRATCH_MNT/fred3 bs=$fsblksz count=$((blks - (b1 + b2))) 2>&1 | _filter_dd
 rm -f $SCRATCH_MNT/fred*
 
 echo "*** one file"
 # now try a single file of that size
-dd if=/dev/zero of=$SCRATCH_MNT/fred bs=512 count=26745 2>&1 | _filter_dd
+dd if=/dev/zero of=$SCRATCH_MNT/fred bs=$fsblksz count=$blks 2>&1 | _filter_dd
 #rm -f $SCRATCH_MNT/fred*
 
 echo "*** one file, a few bytes at a time"
 # now try a single file of that size
-dd if=/dev/zero of=$SCRATCH_MNT/fred bs=15 count=$[26745/15*512] 2>&1 | _filter_dd
+dd if=/dev/zero of=$SCRATCH_MNT/fred bs=15 count=$[blks/15*$fsblksz] 2>&1 | _filter_dd
+echo space: $(_get_available_space $SCRATCH_MNT) >> $seqres.full
 
 # success, all done
 echo "*** done"

[PATCH 7/8] misc: force the exact quota options coded into the test

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Several tests require setting exactly the quota mount options that are
encoded in the test.  Unfortunately, quota mount options set by the user
in MOUNT_OPTIONS can turn on other quota types, which causes the golden
outputs to be off for various reasons (quota report output, behavior
changes, etc.)

So use _qmount_option to delete all the quota mount options in these
tests so that the test itself can run with exactly the config for which
it was written.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/generic/230 |    3 +++
 tests/xfs/075     |    4 ++++
 tests/xfs/116     |    3 +++
 tests/xfs/132     |    5 +++++
 tests/xfs/220     |    3 +++
 5 files changed, 18 insertions(+)


diff --git a/tests/generic/230 b/tests/generic/230
index 66b12d2f..e0a876ba 100755
--- a/tests/generic/230
+++ b/tests/generic/230
@@ -99,6 +99,9 @@ rm -f $seqres.full
 
 grace=2
 
+# Test only the quota type specified in each _scratch_mount clause below.
+_qmount_option 'defaults'
+
 _scratch_mkfs >> $seqres.full 2>&1
 _scratch_mount "-o usrquota,grpquota"
 BLOCK_SIZE=$(_get_file_block_size $SCRATCH_MNT)
diff --git a/tests/xfs/075 b/tests/xfs/075
index caf612bb..dee01462 100755
--- a/tests/xfs/075
+++ b/tests/xfs/075
@@ -25,12 +25,16 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
+. ./common/quota
 
 # real QA test starts here
 _supported_fs xfs
 _supported_os Linux
 _require_scratch
 
+# norecovery mounts do not succeed with quotas eanbled, so shut them off
+_qmount_option "defaults"
+
 echo "Silence is golden"
 _scratch_mkfs_sized $((512 * 1024 * 1024)) >$seqres.full
 
diff --git a/tests/xfs/116 b/tests/xfs/116
index 6887d7e5..4422eb50 100755
--- a/tests/xfs/116
+++ b/tests/xfs/116
@@ -36,6 +36,9 @@ _supported_os Linux
 _require_scratch
 _require_xfs_quota
 
+# Only mount with the quota options we specify below
+_qmount_option "defaults"
+
 _scratch_mkfs >/dev/null 2>&1
 _scratch_mount "-o uquota"
 _scratch_unmount
diff --git a/tests/xfs/132 b/tests/xfs/132
index c5fe8b5a..a421f727 100755
--- a/tests/xfs/132
+++ b/tests/xfs/132
@@ -24,6 +24,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
+. ./common/quota
 
 # remove previous $seqres.full before test
 rm -f $seqres.full
@@ -43,6 +44,10 @@ _disable_dmesg_check
 _require_xfs_mkfs_crc
 _scratch_mkfs -m crc=0 > $seqres.full 2>&1
 
+# The files that EIO in the golden output changes if we have quotas enabled
+# so leave it off.
+_qmount_option "defaults"
+
 # corrupt an inode in the root inode chunk
 root_ino=$(_scratch_xfs_get_metadata_field 'rootino' 'sb 0')
 corrupt_ino=$((root_ino + 15))
diff --git a/tests/xfs/220 b/tests/xfs/220
index 227d5452..61319c9d 100755
--- a/tests/xfs/220
+++ b/tests/xfs/220
@@ -36,6 +36,9 @@ _supported_os Linux
 _require_scratch
 _require_quota
 
+# Only mount with the specific quota options mentioned below
+_qmount_option "defaults"
+
 echo "Silence is golden."
 
 # create scratch filesystem

[PATCH 8/8] xfs: filter out mount options that don't work on v4 filesystems

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

A few tests require v4 filesystems and enforce this by disabling crc's
in the _scratch_mkfs call.  However, if the user specified MOUNT_OPTIONS
that only work with v5 filesystems, these tests fail.  If we detect a
test creating a v4 scratch filesystem, filter out incompatible mount
options that don't work on v4, such as simultaneous group/project quota.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/xfs |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)


diff --git a/common/xfs b/common/xfs
index 80340b0d..484e6ab6 100644
--- a/common/xfs
+++ b/common/xfs
@@ -89,6 +89,7 @@ _scratch_mkfs_xfs()
 	_scratch_do_mkfs "$mkfs_cmd" "$mkfs_filter" $* 2>$tmp.mkfserr 1>$tmp.mkfsstd
 	mkfs_status=$?
 
+	grep -q crc=0 $tmp.mkfsstd && _force_xfsv4_mount_options
 
 	if [ $mkfs_status -eq 0 -a "$LARGE_SCRATCH_DEV" = yes ]; then
 		# manually parse the mkfs output to get the fs size in bytes
@@ -779,3 +780,26 @@ _scratch_get_bmx_prefix() {
 	_scratch_xfs_db -c "inode ${ino}" -c 'p' >> $seqres.full
 	return 1
 }
+
+#
+# Ensures that we don't pass any mount options incompatible with XFS v4
+#
+_force_xfsv4_mount_options()
+{
+	local gquota=0
+	local pquota=0
+
+	# Can't have group and project quotas in XFS v4
+	echo "$MOUNT_OPTIONS" | egrep -q "(gquota|grpquota|grpjquota=|gqnoenforce)" && gquota=1
+	echo "$MOUNT_OPTIONS" | egrep -q "(\bpquota|prjquota|pqnoenforce)" && pquota=1
+
+	if [ $gquota -gt 0 ] && [ $pquota -gt 0 ]; then
+		export MOUNT_OPTIONS=$(echo $MOUNT_OPTIONS \
+			| sed   -e 's/gquota/QUOTA/g'      \
+				-e 's/grpquota/QUOTA/g'    \
+				-e 's/grpjquota=[^, ]/QUOTA/g' \
+				-e 's/gqnoenforce/QUOTA/g' \
+				-e "s/QUOTA/defaults/g")
+	fi
+	echo "MOUNT_OPTIONS = $MOUNT_OPTIONS" >>$seqres.full
+}

Re: [PATCH 3/8] generic/050: fix ro blockdev mount of xfs with quota

[Eryu Guan]

On Thu, Nov 01, 2018 at 04:19:21PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> In XFS, mounting with quota always require a writable device.  If the
> block device is read only, the mount fails, which fails this test.
> Since this is expected, work around this.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  tests/generic/050 |   23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/tests/generic/050 b/tests/generic/050
> index 90e924db..33da228d 100755
> --- a/tests/generic/050
> +++ b/tests/generic/050
> @@ -36,6 +36,21 @@ _require_norecovery
>  
>  _scratch_mkfs >/dev/null 2>&1
>  
> +filter_ro_mount() {
> +	local arg=""
> +
> +	# Mounting with quota on XFS requires a writable fs, which means
> +	# we fail the ro blockdev test with with EPERM.
> +	if [ "$FSTYP" = "xfs" ] && echo "$MOUNT_OPTIONS" | grep -q quota ; then
> +		arg="s|mount: $SCRATCH_MNT: permission denied|mount: device write-protected, mounting read-only|g"
> +	fi
> +	sed -e "$arg" | _filter_ro_mount
> +}
> +
> +filter_umount() {
> +	sed -e "\|umount: $SCRATCH_DEV: not mounted.|d" | _filter_scratch
> +}
> +
>  #
>  # Mark the device read-only
>  #
> @@ -46,7 +61,7 @@ blockdev --setro $SCRATCH_DEV
>  # Mount it, and make sure we can't write to it, and we can unmount it again
>  #
>  echo "mounting read-only block device:"
> -_try_scratch_mount 2>&1 | _filter_ro_mount
> +_try_scratch_mount 2>&1 | filter_ro_mount
>  
>  echo "touching file on read-only filesystem (should fail)"
>  touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch

This touch expects a failure message, but with this change applied, test
still failed when testing with "-o usrquota,grpquota,prjquota" mount
options as

    --- tests/generic/050.out   2018-07-01 20:36:05.796146192 +0800
    +++ /root/workspace/xfstests/results//generic/050.out.bad   2018-11-04 23:25:34.551700377 +0800
    @@ -3,7 +3,6 @@
     mounting read-only block device:
     mount: device write-protected, mounting read-only
     touching file on read-only filesystem (should fail)
    -touch: cannot touch 'SCRATCH_MNT/foo': Read-only file system
     unmounting read-only filesystem
     setting device read-write
     mounting read-write block device:

Thanks,
Eryu

> @@ -56,7 +71,7 @@ touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
>  #	http://oss.sgi.com/bugzilla/show_bug.cgi?id=807
>  #
>  echo "unmounting read-only filesystem"
> -_scratch_unmount 2>&1 | _filter_scratch
> +_scratch_unmount 2>&1 | filter_umount
>  
>  echo "setting device read-write"
>  blockdev --setrw $SCRATCH_DEV
> @@ -92,10 +107,10 @@ _scratch_unmount 2>&1 | _filter_scratch | _filter_ending_dot
>  # data recovery hack.
>  #
>  echo "mounting filesystem with -o norecovery on a read-only device:"
> -_try_scratch_mount -o norecovery 2>&1 | _filter_ro_mount
> +_try_scratch_mount -o norecovery 2>&1 | filter_ro_mount
>  
>  echo "unmounting read-only filesystem"
> -_scratch_unmount 2>&1 | _filter_scratch
> +_scratch_unmount 2>&1 | filter_umount
>  
>  echo "setting device read-write"
>  blockdev --setrw $SCRATCH_DEV
> 

Re: [PATCH 0/8] fstests: fix quota failures on xfs

[Eryu Guan]

On Thu, Nov 01, 2018 at 04:19:02PM -0700, Darrick J. Wong wrote:
> Hi all,
> 
> Several months ago I tried testing xfs with quotas enabled and was
> surprised at the number of failures.  Since then I've been on a bug hunt
> to triage the mess, and the following series fixes some of the low
> hanging fruit.  Some of these are minor behavioral variations when quota
> are enabled, and other fixes upgrade old v4 tests to work on v5
> filesystems.
> 
> To reproduce the results, run xfstests with
> 'MOUNT_OPTIONS="-o usrquota,grpquota,prjquota"' on a v5 filesystem.
> Strike the ',grpquota' on a v4 filesystem.

I merged all patches but patch 3 and 8, because ..

> 
> The first patch fixes the sporadic quota count failures when testing XFS
> with quotas by replacing the xfs_spaceman call that cleans out (only)
> speculative preallocations with a umount/mount pair that forcibly cleans
> out /all/ preallocations and delayed allocations.
> 
> The second patch fixes bugs in a couple of tests that use error
> injection to test log recovery paths but don't succeed in directly
> causing the error to trip.
> 
> The third patch works around the fact that mounting xfs with quotas
> requires a writable block device even for a ro mount, which leads to
> a slightly different error message from the mount call.

The test still fails and patch needs further tweaks.

> 
> Patch 4 and 5 fix the dump and restore tests to handle the case that
> quota inodes get sent along with dumps when quotas are enabled.
> 
> Patch 6 updates xfs/205 to work with v5 filesystems.
> 
> Patch 7 changes tests that require specific quota mount options to clear
> out the background quota mount options so that we test with /exactly/
> the set of mount options.
> 
> Patch 8 filters v5 mount options out of tests that format v4
> filesystems on the scratch device.

This patch touches _scratch_mkfs_xfs() and needs more careful testing.

Thanks,
Eryu

Re: [PATCH 0/8] fstests: fix quota failures on xfs

[Darrick J. Wong]

On Mon, Nov 05, 2018 at 12:01:49AM +0800, Eryu Guan wrote:
> On Thu, Nov 01, 2018 at 04:19:02PM -0700, Darrick J. Wong wrote:
> > Hi all,
> > 
> > Several months ago I tried testing xfs with quotas enabled and was
> > surprised at the number of failures.  Since then I've been on a bug hunt
> > to triage the mess, and the following series fixes some of the low
> > hanging fruit.  Some of these are minor behavioral variations when quota
> > are enabled, and other fixes upgrade old v4 tests to work on v5
> > filesystems.
> > 
> > To reproduce the results, run xfstests with
> > 'MOUNT_OPTIONS="-o usrquota,grpquota,prjquota"' on a v5 filesystem.
> > Strike the ',grpquota' on a v4 filesystem.
> 
> I merged all patches but patch 3 and 8, because ..
> 
> > 
> > The first patch fixes the sporadic quota count failures when testing XFS
> > with quotas by replacing the xfs_spaceman call that cleans out (only)
> > speculative preallocations with a umount/mount pair that forcibly cleans
> > out /all/ preallocations and delayed allocations.
> > 
> > The second patch fixes bugs in a couple of tests that use error
> > injection to test log recovery paths but don't succeed in directly
> > causing the error to trip.
> > 
> > The third patch works around the fact that mounting xfs with quotas
> > requires a writable block device even for a ro mount, which leads to
> > a slightly different error message from the mount call.
> 
> The test still fails and patch needs further tweaks.

I'll have a look tomorrow.

> > 
> > Patch 4 and 5 fix the dump and restore tests to handle the case that
> > quota inodes get sent along with dumps when quotas are enabled.
> > 
> > Patch 6 updates xfs/205 to work with v5 filesystems.
> > 
> > Patch 7 changes tests that require specific quota mount options to clear
> > out the background quota mount options so that we test with /exactly/
> > the set of mount options.
> > 
> > Patch 8 filters v5 mount options out of tests that format v4
> > filesystems on the scratch device.
> 
> This patch touches _scratch_mkfs_xfs() and needs more careful testing.

Yeah, I thought of something today -- each test is run in a separate
child process from ./check, which means that if we force a v4
filesystem during a test, the ./check script is going to have to know
about the rewritten MOUNT_OPTIONS if it remounts the scratch device
after the test exits.  Hmm.  Not sure if we want a big complex machinery
to do that or just detect the incompatible mount options and _notrun...

--D

> Thanks,
> Eryu

Re: [PATCH 3/8] generic/050: fix ro blockdev mount of xfs with quota

[Darrick J. Wong]

On Sun, Nov 04, 2018 at 11:32:36PM +0800, Eryu Guan wrote:
> On Thu, Nov 01, 2018 at 04:19:21PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > In XFS, mounting with quota always require a writable device.  If the
> > block device is read only, the mount fails, which fails this test.
> > Since this is expected, work around this.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  tests/generic/050 |   23 +++++++++++++++++++----
> >  1 file changed, 19 insertions(+), 4 deletions(-)
> > 
> > 
> > diff --git a/tests/generic/050 b/tests/generic/050
> > index 90e924db..33da228d 100755
> > --- a/tests/generic/050
> > +++ b/tests/generic/050
> > @@ -36,6 +36,21 @@ _require_norecovery
> >  
> >  _scratch_mkfs >/dev/null 2>&1
> >  
> > +filter_ro_mount() {
> > +	local arg=""
> > +
> > +	# Mounting with quota on XFS requires a writable fs, which means
> > +	# we fail the ro blockdev test with with EPERM.
> > +	if [ "$FSTYP" = "xfs" ] && echo "$MOUNT_OPTIONS" | grep -q quota ; then
> > +		arg="s|mount: $SCRATCH_MNT: permission denied|mount: device write-protected, mounting read-only|g"
> > +	fi
> > +	sed -e "$arg" | _filter_ro_mount
> > +}
> > +
> > +filter_umount() {
> > +	sed -e "\|umount: $SCRATCH_DEV: not mounted.|d" | _filter_scratch
> > +}
> > +
> >  #
> >  # Mark the device read-only
> >  #
> > @@ -46,7 +61,7 @@ blockdev --setro $SCRATCH_DEV
> >  # Mount it, and make sure we can't write to it, and we can unmount it again
> >  #
> >  echo "mounting read-only block device:"
> > -_try_scratch_mount 2>&1 | _filter_ro_mount
> > +_try_scratch_mount 2>&1 | filter_ro_mount
> >  
> >  echo "touching file on read-only filesystem (should fail)"
> >  touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
> 
> This touch expects a failure message, but with this change applied, test
> still failed when testing with "-o usrquota,grpquota,prjquota" mount
> options as
> 
>     --- tests/generic/050.out   2018-07-01 20:36:05.796146192 +0800
>     +++ /root/workspace/xfstests/results//generic/050.out.bad   2018-11-04 23:25:34.551700377 +0800
>     @@ -3,7 +3,6 @@
>      mounting read-only block device:
>      mount: device write-protected, mounting read-only
>      touching file on read-only filesystem (should fail)
>     -touch: cannot touch 'SCRATCH_MNT/foo': Read-only file system

Hmm, what kernel is this on?  I've never seen this problem, but maybe I
just have too many patches everywhere. :/

--D

>      unmounting read-only filesystem
>      setting device read-write
>      mounting read-write block device:
> 
> Thanks,
> Eryu
> 
> > @@ -56,7 +71,7 @@ touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
> >  #	http://oss.sgi.com/bugzilla/show_bug.cgi?id=807
> >  #
> >  echo "unmounting read-only filesystem"
> > -_scratch_unmount 2>&1 | _filter_scratch
> > +_scratch_unmount 2>&1 | filter_umount
> >  
> >  echo "setting device read-write"
> >  blockdev --setrw $SCRATCH_DEV
> > @@ -92,10 +107,10 @@ _scratch_unmount 2>&1 | _filter_scratch | _filter_ending_dot
> >  # data recovery hack.
> >  #
> >  echo "mounting filesystem with -o norecovery on a read-only device:"
> > -_try_scratch_mount -o norecovery 2>&1 | _filter_ro_mount
> > +_try_scratch_mount -o norecovery 2>&1 | filter_ro_mount
> >  
> >  echo "unmounting read-only filesystem"
> > -_scratch_unmount 2>&1 | _filter_scratch
> > +_scratch_unmount 2>&1 | filter_umount
> >  
> >  echo "setting device read-write"
> >  blockdev --setrw $SCRATCH_DEV
> > 

Re: [PATCH 3/8] generic/050: fix ro blockdev mount of xfs with quota

[Eryu Guan]

On Tue, Nov 27, 2018 at 03:19:55PM -0800, Darrick J. Wong wrote:
> On Sun, Nov 04, 2018 at 11:32:36PM +0800, Eryu Guan wrote:
> > On Thu, Nov 01, 2018 at 04:19:21PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > In XFS, mounting with quota always require a writable device.  If the
> > > block device is read only, the mount fails, which fails this test.
> > > Since this is expected, work around this.
> > > 
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > ---
> > >  tests/generic/050 |   23 +++++++++++++++++++----
> > >  1 file changed, 19 insertions(+), 4 deletions(-)
> > > 
> > > 
> > > diff --git a/tests/generic/050 b/tests/generic/050
> > > index 90e924db..33da228d 100755
> > > --- a/tests/generic/050
> > > +++ b/tests/generic/050
> > > @@ -36,6 +36,21 @@ _require_norecovery
> > >  
> > >  _scratch_mkfs >/dev/null 2>&1
> > >  
> > > +filter_ro_mount() {
> > > +	local arg=""
> > > +
> > > +	# Mounting with quota on XFS requires a writable fs, which means
> > > +	# we fail the ro blockdev test with with EPERM.
> > > +	if [ "$FSTYP" = "xfs" ] && echo "$MOUNT_OPTIONS" | grep -q quota ; then
> > > +		arg="s|mount: $SCRATCH_MNT: permission denied|mount: device write-protected, mounting read-only|g"
> > > +	fi
> > > +	sed -e "$arg" | _filter_ro_mount
> > > +}
> > > +
> > > +filter_umount() {
> > > +	sed -e "\|umount: $SCRATCH_DEV: not mounted.|d" | _filter_scratch
> > > +}
> > > +
> > >  #
> > >  # Mark the device read-only
> > >  #
> > > @@ -46,7 +61,7 @@ blockdev --setro $SCRATCH_DEV
> > >  # Mount it, and make sure we can't write to it, and we can unmount it again
> > >  #
> > >  echo "mounting read-only block device:"
> > > -_try_scratch_mount 2>&1 | _filter_ro_mount
> > > +_try_scratch_mount 2>&1 | filter_ro_mount
> > >  
> > >  echo "touching file on read-only filesystem (should fail)"
> > >  touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
> > 
> > This touch expects a failure message, but with this change applied, test
> > still failed when testing with "-o usrquota,grpquota,prjquota" mount
> > options as
> > 
> >     --- tests/generic/050.out   2018-07-01 20:36:05.796146192 +0800
> >     +++ /root/workspace/xfstests/results//generic/050.out.bad   2018-11-04 23:25:34.551700377 +0800
> >     @@ -3,7 +3,6 @@
> >      mounting read-only block device:
> >      mount: device write-protected, mounting read-only
> >      touching file on read-only filesystem (should fail)
> >     -touch: cannot touch 'SCRATCH_MNT/foo': Read-only file system
> 
> Hmm, what kernel is this on?  I've never seen this problem, but maybe I
> just have too many patches everywhere. :/

I cannot remember.. But I think the problem here is that, ro mount with
quota failed but we amended & hid the failure by filter_ro_mount, then
the 'touch' expected a ro-mounted fs, but it actually created a new file
on rootfs without hitting the EROFS error.

Thanks,
Eryu

Re: [PATCH 3/8] generic/050: fix ro blockdev mount of xfs with quota

[Darrick J. Wong]

On Wed, Nov 28, 2018 at 11:03:46AM +0800, Eryu Guan wrote:
> On Tue, Nov 27, 2018 at 03:19:55PM -0800, Darrick J. Wong wrote:
> > On Sun, Nov 04, 2018 at 11:32:36PM +0800, Eryu Guan wrote:
> > > On Thu, Nov 01, 2018 at 04:19:21PM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > > 
> > > > In XFS, mounting with quota always require a writable device.  If the
> > > > block device is read only, the mount fails, which fails this test.
> > > > Since this is expected, work around this.
> > > > 
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > > ---
> > > >  tests/generic/050 |   23 +++++++++++++++++++----
> > > >  1 file changed, 19 insertions(+), 4 deletions(-)
> > > > 
> > > > 
> > > > diff --git a/tests/generic/050 b/tests/generic/050
> > > > index 90e924db..33da228d 100755
> > > > --- a/tests/generic/050
> > > > +++ b/tests/generic/050
> > > > @@ -36,6 +36,21 @@ _require_norecovery
> > > >  
> > > >  _scratch_mkfs >/dev/null 2>&1
> > > >  
> > > > +filter_ro_mount() {
> > > > +	local arg=""
> > > > +
> > > > +	# Mounting with quota on XFS requires a writable fs, which means
> > > > +	# we fail the ro blockdev test with with EPERM.
> > > > +	if [ "$FSTYP" = "xfs" ] && echo "$MOUNT_OPTIONS" | grep -q quota ; then
> > > > +		arg="s|mount: $SCRATCH_MNT: permission denied|mount: device write-protected, mounting read-only|g"
> > > > +	fi
> > > > +	sed -e "$arg" | _filter_ro_mount
> > > > +}
> > > > +
> > > > +filter_umount() {
> > > > +	sed -e "\|umount: $SCRATCH_DEV: not mounted.|d" | _filter_scratch
> > > > +}
> > > > +
> > > >  #
> > > >  # Mark the device read-only
> > > >  #
> > > > @@ -46,7 +61,7 @@ blockdev --setro $SCRATCH_DEV
> > > >  # Mount it, and make sure we can't write to it, and we can unmount it again
> > > >  #
> > > >  echo "mounting read-only block device:"
> > > > -_try_scratch_mount 2>&1 | _filter_ro_mount
> > > > +_try_scratch_mount 2>&1 | filter_ro_mount
> > > >  
> > > >  echo "touching file on read-only filesystem (should fail)"
> > > >  touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
> > > 
> > > This touch expects a failure message, but with this change applied, test
> > > still failed when testing with "-o usrquota,grpquota,prjquota" mount
> > > options as
> > > 
> > >     --- tests/generic/050.out   2018-07-01 20:36:05.796146192 +0800
> > >     +++ /root/workspace/xfstests/results//generic/050.out.bad   2018-11-04 23:25:34.551700377 +0800
> > >     @@ -3,7 +3,6 @@
> > >      mounting read-only block device:
> > >      mount: device write-protected, mounting read-only
> > >      touching file on read-only filesystem (should fail)
> > >     -touch: cannot touch 'SCRATCH_MNT/foo': Read-only file system
> > 
> > Hmm, what kernel is this on?  I've never seen this problem, but maybe I
> > just have too many patches everywhere. :/
> 
> I cannot remember.. But I think the problem here is that, ro mount with
> quota failed but we amended & hid the failure by filter_ro_mount, then
> the 'touch' expected a ro-mounted fs, but it actually created a new file
> on rootfs without hitting the EROFS error.

ARrrggh, I forgot that most people run fstests with a rw rootfs. :/

Ok that gives me something to go on.

--D

> Thanks,
> Eryu