[PATCH 0/8] Btrfs fstests fixups

[PATCH 0/8] Btrfs fstests fixups

[David Sterba]

Hi,

I'm sending first batch of fixups on behalf of Josef.  I think most of
them have been sent to fstests@ at some point.  They have been in our CI
branch and consider them tested and working for our needs. All of them
are btrfs-specific and will not affect other filesystems.  (The change
in common/rc only moves a helper.) The last patch is a new test split
from another one.

Josef Bacik (8):
  btrfs/011: increase the runtime for replace cancel
  btrfs/012: adjust how we populate the fs to convert
  btrfs/131: don't run with subpage blocksizes
  btrfs/213: make the test more reliable
  btrfs/271: adjust failure condition
  btrfs/287,btrfs/293: filter all btrfs subvolume delete calls
  btrfs/291: remove image file after teardown
  btrfs/400: test normal qgroup operations in a compress friendly way

 check               |   6 ---
 common/rc           |   5 +++
 tests/btrfs/011     |   9 +++-
 tests/btrfs/012     |  14 +++---
 tests/btrfs/022     |  86 ++---------------------------------
 tests/btrfs/131     |   4 ++
 tests/btrfs/213     |  20 ++++-----
 tests/btrfs/271     |  11 ++---
 tests/btrfs/287     |   4 +-
 tests/btrfs/287.out |   2 +-
 tests/btrfs/291     |   2 +-
 tests/btrfs/293     |   6 +--
 tests/btrfs/293.out |   4 +-
 tests/btrfs/400     | 107 ++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/400.out |   2 +
 15 files changed, 161 insertions(+), 121 deletions(-)
 create mode 100755 tests/btrfs/400
 create mode 100644 tests/btrfs/400.out

-- 
2.42.1

[PATCH 1/8] btrfs/011: increase the runtime for replace cancel

[David Sterba]

From: Josef Bacik <josef@toxicpanda.com>

This test exercises the btrfs replace cancel path, but in order to do
this we have to have enough work to do in order to successfully cancel
the balance, otherwise the test fails because the operation has
completed before we're able to cancel.  This test has a very low pass
rate because we do not generate a large enough file system for replace
to have enough work, passing around 5% of the time.  Increase the time
spent to 10x the time we wait for the replace to start its work before
we cancel, this allows us to consistently pass this test.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/btrfs/011 | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tests/btrfs/011 b/tests/btrfs/011
index ff52ada94a17d0..d8b5a978275032 100755
--- a/tests/btrfs/011
+++ b/tests/btrfs/011
@@ -60,6 +60,7 @@ wait_time=1
 fill_scratch()
 {
 	local fssize=$1
+	local with_cancel=$2
 	local filler_pid
 
 	# Fill inline extents.
@@ -88,7 +89,11 @@ fill_scratch()
 	$XFS_IO_PROG -f -d -c "pwrite -b 64k 0 1E" "$SCRATCH_MNT/t_filler" &>\
 		$tmp.filler_result &
 	filler_pid=$!
-	sleep $((2 * $wait_time))
+	if [ "${with_cancel}" = "cancel" ]; then
+		sleep $((10 * $wait_time))
+	else
+		sleep $((2 * $wait_time))
+	fi
 	kill -KILL $filler_pid &> /dev/null
 	wait $filler_pid &> /dev/null
 
@@ -125,7 +130,7 @@ workout()
 	_scratch_mount
 	_require_fs_space $SCRATCH_MNT $((2 * 512 * 1024)) #2.5G
 
-	fill_scratch $fssize
+	fill_scratch $fssize $with_cancel
 	_run_btrfs_util_prog filesystem show -m $SCRATCH_MNT
 
 	echo -e "Replace from $source_dev to $SPARE_DEV\\n" >> $seqres.full
-- 
2.42.1

[PATCH 2/8] btrfs/012: adjust how we populate the fs to convert

[David Sterba]

From: Josef Bacik <josef@toxicpanda.com>

/lib/modules/$(uname -r)/ can have symlinks to the source tree where the
kernel was built from, which can have all sorts of stuff, which will
make the runtime for this test exceedingly long.  We're just trying to
copy some data into our tree to test with, we don't need the entire
devel tree of whatever we're doing.  Additionally VM's that aren't built
with modules will fail this test.

Update the test to use /etc, which will always exist.  Additionally use
timeout just in case there's large files or some other shenanigans so
the test doesn't run forever copying large amounts of files.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/btrfs/012 | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/tests/btrfs/012 b/tests/btrfs/012
index d9faf81ce1ad8e..7bc0c3ce59d28f 100755
--- a/tests/btrfs/012
+++ b/tests/btrfs/012
@@ -33,6 +33,8 @@ _require_non_zoned_device "${SCRATCH_DEV}"
 _require_loop
 _require_extra_fs ext4
 
+SOURCE_DIR=/etc
+BASENAME=$(basename $SOURCE_DIR)
 BLOCK_SIZE=`_get_block_size $TEST_DIR`
 
 # Create & populate an ext4 filesystem
@@ -41,9 +43,9 @@ $MKFS_EXT4_PROG -F -b $BLOCK_SIZE $SCRATCH_DEV > $seqres.full 2>&1 || \
 # Manual mount so we don't use -t btrfs or selinux context
 mount -t ext4 $SCRATCH_DEV $SCRATCH_MNT
 
-_require_fs_space $SCRATCH_MNT $(du -s /lib/modules/`uname -r` | ${AWK_PROG} '{print $1}')
+_require_fs_space $SCRATCH_MNT $(du -s $SOURCE_DIR | ${AWK_PROG} '{print $1}')
 
-cp -aR /lib/modules/`uname -r`/ $SCRATCH_MNT
+timeout 10 cp -aRP $SOURCE_DIR $SCRATCH_MNT
 _scratch_unmount
 
 # Convert it to btrfs, mount it, verify the data
@@ -51,7 +53,7 @@ $BTRFS_CONVERT_PROG $SCRATCH_DEV >> $seqres.full 2>&1 || \
 	_fail "btrfs-convert failed"
 _try_scratch_mount || _fail "Could not mount new btrfs fs"
 # (Ignore the symlinks which may be broken/nonexistent)
-diff -r /lib/modules/`uname -r`/ $SCRATCH_MNT/`uname -r`/ 2>&1 | grep -vw "source\|build"
+diff --no-dereference -r $SOURCE_DIR $SCRATCH_MNT/$BASENAME/ 2>&1
 
 # Old ext4 image file should exist & be consistent
 $E2FSCK_PROG -fn $SCRATCH_MNT/ext2_saved/image >> $seqres.full 2>&1 || \
@@ -62,12 +64,12 @@ mkdir -p $SCRATCH_MNT/mnt
 mount -o loop $SCRATCH_MNT/ext2_saved/image $SCRATCH_MNT/mnt || \
 	_fail "could not loop mount saved ext4 image"
 # Ignore the symlinks which may be broken/nonexistent
-diff -r /lib/modules/`uname -r`/ $SCRATCH_MNT/mnt/`uname -r`/ 2>&1 | grep -vw "source\|build"
+diff --no-dereference -r $SOURCE_DIR $SCRATCH_MNT/mnt/$BASENAME/ 2>&1
 umount $SCRATCH_MNT/mnt
 
 # Now put some fresh data on the btrfs fs
 mkdir -p $SCRATCH_MNT/new 
-cp -aR /lib/modules/`uname -r`/ $SCRATCH_MNT/new
+timeout 10 cp -aRP $SOURCE_DIR $SCRATCH_MNT/new
 
 _scratch_unmount
 
@@ -82,7 +84,7 @@ $E2FSCK_PROG -fn $SCRATCH_DEV >> $seqres.full 2>&1 || \
 # Mount the un-converted ext4 device & check the contents
 mount -t ext4 $SCRATCH_DEV $SCRATCH_MNT
 # (Ignore the symlinks which may be broken/nonexistent)
-diff -r /lib/modules/`uname -r`/ $SCRATCH_MNT/`uname -r`/ 2>&1 | grep -vw "source\|build"
+diff --no-dereference -r $SOURCE_DIR $SCRATCH_MNT/$BASENAME/ 2>&1
 
 _scratch_unmount
 
-- 
2.42.1

Re: [PATCH 2/8] btrfs/012: adjust how we populate the fs to convert

[Anand Jain]

On 3/6/24 00:22, David Sterba wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> /lib/modules/$(uname -r)/ can have symlinks to the source tree where the
> kernel was built from, which can have all sorts of stuff, which will
> make the runtime for this test exceedingly long.  We're just trying to
> copy some data into our tree to test with, we don't need the entire
> devel tree of whatever we're doing.  Additionally VM's that aren't built
> with modules will fail this test.
> 
> Update the test to use /etc, which will always exist.  Additionally use
> timeout just in case there's large files or some other shenanigans so
> the test doesn't run forever copying large amounts of files.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>   tests/btrfs/012 | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/btrfs/012 b/tests/btrfs/012
> index d9faf81ce1ad8e..7bc0c3ce59d28f 100755
> --- a/tests/btrfs/012
> +++ b/tests/btrfs/012
> @@ -33,6 +33,8 @@ _require_non_zoned_device "${SCRATCH_DEV}"
>   _require_loop
>   _require_extra_fs ext4
>   
> +SOURCE_DIR=/etc
> +BASENAME=$(basename $SOURCE_DIR)
>   BLOCK_SIZE=`_get_block_size $TEST_DIR`
>   
>   # Create & populate an ext4 filesystem
> @@ -41,9 +43,9 @@ $MKFS_EXT4_PROG -F -b $BLOCK_SIZE $SCRATCH_DEV > $seqres.full 2>&1 || \
>   # Manual mount so we don't use -t btrfs or selinux context
>   mount -t ext4 $SCRATCH_DEV $SCRATCH_MNT
>   
> -_require_fs_space $SCRATCH_MNT $(du -s /lib/modules/`uname -r` | ${AWK_PROG} '{print $1}')
> +_require_fs_space $SCRATCH_MNT $(du -s $SOURCE_DIR | ${AWK_PROG} '{print $1}')
>   
> -cp -aR /lib/modules/`uname -r`/ $SCRATCH_MNT
> +timeout 10 cp -aRP $SOURCE_DIR $SCRATCH_MNT

TIMEOUT_PROG

>   _scratch_unmount
>   
>   # Convert it to btrfs, mount it, verify the data
> @@ -51,7 +53,7 @@ $BTRFS_CONVERT_PROG $SCRATCH_DEV >> $seqres.full 2>&1 || \
>   	_fail "btrfs-convert failed"
>   _try_scratch_mount || _fail "Could not mount new btrfs fs"
>   # (Ignore the symlinks which may be broken/nonexistent)
> -diff -r /lib/modules/`uname -r`/ $SCRATCH_MNT/`uname -r`/ 2>&1 | grep -vw "source\|build"
> +diff --no-dereference -r $SOURCE_DIR $SCRATCH_MNT/$BASENAME/ 2>&1
>   
>   # Old ext4 image file should exist & be consistent
>   $E2FSCK_PROG -fn $SCRATCH_MNT/ext2_saved/image >> $seqres.full 2>&1 || \
> @@ -62,12 +64,12 @@ mkdir -p $SCRATCH_MNT/mnt
>   mount -o loop $SCRATCH_MNT/ext2_saved/image $SCRATCH_MNT/mnt || \
>   	_fail "could not loop mount saved ext4 image"
>   # Ignore the symlinks which may be broken/nonexistent
> -diff -r /lib/modules/`uname -r`/ $SCRATCH_MNT/mnt/`uname -r`/ 2>&1 | grep -vw "source\|build"
> +diff --no-dereference -r $SOURCE_DIR $SCRATCH_MNT/mnt/$BASENAME/ 2>&1
>   umount $SCRATCH_MNT/mnt
>   
>   # Now put some fresh data on the btrfs fs
>   mkdir -p $SCRATCH_MNT/new
> -cp -aR /lib/modules/`uname -r`/ $SCRATCH_MNT/new
> +timeout 10 cp -aRP $SOURCE_DIR $SCRATCH_MNT/new
>   

TIMEOUT_PROG

looks good.

Added for the pr.




>   _scratch_unmount
>   
> @@ -82,7 +84,7 @@ $E2FSCK_PROG -fn $SCRATCH_DEV >> $seqres.full 2>&1 || \
>   # Mount the un-converted ext4 device & check the contents
>   mount -t ext4 $SCRATCH_DEV $SCRATCH_MNT
>   # (Ignore the symlinks which may be broken/nonexistent)
> -diff -r /lib/modules/`uname -r`/ $SCRATCH_MNT/`uname -r`/ 2>&1 | grep -vw "source\|build"
> +diff --no-dereference -r $SOURCE_DIR $SCRATCH_MNT/$BASENAME/ 2>&1
>   
>   _scratch_unmount
>   

[PATCH 3/8] btrfs/131: don't run with subpage blocksizes

[David Sterba]

From: Josef Bacik <josef@toxicpanda.com>

This test requires a feature that is incompatible with subpage
blocksizes.  Check to see if that's what we're testing and simply skip
this test.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/btrfs/131 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/btrfs/131 b/tests/btrfs/131
index 1e072b285ecfea..529ee3e80f87eb 100755
--- a/tests/btrfs/131
+++ b/tests/btrfs/131
@@ -21,6 +21,10 @@ _require_btrfs_fs_feature free_space_tree
 # Zoned btrfs does not support space_cache(v1)
 _require_non_zoned_device "${SCRATCH_DEV}"
 
+_scratch_mkfs >/dev/null 2>&1
+[ "$(_get_page_size)" -gt "$(_scratch_btrfs_sectorsize)" ] && \
+	_notrun "cannot run with subpage sectorsize"
+
 mkfs_v1()
 {
 	_scratch_mkfs >/dev/null 2>&1
-- 
2.42.1

[PATCH 4/8] btrfs/213: make the test more reliable

[David Sterba]

From: Josef Bacik <josef@toxicpanda.com>

This test will write for 8 seconds and then try to balance, but for some
setups 8 seconds may be enough to fill the disk.  Instead figure out
what half the size of the disk is and write at most that many bytes, or
for 8 seconds, whichever comes first.  Then use the amount of time it
took to do the write to determine how long we should allow the balance
to continue before we attempt to cancel it.

Additionally the macro is '_notrun' not '_not_run'.  With this change
this test now does the correct thing on my ARM CI VM.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 check           |  6 ------
 common/rc       |  5 +++++
 tests/btrfs/213 | 20 ++++++++++----------
 3 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/check b/check
index 71b9fbd075223f..c6dba89b5b506e 100755
--- a/check
+++ b/check
@@ -204,12 +204,6 @@ trim_test_list()
 	rm -f $tmp.grep
 }
 
-
-_wallclock()
-{
-    date "+%s"
-}
-
 _timestamp()
 {
     local now=`date "+%T"`
diff --git a/common/rc b/common/rc
index 6cbceb7ae7c6bb..9b6dfcaaeddadb 100644
--- a/common/rc
+++ b/common/rc
@@ -6,6 +6,11 @@
 
 BC="$(type -P bc)" || BC=
 
+_wallclock()
+{
+    date "+%s"
+}
+
 _require_math()
 {
 	if [ -z "$BC" ]; then
diff --git a/tests/btrfs/213 b/tests/btrfs/213
index 6def4f6ef79acf..816041a0cc2ea0 100755
--- a/tests/btrfs/213
+++ b/tests/btrfs/213
@@ -31,23 +31,23 @@ _fixed_by_kernel_commit 1dae7e0e58b4 \
 _scratch_mkfs >> $seqres.full
 _scratch_mount
 
-runtime=8
+max_space=$(_get_total_space $SCRATCH_MNT)
+max_space=$(( max_space / 2 ))
 
-# Create enough IO so that we need around $runtime seconds to relocate it.
-#
-# Here we don't want any wrapper, as we want full control of the process.
-$XFS_IO_PROG -f -c "pwrite -D -b 1M 0 1024T" "$SCRATCH_MNT/file" &> /dev/null &
-write_pid=$!
-sleep $runtime
-kill $write_pid
-wait $write_pid
+# Create enough IO so that we need around 8 seconds to relocate it.
+start_ts=$(_wallclock)
+$TIMEOUT_PROG 8s $XFS_IO_PROG -f -c "pwrite -D -b 1M 0 $max_space" \
+	"$SCRATCH_MNT/file" > /dev/null 2>&1
+stop_ts=$(_wallclock)
+
+runtime=$(( stop_ts - start_ts ))
 
 # Unmount and mount again the fs to clear any cached data and metadata, so that
 # it's less likely balance has already finished when we try to cancel it below.
 _scratch_cycle_mount
 
 # Now balance should take at least $runtime seconds, we can cancel it at
-# $runtime/2 to ensure a success cancel.
+# $runtime/4 to ensure a success cancel.
 _run_btrfs_balance_start -d --bg "$SCRATCH_MNT"
 sleep $(($runtime / 4))
 # It's possible that balance has already completed. It's unlikely but often
-- 
2.42.1

[PATCH 5/8] btrfs/271: adjust failure condition

[David Sterba]

From: Josef Bacik <josef@toxicpanda.com>

btrfs/271 was failing with the subpage blocksize VM's.  This is because
there's an assumption made that the device error counters are
per-sector, but they're per-io.  With a 16kib pagesize and a 4k
sectorsize/nodesize the threshold was expecting 16 failed IO's, but
instead we were getting 5.

This other gotcha here is that with the tree log we will write the log
tree first, and then update the log root tree with the location of the
log tree root node.  With pagesize == nodesize this is fine, we will
only write the log tree root node.  However with subpage blocksize both
of these nodes could be on the same page, and thus they are both written
out during that initial write.  When we update the pointer for the log
root tree we will COW the log root tree root node and submit another IO,
resulting in 3 metadata IO's instead of 2.

Fix the failure case to be < 4 blocks, which is the minimum number of
IO's we should be seeing.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/btrfs/271 | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tests/btrfs/271 b/tests/btrfs/271
index 273799f17924c0..a342af3aed44cf 100755
--- a/tests/btrfs/271
+++ b/tests/btrfs/271
@@ -25,10 +25,6 @@ _scratch_mount
 
 dev2=`echo $SCRATCH_DEV_POOL | $AWK_PROG '{print $2}'`
 
-pagesize=$(_get_page_size)
-blocksize=$(_get_block_size $SCRATCH_MNT)
-sectors_per_page=$(($pagesize / $blocksize))
-
 _allow_fail_make_request
 
 echo "Step 1: writing with one failing mirror:"
@@ -36,9 +32,14 @@ _bdev_fail_make_request $SCRATCH_DEV 1
 $XFS_IO_PROG -f -c "pwrite -W -S 0xaa 0 8K" $SCRATCH_MNT/foobar | _filter_xfs_io
 _bdev_fail_make_request $SCRATCH_DEV 0
 
+# btrfs counts errors per IO, assuming the data is merged that'll be 1 IO, then
+# the log tree block and then the log root tree block and then the super block.
+# We should see at least 4 failed IO's, but with subpage blocksize we could see
+# more if the log blocks end up on the same page, or if the data IO gets split
+# at all.
 errs=$($BTRFS_UTIL_PROG device stats $SCRATCH_DEV | \
 	$AWK_PROG '/write_io_errs/ { print $2 }')
-if [ $errs -ne $((4 * $sectors_per_page)) ]; then
+if [ $errs -lt 4 ]; then
         _fail "Errors: $errs expected: 4"
 fi
 
-- 
2.42.1

[PATCH 6/8] btrfs/287,btrfs/293: filter all btrfs subvolume delete calls

[David Sterba]

From: Josef Bacik <josef@toxicpanda.com>

Some of our btrfs subvolume delete calls get put into the golden output,
and many of them simply _filter_scratch.  This works fine, but we
recently changed btrfs subvolume delete output, and it would have been
nice to simply filter this in one place.  We have a
_filter_btrfs_subvol_delete helper, but it's only used in one place.
Fix all of these uses to call _filter_btrfs_subvol_delete, this will
allow for follow up fixes against _filter_btrfs_subvol_delete itself to
deal with changed output.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/btrfs/287     | 4 ++--
 tests/btrfs/287.out | 2 +-
 tests/btrfs/293     | 6 +++---
 tests/btrfs/293.out | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/btrfs/287 b/tests/btrfs/287
index 04871d46036aa2..64e6ef35250c8e 100755
--- a/tests/btrfs/287
+++ b/tests/btrfs/287
@@ -9,7 +9,7 @@
 . ./common/preamble
 _begin_fstest auto quick snapshot clone punch logical_resolve
 
-. ./common/filter
+. ./common/filter.btrfs
 . ./common/reflink
 
 _supported_fs btrfs
@@ -148,7 +148,7 @@ echo "resolve second extent with ignore offset option:"
 query_logical_ino -o $second_extent_bytenr | filter_snapshot_ids
 
 # Now delete the first snapshot and repeat the last 2 queries.
-$BTRFS_UTIL_PROG subvolume delete -C $SCRATCH_MNT/snap1 | _filter_scratch
+$BTRFS_UTIL_PROG subvolume delete -C $SCRATCH_MNT/snap1 | _filter_btrfs_subvol_delete
 
 # Query the second extent with an offset of 0, should return file offsets 12M
 # and 20M for the default subvolume (root 5) and file offsets 4M, 12M and 20M
diff --git a/tests/btrfs/287.out b/tests/btrfs/287.out
index 0d69473364fa31..30eac8fa444c8f 100644
--- a/tests/btrfs/287.out
+++ b/tests/btrfs/287.out
@@ -79,7 +79,7 @@ inode 257 offset 4194304 snap1
 inode 257 offset 20971520 root 5
 inode 257 offset 12582912 root 5
 inode 257 offset 5242880 root 5
-Delete subvolume (commit): 'SCRATCH_MNT/snap1'
+Delete subvolume 'SCRATCH_MNT/snap1'
 resolve second extent:
 inode 257 offset 20971520 snap2
 inode 257 offset 12582912 snap2
diff --git a/tests/btrfs/293 b/tests/btrfs/293
index cded956468ee9a..06f96dc414b05b 100755
--- a/tests/btrfs/293
+++ b/tests/btrfs/293
@@ -18,7 +18,7 @@ _cleanup()
 	test -n "$swap_file" && swapoff $swap_file &> /dev/null
 }
 
-. ./common/filter
+. ./common/filter.btrfs
 
 _supported_fs btrfs
 _fixed_by_kernel_commit deccae40e4b3 \
@@ -40,7 +40,7 @@ echo "Activating swap file... (should fail due to snapshots)"
 _swapon_file $swap_file 2>&1 | _filter_scratch
 
 echo "Deleting first snapshot..."
-$BTRFS_UTIL_PROG subvolume delete -c $SCRATCH_MNT/snap1 | _filter_scratch
+$BTRFS_UTIL_PROG subvolume delete -c $SCRATCH_MNT/snap1 | _filter_btrfs_subvol_delete
 
 # We deleted the snapshot and committed the transaction used to delete it (-c),
 # but all its extents are actually only deleted in the background, by the cleaner
@@ -55,7 +55,7 @@ echo "Activating swap file... (should fail due to snapshot)"
 _swapon_file $swap_file 2>&1 | _filter_scratch
 
 echo "Deleting second snapshot..."
-$BTRFS_UTIL_PROG subvolume delete -c $SCRATCH_MNT/snap2 | _filter_scratch
+$BTRFS_UTIL_PROG subvolume delete -c $SCRATCH_MNT/snap2 | _filter_btrfs_subvol_delete
 
 echo "Remounting and waiting for cleaner thread to remove the second snapshot..."
 _scratch_remount commit=1
diff --git a/tests/btrfs/293.out b/tests/btrfs/293.out
index 2ac1663281947a..fd04ac9139b849 100644
--- a/tests/btrfs/293.out
+++ b/tests/btrfs/293.out
@@ -6,12 +6,12 @@ Create a snapshot of 'SCRATCH_MNT' in 'SCRATCH_MNT/snap2'
 Activating swap file... (should fail due to snapshots)
 swapon: SCRATCH_MNT/swapfile: swapon failed: Invalid argument
 Deleting first snapshot...
-Delete subvolume (commit): 'SCRATCH_MNT/snap1'
+Delete subvolume 'SCRATCH_MNT/snap1'
 Remounting and waiting for cleaner thread to remove the first snapshot...
 Activating swap file... (should fail due to snapshot)
 swapon: SCRATCH_MNT/swapfile: swapon failed: Invalid argument
 Deleting second snapshot...
-Delete subvolume (commit): 'SCRATCH_MNT/snap2'
+Delete subvolume 'SCRATCH_MNT/snap2'
 Remounting and waiting for cleaner thread to remove the second snapshot...
 Activating swap file...
 Disabling swap file...
-- 
2.42.1

[PATCH 7/8] btrfs/291: remove image file after teardown

[David Sterba]

From: Josef Bacik <josef@toxicpanda.com>

LVM doesn't like it when you remove the file out from underneath the
backing device.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/btrfs/291 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/btrfs/291 b/tests/btrfs/291
index c5947133239940..bfffb845fea740 100755
--- a/tests/btrfs/291
+++ b/tests/btrfs/291
@@ -20,9 +20,9 @@ _cleanup()
 {
 	cd /
 	_log_writes_cleanup &> /dev/null
-	rm -f $img
 	$LVM_PROG vgremove -f -y $vgname >>$seqres.full 2>&1
 	losetup -d $loop_dev >>$seqres.full 2>&1
+	rm -f $img
 	_restore_fsverity_signatures
 }
 
-- 
2.42.1

[PATCH 8/8] btrfs/400: test normal qgroup operations in a compress friendly way

[David Sterba]

From: Josef Bacik <josef@toxicpanda.com>

btrfs/022 currently fails if you are testing with -o compress because it
does a limit exceed test which will pass with compression on.

However the other functionality this test tests is completely acceptable
with compression enabled.  Handle this by breaking the test into two
tests, one that simply tests the qgroup exceed limits test that requires
no compression, and the rest of the tests that do not have the no
compression restriction.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/btrfs/022     |  86 ++---------------------------------
 tests/btrfs/400     | 107 ++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/400.out |   2 +
 3 files changed, 112 insertions(+), 83 deletions(-)
 create mode 100755 tests/btrfs/400
 create mode 100644 tests/btrfs/400.out

diff --git a/tests/btrfs/022 b/tests/btrfs/022
index b1ef2fdf787640..32ad80bf9c64e8 100755
--- a/tests/btrfs/022
+++ b/tests/btrfs/022
@@ -4,7 +4,7 @@
 #
 # FS QA Test No. 022
 #
-# Test the basic functionality of qgroups
+# Test the basic qgroup exceed case
 #
 . ./common/preamble
 _begin_fstest auto qgroup limit
@@ -17,59 +17,8 @@ _require_scratch
 _require_qgroup_rescan
 _require_btrfs_qgroup_report
 
-# Test to make sure we can actually turn it on and it makes sense
-_basic_test()
-{
-	echo "=== basic test ===" >> $seqres.full
-	_run_btrfs_util_prog subvolume create $SCRATCH_MNT/a
-	_run_btrfs_util_prog quota enable $SCRATCH_MNT/a
-	_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
-	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT a)
-	$BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid >> \
-		$seqres.full 2>&1
-	[ $? -eq 0 ] || _fail "couldn't find our subvols quota group"
-	run_check $FSSTRESS_PROG -d $SCRATCH_MNT/a -w -p 1 -n 2000 \
-		$FSSTRESS_AVOID
-	_run_btrfs_util_prog subvolume snapshot $SCRATCH_MNT/a \
-		$SCRATCH_MNT/b
-
-	# the shared values of both the original subvol and snapshot should
-	# match
-	a_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
-	a_shared=$(echo $a_shared | $AWK_PROG '{ print $2 }')
-	echo "subvol a id=$subvolid" >> $seqres.full
-	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT b)
-	echo "subvol b id=$subvolid" >> $seqres.full
-	b_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
-	b_shared=$(echo $b_shared | $AWK_PROG '{ print $2 }')
-	$BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT >> $seqres.full
-	[ $b_shared -eq $a_shared ] || _fail "shared values don't match"
-}
-
-#enable quotas, do some work, check our values and then rescan and make sure we
-#come up with the same answer
-_rescan_test()
-{
-	echo "=== rescan test ===" >> $seqres.full
-	# first with a blank subvol
-	_run_btrfs_util_prog subvolume create $SCRATCH_MNT/a
-	_run_btrfs_util_prog quota enable $SCRATCH_MNT/a
-	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT a)
-	run_check $FSSTRESS_PROG -d $SCRATCH_MNT/a -w -p 1 -n 2000 \
-		$FSSTRESS_AVOID
-	sync
-	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
-	echo "qgroup values before rescan: $output" >> $seqres.full
-	refer=$(echo $output | $AWK_PROG '{ print $2 }')
-	excl=$(echo $output | $AWK_PROG '{ print $3 }')
-	_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
-	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
-	echo "qgroup values after rescan: $output" >> $seqres.full
-	[ $refer -eq $(echo $output | $AWK_PROG '{ print $2 }') ] || \
-		_fail "reference values don't match after rescan"
-	[ $excl -eq $(echo $output | $AWK_PROG '{ print $3 }') ] || \
-		_fail "exclusive values don't match after rescan"
-}
+# This test requires specific data usage, skip if we have compression enabled
+_require_no_compress
 
 #basic exceed limit testing
 _limit_test_exceed()
@@ -82,43 +31,14 @@ _limit_test_exceed()
 	_ddt of=$SCRATCH_MNT/a/file bs=10M count=1 >> $seqres.full 2>&1
 	[ $? -ne 0 ] || _fail "quota should have limited us"
 }
-
-#basic noexceed limit testing
-_limit_test_noexceed()
-{
-	echo "=== limit not exceed test ===" >> $seqres.full
-	_run_btrfs_util_prog subvolume create $SCRATCH_MNT/a
-	_run_btrfs_util_prog quota enable $SCRATCH_MNT
-	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT a)
-	_run_btrfs_util_prog qgroup limit 5M 0/$subvolid $SCRATCH_MNT
-	_ddt of=$SCRATCH_MNT/a/file bs=4M count=1 >> $seqres.full 2>&1
-	[ $? -eq 0 ] || _fail "should have been allowed to write"
-}
-
 units=`_btrfs_qgroup_units`
 
-_scratch_mkfs > /dev/null 2>&1
-_scratch_mount
-_basic_test
-_scratch_unmount
-_check_scratch_fs
-
-_scratch_mkfs > /dev/null 2>&1
-_scratch_mount
-_rescan_test
-_scratch_unmount
-_check_scratch_fs
-
 _scratch_mkfs > /dev/null 2>&1
 _scratch_mount
 _limit_test_exceed
 _scratch_unmount
 _check_scratch_fs
 
-_scratch_mkfs > /dev/null 2>&1
-_scratch_mount
-_limit_test_noexceed
-
 # success, all done
 echo "Silence is golden"
 status=0
diff --git a/tests/btrfs/400 b/tests/btrfs/400
new file mode 100755
index 00000000000000..ad0363fb72d186
--- /dev/null
+++ b/tests/btrfs/400
@@ -0,0 +1,107 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Facebook.  All Rights Reserved.
+#
+# FS QA Test No. 400
+#
+# Test qgroups to validate the creation works, the counters are sane, rescan
+# works, and we do not get failures when we write less than the limit amount.
+#
+. ./common/preamble
+_begin_fstest auto qgroup limit
+
+# Import common functions.
+. ./common/filter
+
+_supported_fs btrfs
+_require_scratch
+_require_btrfs_qgroup_report
+
+# Test to make sure we can actually turn it on and it makes sense
+_basic_test()
+{
+	echo "=== basic test ===" >> $seqres.full
+	_run_btrfs_util_prog subvolume create $SCRATCH_MNT/a
+	_run_btrfs_util_prog quota enable $SCRATCH_MNT/a
+	_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
+	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT a)
+	$BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid >> \
+		$seqres.full 2>&1
+	[ $? -eq 0 ] || _fail "couldn't find our subvols quota group"
+	run_check $FSSTRESS_PROG -d $SCRATCH_MNT/a -w -p 1 -n 2000 \
+		$FSSTRESS_AVOID
+	_run_btrfs_util_prog subvolume snapshot $SCRATCH_MNT/a \
+		$SCRATCH_MNT/b
+
+	# the shared values of both the original subvol and snapshot should
+	# match
+	a_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
+	a_shared=$(echo $a_shared | $AWK_PROG '{ print $2 }')
+	echo "subvol a id=$subvolid" >> $seqres.full
+	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT b)
+	echo "subvol b id=$subvolid" >> $seqres.full
+	b_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
+	b_shared=$(echo $b_shared | $AWK_PROG '{ print $2 }')
+	$BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT >> $seqres.full
+	[ $b_shared -eq $a_shared ] || _fail "shared values don't match"
+}
+
+#enable quotas, do some work, check our values and then rescan and make sure we
+#come up with the same answer
+_rescan_test()
+{
+	echo "=== rescan test ===" >> $seqres.full
+	# first with a blank subvol
+	_run_btrfs_util_prog subvolume create $SCRATCH_MNT/a
+	_run_btrfs_util_prog quota enable $SCRATCH_MNT/a
+	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT a)
+	run_check $FSSTRESS_PROG -d $SCRATCH_MNT/a -w -p 1 -n 2000 \
+		$FSSTRESS_AVOID
+	sync
+	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
+	echo "qgroup values before rescan: $output" >> $seqres.full
+	refer=$(echo $output | $AWK_PROG '{ print $2 }')
+	excl=$(echo $output | $AWK_PROG '{ print $3 }')
+	_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
+	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
+	echo "qgroup values after rescan: $output" >> $seqres.full
+	[ $refer -eq $(echo $output | $AWK_PROG '{ print $2 }') ] || \
+		_fail "reference values don't match after rescan"
+	[ $excl -eq $(echo $output | $AWK_PROG '{ print $3 }') ] || \
+		_fail "exclusive values don't match after rescan"
+}
+
+#basic noexceed limit testing
+_limit_test_noexceed()
+{
+	echo "=== limit not exceed test ===" >> $seqres.full
+	_run_btrfs_util_prog subvolume create $SCRATCH_MNT/a
+	_run_btrfs_util_prog quota enable $SCRATCH_MNT
+	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT a)
+	_run_btrfs_util_prog qgroup limit 5M 0/$subvolid $SCRATCH_MNT
+	_ddt of=$SCRATCH_MNT/a/file bs=4M count=1 >> $seqres.full 2>&1
+	[ $? -eq 0 ] || _fail "should have been allowed to write"
+}
+
+units=`_btrfs_qgroup_units`
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+_basic_test
+_scratch_unmount
+_check_scratch_fs
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+_rescan_test
+_scratch_unmount
+_check_scratch_fs
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+_limit_test_noexceed
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/btrfs/400.out b/tests/btrfs/400.out
new file mode 100644
index 00000000000000..c940c6206bcd81
--- /dev/null
+++ b/tests/btrfs/400.out
@@ -0,0 +1,2 @@
+QA output created by 400
+Silence is golden
-- 
2.42.1

Re: [PATCH 0/8] Btrfs fstests fixups

[Anand Jain]

Looks good.

Reviewed-by: Anand Jain <anand.jain@oracle.com>

for the whole series.

Staged for the PR

Thanks, Anand

On 3/6/24 00:21, David Sterba wrote:
> Hi,
> 
> I'm sending first batch of fixups on behalf of Josef.  I think most of
> them have been sent to fstests@ at some point.  They have been in our CI
> branch and consider them tested and working for our needs. All of them
> are btrfs-specific and will not affect other filesystems.  (The change
> in common/rc only moves a helper.) The last patch is a new test split
> from another one.
> 
> Josef Bacik (8):
>    btrfs/011: increase the runtime for replace cancel
>    btrfs/012: adjust how we populate the fs to convert
>    btrfs/131: don't run with subpage blocksizes
>    btrfs/213: make the test more reliable
>    btrfs/271: adjust failure condition
>    btrfs/287,btrfs/293: filter all btrfs subvolume delete calls
>    btrfs/291: remove image file after teardown
>    btrfs/400: test normal qgroup operations in a compress friendly way
> 
>   check               |   6 ---
>   common/rc           |   5 +++
>   tests/btrfs/011     |   9 +++-
>   tests/btrfs/012     |  14 +++---
>   tests/btrfs/022     |  86 ++---------------------------------
>   tests/btrfs/131     |   4 ++
>   tests/btrfs/213     |  20 ++++-----
>   tests/btrfs/271     |  11 ++---
>   tests/btrfs/287     |   4 +-
>   tests/btrfs/287.out |   2 +-
>   tests/btrfs/291     |   2 +-
>   tests/btrfs/293     |   6 +--
>   tests/btrfs/293.out |   4 +-
>   tests/btrfs/400     | 107 ++++++++++++++++++++++++++++++++++++++++++++
>   tests/btrfs/400.out |   2 +
>   15 files changed, 161 insertions(+), 121 deletions(-)
>   create mode 100755 tests/btrfs/400
>   create mode 100644 tests/btrfs/400.out
>