* [PATCH 0/2] btrfs/301: test fixes for squotas vs other features
@ 2023-11-20 18:45 Boris Burkov
2023-11-20 18:45 ` [PATCH 1/2] btrfs/301: fix hardcoded subvolids Boris Burkov
2023-11-20 18:45 ` [PATCH 2/2] btrfs/301: require_no_compress Boris Burkov
0 siblings, 2 replies; 5+ messages in thread
From: Boris Burkov @ 2023-11-20 18:45 UTC (permalink / raw)
To: linux-btrfs, kernel-team, fstests
the squotas targeting tests in btrfs/301 do not currently pass test runs
with noholes and compression enabled. Noholes can be fixed relatively
easily. Skip compression, it is covered anyway by fsck on the rest of
the tests.
Boris Burkov (2):
btrfs/301: fix hardcoded subvolids
btrfs/301: require_no_compress
tests/btrfs/301 | 139 +++++++++++++++++++++++++-----------------------
1 file changed, 71 insertions(+), 68 deletions(-)
--
2.42.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] btrfs/301: fix hardcoded subvolids
2023-11-20 18:45 [PATCH 0/2] btrfs/301: test fixes for squotas vs other features Boris Burkov
@ 2023-11-20 18:45 ` Boris Burkov
2023-11-20 19:45 ` Filipe Manana
2023-11-20 18:45 ` [PATCH 2/2] btrfs/301: require_no_compress Boris Burkov
1 sibling, 1 reply; 5+ messages in thread
From: Boris Burkov @ 2023-11-20 18:45 UTC (permalink / raw)
To: linux-btrfs, kernel-team, fstests
Hardcoded subvolids break noholes test runs, so change the test to use
_btrfs_get_subvolid instead of assuming 256, 257, etc...
Signed-off-by: Boris Burkov <boris@bur.io>
---
tests/btrfs/301 | 138 ++++++++++++++++++++++++------------------------
1 file changed, 70 insertions(+), 68 deletions(-)
diff --git a/tests/btrfs/301 b/tests/btrfs/301
index 7a0b4c0e1..5bb6b16a6 100755
--- a/tests/btrfs/301
+++ b/tests/btrfs/301
@@ -172,25 +172,27 @@ prepare()
_scratch_mount
enable_quota "s"
$BTRFS_UTIL_PROG subvolume create $subv >> $seqres.full
- set_subvol_limit 256 $limit
- check_subvol_usage 256 0
+ subvid=$(_btrfs_get_subvolid $SCRATCH_MNT subv)
+ set_subvol_limit $subvid $limit
+ check_subvol_usage $subvid 0
# Create a bunch of little filler files to generate several levels in
# the btree, to make snapshotting sharing scenarios complex enough.
$FIO_PROG $prep_fio_config --output=$fio_out
- check_subvol_usage 256 $total_fill
+ check_subvol_usage $subvid $total_fill
# Create a single file whose extents we will explicitly share/unshare.
do_write $subv/f $ext_sz
- check_subvol_usage 256 $(($total_fill + $ext_sz))
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
}
prepare_snapshotted()
{
prepare
$BTRFS_UTIL_PROG subvolume snapshot $subv $snap >> $seqres.full
- check_subvol_usage 256 $(($total_fill + $ext_sz))
- check_subvol_usage 257 0
+ snapid=$(_btrfs_get_subvolid $SCRATCH_MNT snap)
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
+ check_subvol_usage $snapid 0
}
prepare_nested()
@@ -198,13 +200,13 @@ prepare_nested()
prepare
$BTRFS_UTIL_PROG qgroup create 1/100 $SCRATCH_MNT
$BTRFS_UTIL_PROG qgroup limit $limit 1/100 $SCRATCH_MNT
- $BTRFS_UTIL_PROG qgroup assign 0/256 1/100 $SCRATCH_MNT >> $seqres.full
+ $BTRFS_UTIL_PROG qgroup assign 0/$subvid 1/100 $SCRATCH_MNT >> $seqres.full
$BTRFS_UTIL_PROG subvolume create $nested >> $seqres.full
do_write $nested/f $ext_sz
- check_subvol_usage 257 $ext_sz
- check_subvol_usage 256 $(($total_fill + $ext_sz))
- local subv_usage=$(get_subvol_usage 256)
- local nested_usage=$(get_subvol_usage 257)
+ check_subvol_usage $snapid $ext_sz
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
+ local subv_usage=$(get_subvol_usage $subvid)
+ local nested_usage=$(get_subvol_usage $snapid)
check_qgroup_usage 1/100 $(($subv_usage + $nested_usage))
}
@@ -214,8 +216,8 @@ basic_accounting()
echo "basic accounting"
prepare
rm $subv/f
- check_subvol_usage 256 $total_fill
- cycle_mount_check_subvol_usage 256 $total_fill
+ check_subvol_usage $subvid $total_fill
+ cycle_mount_check_subvol_usage $subvid $total_fill
do_write $subv/tmp 512M
rm $subv/tmp
do_write $subv/tmp 512M
@@ -245,19 +247,19 @@ snapshot_accounting()
echo "snapshot accounting"
prepare_snapshotted
touch $snap/f
- check_subvol_usage 256 $(($total_fill + $ext_sz))
- check_subvol_usage 257 0
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
+ check_subvol_usage $snapid 0
do_write $snap/f $ext_sz
- check_subvol_usage 256 $(($total_fill + $ext_sz))
- check_subvol_usage 257 $ext_sz
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
+ check_subvol_usage $snapid $ext_sz
rm $snap/f
- check_subvol_usage 256 $(($total_fill + $ext_sz))
- check_subvol_usage 257 0
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
+ check_subvol_usage $snapid 0
rm $subv/f
- check_subvol_usage 256 $total_fill
- check_subvol_usage 257 0
- cycle_mount_check_subvol_usage 256 $total_fill
- check_subvol_usage 257 0
+ check_subvol_usage $subvid $total_fill
+ check_subvol_usage $snapid 0
+ cycle_mount_check_subvol_usage $subvid $total_fill
+ check_subvol_usage $snapid 0
_scratch_unmount
}
@@ -267,14 +269,14 @@ delete_snapshot_src_ref()
echo "delete src ref first"
prepare_snapshotted
rm $subv/f
- check_subvol_usage 256 $(($total_fill + $ext_sz))
- check_subvol_usage 257 0
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
+ check_subvol_usage $snapid 0
rm $snap/f
trigger_cleaner
- check_subvol_usage 256 $total_fill
- check_subvol_usage 257 0
- cycle_mount_check_subvol_usage 256 $total_fill
- check_subvol_usage 257 0
+ check_subvol_usage $subvid $total_fill
+ check_subvol_usage $snapid 0
+ cycle_mount_check_subvol_usage $subvid $total_fill
+ check_subvol_usage $snapid 0
_scratch_unmount
}
@@ -284,13 +286,13 @@ delete_snapshot_ref()
echo "delete snapshot ref first"
prepare_snapshotted
rm $snap/f
- check_subvol_usage 256 $(($total_fill + $ext_sz))
- check_subvol_usage 257 0
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
+ check_subvol_usage $snapid 0
rm $subv/f
- check_subvol_usage 256 $total_fill
- check_subvol_usage 257 0
- cycle_mount_check_subvol_usage 256 $total_fill
- check_subvol_usage 257 0
+ check_subvol_usage $subvid $total_fill
+ check_subvol_usage $snapid 0
+ cycle_mount_check_subvol_usage $subvid $total_fill
+ check_subvol_usage $snapid 0
_scratch_unmount
}
@@ -300,18 +302,18 @@ delete_snapshot_src()
echo "delete snapshot src first"
prepare_snapshotted
$BTRFS_UTIL_PROG subvolume delete $subv >> $seqres.full
- check_subvol_usage 256 $(($total_fill + $ext_sz))
- check_subvol_usage 257 0
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
+ check_subvol_usage $snapid 0
rm $snap/f
trigger_cleaner
- check_subvol_usage 256 $total_fill
- check_subvol_usage 257 0
+ check_subvol_usage $subvid $total_fill
+ check_subvol_usage $snapid 0
$BTRFS_UTIL_PROG subvolume delete $snap >> $seqres.full
trigger_cleaner
- check_subvol_usage 256 0
- check_subvol_usage 257 0
- cycle_mount_check_subvol_usage 256 0
- check_subvol_usage 257 0
+ check_subvol_usage $subvid 0
+ check_subvol_usage $snapid 0
+ cycle_mount_check_subvol_usage $subvid 0
+ check_subvol_usage $snapid 0
_scratch_unmount
}
@@ -321,12 +323,12 @@ delete_snapshot()
echo "delete snapshot first"
prepare_snapshotted
$BTRFS_UTIL_PROG subvolume delete $snap >> $seqres.full
- check_subvol_usage 256 $(($total_fill + $ext_sz))
- check_subvol_usage 257 0
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
+ check_subvol_usage $snapid 0
$BTRFS_UTIL_PROG subvolume delete $subv >> $seqres.full
trigger_cleaner
- check_subvol_usage 256 0
- check_subvol_usage 257 0
+ check_subvol_usage $subvid 0
+ check_subvol_usage $snapid 0
_scratch_unmount
}
@@ -337,16 +339,16 @@ nested_accounting()
echo "nested accounting"
prepare_nested
rm $subv/f
- check_subvol_usage 256 $total_fill
- check_subvol_usage 257 $ext_sz
- local subv_usage=$(get_subvol_usage 256)
- local nested_usage=$(get_subvol_usage 257)
+ check_subvol_usage $subvid $total_fill
+ check_subvol_usage $snapid $ext_sz
+ local subv_usage=$(get_subvol_usage $subvid)
+ local nested_usage=$(get_subvol_usage $snapid)
check_qgroup_usage 1/100 $(($subv_usage + $nested_usage))
rm $nested/f
- check_subvol_usage 256 $total_fill
- check_subvol_usage 257 0
- subv_usage=$(get_subvol_usage 256)
- nested_usage=$(get_subvol_usage 257)
+ check_subvol_usage $subvid $total_fill
+ check_subvol_usage $snapid 0
+ subv_usage=$(get_subvol_usage $subvid)
+ nested_usage=$(get_subvol_usage $snapid)
check_qgroup_usage 1/100 $(($subv_usage + $nested_usage))
do_enospc_falloc $nested/large_falloc 2G
do_enospc_write $nested/large 2G
@@ -365,21 +367,21 @@ enable_mature()
# we did before enabling.
sync
enable_quota "s"
- set_subvol_limit 256 $limit
+ set_subvol_limit $subvid $limit
_scratch_cycle_mount
- usage=$(get_subvol_usage 256)
+ usage=$(get_subvol_usage $subvid)
[ $usage -lt $ext_sz ] || \
echo "captured usage from before enable $usage >= $ext_sz"
do_write $subv/g $ext_sz
- usage=$(get_subvol_usage 256)
+ usage=$(get_subvol_usage $subvid)
[ $usage -lt $ext_sz ] && \
echo "failed to capture usage after enable $usage < $ext_sz"
- check_subvol_usage 256 $ext_sz
+ check_subvol_usage $subvid $ext_sz
rm $subv/f
- check_subvol_usage 256 $ext_sz
+ check_subvol_usage $subvid $ext_sz
_scratch_cycle_mount
rm $subv/g
- check_subvol_usage 256 0
+ check_subvol_usage $subvid 0
_scratch_unmount
}
@@ -394,7 +396,7 @@ reflink_accounting()
_cp_reflink $subv/f $subv/f.i
done
# Confirm that there is no additional data usage from the reflinks.
- check_subvol_usage 256 $(($total_fill + $ext_sz))
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
_scratch_unmount
}
@@ -404,11 +406,11 @@ delete_reflink_src_ref()
echo "delete reflink src ref"
prepare
_cp_reflink $subv/f $subv/f.link
- check_subvol_usage 256 $(($total_fill + $ext_sz))
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
rm $subv/f
- check_subvol_usage 256 $(($total_fill + $ext_sz))
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
rm $subv/f.link
- check_subvol_usage 256 $(($total_fill))
+ check_subvol_usage $subvid $(($total_fill))
_scratch_unmount
}
@@ -418,11 +420,11 @@ delete_reflink_ref()
echo "delete reflink ref"
prepare
_cp_reflink $subv/f $subv/f.link
- check_subvol_usage 256 $(($total_fill + $ext_sz))
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
rm $subv/f.link
- check_subvol_usage 256 $(($total_fill + $ext_sz))
+ check_subvol_usage $subvid $(($total_fill + $ext_sz))
rm $subv/f
- check_subvol_usage 256 $(($total_fill))
+ check_subvol_usage $subvid $(($total_fill))
_scratch_unmount
}
--
2.42.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] btrfs/301: require_no_compress
2023-11-20 18:45 [PATCH 0/2] btrfs/301: test fixes for squotas vs other features Boris Burkov
2023-11-20 18:45 ` [PATCH 1/2] btrfs/301: fix hardcoded subvolids Boris Burkov
@ 2023-11-20 18:45 ` Boris Burkov
1 sibling, 0 replies; 5+ messages in thread
From: Boris Burkov @ 2023-11-20 18:45 UTC (permalink / raw)
To: linux-btrfs, kernel-team, fstests
btrfs/301 makes detailed size calculations to test squota edge cases
which rely on assumptions that break down with compression enabled.
Fix it by disabling the test with compression. Compression + squotas
still gets quite solid test coverage via squotas support in fsck and
normal compression enabled fstests runs.
Signed-off-by: Boris Burkov <boris@bur.io>
---
tests/btrfs/301 | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/btrfs/301 b/tests/btrfs/301
index 5bb6b16a6..165c6e417 100755
--- a/tests/btrfs/301
+++ b/tests/btrfs/301
@@ -22,6 +22,7 @@ _require_cp_reflink
_require_btrfs_command inspect-internal dump-tree
_require_xfs_io_command "falloc"
_require_scratch_enable_simple_quota
+_require_no_compress
subv=$SCRATCH_MNT/subv
nested=$SCRATCH_MNT/subv/nested
--
2.42.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] btrfs/301: fix hardcoded subvolids
2023-11-20 18:45 ` [PATCH 1/2] btrfs/301: fix hardcoded subvolids Boris Burkov
@ 2023-11-20 19:45 ` Filipe Manana
2023-11-20 19:53 ` Boris Burkov
0 siblings, 1 reply; 5+ messages in thread
From: Filipe Manana @ 2023-11-20 19:45 UTC (permalink / raw)
To: Boris Burkov; +Cc: linux-btrfs, kernel-team, fstests
On Mon, Nov 20, 2023 at 6:44 PM Boris Burkov <boris@bur.io> wrote:
>
> Hardcoded subvolids break noholes test runs, so change the test to use
> _btrfs_get_subvolid instead of assuming 256, 257, etc...
How exactly does no-holes affect the assigned IDs to subvolumes?
It's a default feature for quite a while and the test passes with it
and without it (-O ^no-holes).
Aren't you confusing this with enabling or disabling features that add
some tree?
Like disabling the free space tree (which is a default for quite a
while now), for example.
For me it fails with -O ^free-space-tree, and it fails even with this
patch applied:
$ MKFS_OPTIONS="-O ^free-space-tree" ./check btrfs/301
FSTYP -- btrfs
PLATFORM -- Linux/x86_64 debian0 6.6.0-rc5-btrfs-next-140+ #1 SMP
PREEMPT_DYNAMIC Thu Oct 19 16:55:42 WEST 2023
MKFS_OPTIONS -- -O ^free-space-tree /dev/sdb
MOUNT_OPTIONS -- /dev/sdb /home/fdmanana/btrfs-tests/scratch_1
btrfs/301 70s ... - output mismatch (see
/home/fdmanana/git/hub/xfstests/results//btrfs/301.out.bad)
--- tests/btrfs/301.out 2023-10-18 23:29:06.029292800 +0100
+++ /home/fdmanana/git/hub/xfstests/results//btrfs/301.out.bad
2023-11-20 19:33:04.988824928 +0000
@@ -13,6 +13,16 @@
fallocate: Disk quota exceeded
pwrite: Disk quota exceeded
enable mature
+ERROR: unable to limit requested quota group: No such file or directory
+/home/fdmanana/git/hub/xfstests/tests/btrfs/301: line 373: [:
-lt: unary operator expected
+captured usage from before enable >= 134217728
+/home/fdmanana/git/hub/xfstests/tests/btrfs/301: line 377: [:
-lt: unary operator expected
...
(Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/301.out
/home/fdmanana/git/hub/xfstests/results//btrfs/301.out.bad' to see
the entire diff)
Ran: btrfs/301
Failures: btrfs/301
Failed 1 of 1 tests
I see variables like subvid below being declared as global in one
function and then used in other functions.
Maybe there's something wrong with that... it would be cleaner to make
the variable local and pass the subvolume id as an argument to other
functions.
Thanks.
>
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---
> tests/btrfs/301 | 138 ++++++++++++++++++++++++------------------------
> 1 file changed, 70 insertions(+), 68 deletions(-)
>
> diff --git a/tests/btrfs/301 b/tests/btrfs/301
> index 7a0b4c0e1..5bb6b16a6 100755
> --- a/tests/btrfs/301
> +++ b/tests/btrfs/301
> @@ -172,25 +172,27 @@ prepare()
> _scratch_mount
> enable_quota "s"
> $BTRFS_UTIL_PROG subvolume create $subv >> $seqres.full
> - set_subvol_limit 256 $limit
> - check_subvol_usage 256 0
> + subvid=$(_btrfs_get_subvolid $SCRATCH_MNT subv)
> + set_subvol_limit $subvid $limit
> + check_subvol_usage $subvid 0
>
> # Create a bunch of little filler files to generate several levels in
> # the btree, to make snapshotting sharing scenarios complex enough.
> $FIO_PROG $prep_fio_config --output=$fio_out
> - check_subvol_usage 256 $total_fill
> + check_subvol_usage $subvid $total_fill
>
> # Create a single file whose extents we will explicitly share/unshare.
> do_write $subv/f $ext_sz
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> }
>
> prepare_snapshotted()
> {
> prepare
> $BTRFS_UTIL_PROG subvolume snapshot $subv $snap >> $seqres.full
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> - check_subvol_usage 257 0
> + snapid=$(_btrfs_get_subvolid $SCRATCH_MNT snap)
Make the variable local please.
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> + check_subvol_usage $snapid 0
> }
>
> prepare_nested()
> @@ -198,13 +200,13 @@ prepare_nested()
> prepare
> $BTRFS_UTIL_PROG qgroup create 1/100 $SCRATCH_MNT
> $BTRFS_UTIL_PROG qgroup limit $limit 1/100 $SCRATCH_MNT
> - $BTRFS_UTIL_PROG qgroup assign 0/256 1/100 $SCRATCH_MNT >> $seqres.full
> + $BTRFS_UTIL_PROG qgroup assign 0/$subvid 1/100 $SCRATCH_MNT >> $seqres.full
> $BTRFS_UTIL_PROG subvolume create $nested >> $seqres.full
> do_write $nested/f $ext_sz
> - check_subvol_usage 257 $ext_sz
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> - local subv_usage=$(get_subvol_usage 256)
> - local nested_usage=$(get_subvol_usage 257)
> + check_subvol_usage $snapid $ext_sz
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> + local subv_usage=$(get_subvol_usage $subvid)
> + local nested_usage=$(get_subvol_usage $snapid)
> check_qgroup_usage 1/100 $(($subv_usage + $nested_usage))
> }
>
> @@ -214,8 +216,8 @@ basic_accounting()
> echo "basic accounting"
> prepare
> rm $subv/f
> - check_subvol_usage 256 $total_fill
> - cycle_mount_check_subvol_usage 256 $total_fill
> + check_subvol_usage $subvid $total_fill
> + cycle_mount_check_subvol_usage $subvid $total_fill
> do_write $subv/tmp 512M
> rm $subv/tmp
> do_write $subv/tmp 512M
> @@ -245,19 +247,19 @@ snapshot_accounting()
> echo "snapshot accounting"
> prepare_snapshotted
> touch $snap/f
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> - check_subvol_usage 257 0
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> + check_subvol_usage $snapid 0
> do_write $snap/f $ext_sz
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> - check_subvol_usage 257 $ext_sz
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> + check_subvol_usage $snapid $ext_sz
> rm $snap/f
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> - check_subvol_usage 257 0
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> + check_subvol_usage $snapid 0
> rm $subv/f
> - check_subvol_usage 256 $total_fill
> - check_subvol_usage 257 0
> - cycle_mount_check_subvol_usage 256 $total_fill
> - check_subvol_usage 257 0
> + check_subvol_usage $subvid $total_fill
> + check_subvol_usage $snapid 0
> + cycle_mount_check_subvol_usage $subvid $total_fill
> + check_subvol_usage $snapid 0
> _scratch_unmount
> }
>
> @@ -267,14 +269,14 @@ delete_snapshot_src_ref()
> echo "delete src ref first"
> prepare_snapshotted
> rm $subv/f
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> - check_subvol_usage 257 0
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> + check_subvol_usage $snapid 0
> rm $snap/f
> trigger_cleaner
> - check_subvol_usage 256 $total_fill
> - check_subvol_usage 257 0
> - cycle_mount_check_subvol_usage 256 $total_fill
> - check_subvol_usage 257 0
> + check_subvol_usage $subvid $total_fill
> + check_subvol_usage $snapid 0
> + cycle_mount_check_subvol_usage $subvid $total_fill
> + check_subvol_usage $snapid 0
> _scratch_unmount
> }
>
> @@ -284,13 +286,13 @@ delete_snapshot_ref()
> echo "delete snapshot ref first"
> prepare_snapshotted
> rm $snap/f
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> - check_subvol_usage 257 0
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> + check_subvol_usage $snapid 0
> rm $subv/f
> - check_subvol_usage 256 $total_fill
> - check_subvol_usage 257 0
> - cycle_mount_check_subvol_usage 256 $total_fill
> - check_subvol_usage 257 0
> + check_subvol_usage $subvid $total_fill
> + check_subvol_usage $snapid 0
> + cycle_mount_check_subvol_usage $subvid $total_fill
> + check_subvol_usage $snapid 0
> _scratch_unmount
> }
>
> @@ -300,18 +302,18 @@ delete_snapshot_src()
> echo "delete snapshot src first"
> prepare_snapshotted
> $BTRFS_UTIL_PROG subvolume delete $subv >> $seqres.full
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> - check_subvol_usage 257 0
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> + check_subvol_usage $snapid 0
> rm $snap/f
> trigger_cleaner
> - check_subvol_usage 256 $total_fill
> - check_subvol_usage 257 0
> + check_subvol_usage $subvid $total_fill
> + check_subvol_usage $snapid 0
> $BTRFS_UTIL_PROG subvolume delete $snap >> $seqres.full
> trigger_cleaner
> - check_subvol_usage 256 0
> - check_subvol_usage 257 0
> - cycle_mount_check_subvol_usage 256 0
> - check_subvol_usage 257 0
> + check_subvol_usage $subvid 0
> + check_subvol_usage $snapid 0
> + cycle_mount_check_subvol_usage $subvid 0
> + check_subvol_usage $snapid 0
> _scratch_unmount
> }
>
> @@ -321,12 +323,12 @@ delete_snapshot()
> echo "delete snapshot first"
> prepare_snapshotted
> $BTRFS_UTIL_PROG subvolume delete $snap >> $seqres.full
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> - check_subvol_usage 257 0
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> + check_subvol_usage $snapid 0
> $BTRFS_UTIL_PROG subvolume delete $subv >> $seqres.full
> trigger_cleaner
> - check_subvol_usage 256 0
> - check_subvol_usage 257 0
> + check_subvol_usage $subvid 0
> + check_subvol_usage $snapid 0
> _scratch_unmount
> }
>
> @@ -337,16 +339,16 @@ nested_accounting()
> echo "nested accounting"
> prepare_nested
> rm $subv/f
> - check_subvol_usage 256 $total_fill
> - check_subvol_usage 257 $ext_sz
> - local subv_usage=$(get_subvol_usage 256)
> - local nested_usage=$(get_subvol_usage 257)
> + check_subvol_usage $subvid $total_fill
> + check_subvol_usage $snapid $ext_sz
> + local subv_usage=$(get_subvol_usage $subvid)
> + local nested_usage=$(get_subvol_usage $snapid)
> check_qgroup_usage 1/100 $(($subv_usage + $nested_usage))
> rm $nested/f
> - check_subvol_usage 256 $total_fill
> - check_subvol_usage 257 0
> - subv_usage=$(get_subvol_usage 256)
> - nested_usage=$(get_subvol_usage 257)
> + check_subvol_usage $subvid $total_fill
> + check_subvol_usage $snapid 0
> + subv_usage=$(get_subvol_usage $subvid)
> + nested_usage=$(get_subvol_usage $snapid)
> check_qgroup_usage 1/100 $(($subv_usage + $nested_usage))
> do_enospc_falloc $nested/large_falloc 2G
> do_enospc_write $nested/large 2G
> @@ -365,21 +367,21 @@ enable_mature()
> # we did before enabling.
> sync
> enable_quota "s"
> - set_subvol_limit 256 $limit
> + set_subvol_limit $subvid $limit
> _scratch_cycle_mount
> - usage=$(get_subvol_usage 256)
> + usage=$(get_subvol_usage $subvid)
> [ $usage -lt $ext_sz ] || \
> echo "captured usage from before enable $usage >= $ext_sz"
> do_write $subv/g $ext_sz
> - usage=$(get_subvol_usage 256)
> + usage=$(get_subvol_usage $subvid)
> [ $usage -lt $ext_sz ] && \
> echo "failed to capture usage after enable $usage < $ext_sz"
> - check_subvol_usage 256 $ext_sz
> + check_subvol_usage $subvid $ext_sz
> rm $subv/f
> - check_subvol_usage 256 $ext_sz
> + check_subvol_usage $subvid $ext_sz
> _scratch_cycle_mount
> rm $subv/g
> - check_subvol_usage 256 0
> + check_subvol_usage $subvid 0
> _scratch_unmount
> }
>
> @@ -394,7 +396,7 @@ reflink_accounting()
> _cp_reflink $subv/f $subv/f.i
> done
> # Confirm that there is no additional data usage from the reflinks.
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> _scratch_unmount
> }
>
> @@ -404,11 +406,11 @@ delete_reflink_src_ref()
> echo "delete reflink src ref"
> prepare
> _cp_reflink $subv/f $subv/f.link
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> rm $subv/f
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> rm $subv/f.link
> - check_subvol_usage 256 $(($total_fill))
> + check_subvol_usage $subvid $(($total_fill))
> _scratch_unmount
> }
>
> @@ -418,11 +420,11 @@ delete_reflink_ref()
> echo "delete reflink ref"
> prepare
> _cp_reflink $subv/f $subv/f.link
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> rm $subv/f.link
> - check_subvol_usage 256 $(($total_fill + $ext_sz))
> + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> rm $subv/f
> - check_subvol_usage 256 $(($total_fill))
> + check_subvol_usage $subvid $(($total_fill))
> _scratch_unmount
> }
>
> --
> 2.42.0
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] btrfs/301: fix hardcoded subvolids
2023-11-20 19:45 ` Filipe Manana
@ 2023-11-20 19:53 ` Boris Burkov
0 siblings, 0 replies; 5+ messages in thread
From: Boris Burkov @ 2023-11-20 19:53 UTC (permalink / raw)
To: Filipe Manana; +Cc: linux-btrfs, kernel-team, fstests
On Mon, Nov 20, 2023 at 07:45:01PM +0000, Filipe Manana wrote:
> On Mon, Nov 20, 2023 at 6:44 PM Boris Burkov <boris@bur.io> wrote:
> >
> > Hardcoded subvolids break noholes test runs, so change the test to use
> > _btrfs_get_subvolid instead of assuming 256, 257, etc...
>
> How exactly does no-holes affect the assigned IDs to subvolumes?
> It's a default feature for quite a while and the test passes with it
> and without it (-O ^no-holes).
>
> Aren't you confusing this with enabling or disabling features that add
> some tree?
> Like disabling the free space tree (which is a default for quite a
> while now), for example.
Yes I am. Josef reported this to me and I misread the name of his
configuration that was failing. For completeness it was one that doesn't
use free space tree, like you say.
>
> For me it fails with -O ^free-space-tree, and it fails even with this
> patch applied:
And didn't reproduce it, since he proposed the fix, which seemed
"straightforward" enough to me.
>
> $ MKFS_OPTIONS="-O ^free-space-tree" ./check btrfs/301
> FSTYP -- btrfs
> PLATFORM -- Linux/x86_64 debian0 6.6.0-rc5-btrfs-next-140+ #1 SMP
> PREEMPT_DYNAMIC Thu Oct 19 16:55:42 WEST 2023
> MKFS_OPTIONS -- -O ^free-space-tree /dev/sdb
> MOUNT_OPTIONS -- /dev/sdb /home/fdmanana/btrfs-tests/scratch_1
>
> btrfs/301 70s ... - output mismatch (see
> /home/fdmanana/git/hub/xfstests/results//btrfs/301.out.bad)
> --- tests/btrfs/301.out 2023-10-18 23:29:06.029292800 +0100
> +++ /home/fdmanana/git/hub/xfstests/results//btrfs/301.out.bad
> 2023-11-20 19:33:04.988824928 +0000
> @@ -13,6 +13,16 @@
> fallocate: Disk quota exceeded
> pwrite: Disk quota exceeded
> enable mature
> +ERROR: unable to limit requested quota group: No such file or directory
> +/home/fdmanana/git/hub/xfstests/tests/btrfs/301: line 373: [:
> -lt: unary operator expected
> +captured usage from before enable >= 134217728
> +/home/fdmanana/git/hub/xfstests/tests/btrfs/301: line 377: [:
> -lt: unary operator expected
> ...
> (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/301.out
> /home/fdmanana/git/hub/xfstests/results//btrfs/301.out.bad' to see
> the entire diff)
> Ran: btrfs/301
> Failures: btrfs/301
> Failed 1 of 1 tests
>
> I see variables like subvid below being declared as global in one
> function and then used in other functions.
I tried to test that they get populated correctly (confused why it works
sometimes...)
But clearly it's broken in general and I'll do something more robust. I
think the basic problem is I call prepare() in each test function, so I
wanted prepare to set the global state, without making each test lookup
the subvolid. But it's not the end of the world to have the tests get it
one way or the other.
> Maybe there's something wrong with that... it would be cleaner to make
> the variable local and pass the subvolume id as an argument to other
> functions.
>
> Thanks.
Thanks for catching the mistake.
>
> >
> > Signed-off-by: Boris Burkov <boris@bur.io>
> > ---
> > tests/btrfs/301 | 138 ++++++++++++++++++++++++------------------------
> > 1 file changed, 70 insertions(+), 68 deletions(-)
> >
> > diff --git a/tests/btrfs/301 b/tests/btrfs/301
> > index 7a0b4c0e1..5bb6b16a6 100755
> > --- a/tests/btrfs/301
> > +++ b/tests/btrfs/301
> > @@ -172,25 +172,27 @@ prepare()
> > _scratch_mount
> > enable_quota "s"
> > $BTRFS_UTIL_PROG subvolume create $subv >> $seqres.full
> > - set_subvol_limit 256 $limit
> > - check_subvol_usage 256 0
> > + subvid=$(_btrfs_get_subvolid $SCRATCH_MNT subv)
> > + set_subvol_limit $subvid $limit
> > + check_subvol_usage $subvid 0
> >
> > # Create a bunch of little filler files to generate several levels in
> > # the btree, to make snapshotting sharing scenarios complex enough.
> > $FIO_PROG $prep_fio_config --output=$fio_out
> > - check_subvol_usage 256 $total_fill
> > + check_subvol_usage $subvid $total_fill
> >
> > # Create a single file whose extents we will explicitly share/unshare.
> > do_write $subv/f $ext_sz
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > }
> >
> > prepare_snapshotted()
> > {
> > prepare
> > $BTRFS_UTIL_PROG subvolume snapshot $subv $snap >> $seqres.full
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > - check_subvol_usage 257 0
> > + snapid=$(_btrfs_get_subvolid $SCRATCH_MNT snap)
>
> Make the variable local please.
>
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > + check_subvol_usage $snapid 0
> > }
> >
> > prepare_nested()
> > @@ -198,13 +200,13 @@ prepare_nested()
> > prepare
> > $BTRFS_UTIL_PROG qgroup create 1/100 $SCRATCH_MNT
> > $BTRFS_UTIL_PROG qgroup limit $limit 1/100 $SCRATCH_MNT
> > - $BTRFS_UTIL_PROG qgroup assign 0/256 1/100 $SCRATCH_MNT >> $seqres.full
> > + $BTRFS_UTIL_PROG qgroup assign 0/$subvid 1/100 $SCRATCH_MNT >> $seqres.full
> > $BTRFS_UTIL_PROG subvolume create $nested >> $seqres.full
> > do_write $nested/f $ext_sz
> > - check_subvol_usage 257 $ext_sz
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > - local subv_usage=$(get_subvol_usage 256)
> > - local nested_usage=$(get_subvol_usage 257)
> > + check_subvol_usage $snapid $ext_sz
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > + local subv_usage=$(get_subvol_usage $subvid)
> > + local nested_usage=$(get_subvol_usage $snapid)
> > check_qgroup_usage 1/100 $(($subv_usage + $nested_usage))
> > }
> >
> > @@ -214,8 +216,8 @@ basic_accounting()
> > echo "basic accounting"
> > prepare
> > rm $subv/f
> > - check_subvol_usage 256 $total_fill
> > - cycle_mount_check_subvol_usage 256 $total_fill
> > + check_subvol_usage $subvid $total_fill
> > + cycle_mount_check_subvol_usage $subvid $total_fill
> > do_write $subv/tmp 512M
> > rm $subv/tmp
> > do_write $subv/tmp 512M
> > @@ -245,19 +247,19 @@ snapshot_accounting()
> > echo "snapshot accounting"
> > prepare_snapshotted
> > touch $snap/f
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > - check_subvol_usage 257 0
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > + check_subvol_usage $snapid 0
> > do_write $snap/f $ext_sz
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > - check_subvol_usage 257 $ext_sz
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > + check_subvol_usage $snapid $ext_sz
> > rm $snap/f
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > - check_subvol_usage 257 0
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > + check_subvol_usage $snapid 0
> > rm $subv/f
> > - check_subvol_usage 256 $total_fill
> > - check_subvol_usage 257 0
> > - cycle_mount_check_subvol_usage 256 $total_fill
> > - check_subvol_usage 257 0
> > + check_subvol_usage $subvid $total_fill
> > + check_subvol_usage $snapid 0
> > + cycle_mount_check_subvol_usage $subvid $total_fill
> > + check_subvol_usage $snapid 0
> > _scratch_unmount
> > }
> >
> > @@ -267,14 +269,14 @@ delete_snapshot_src_ref()
> > echo "delete src ref first"
> > prepare_snapshotted
> > rm $subv/f
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > - check_subvol_usage 257 0
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > + check_subvol_usage $snapid 0
> > rm $snap/f
> > trigger_cleaner
> > - check_subvol_usage 256 $total_fill
> > - check_subvol_usage 257 0
> > - cycle_mount_check_subvol_usage 256 $total_fill
> > - check_subvol_usage 257 0
> > + check_subvol_usage $subvid $total_fill
> > + check_subvol_usage $snapid 0
> > + cycle_mount_check_subvol_usage $subvid $total_fill
> > + check_subvol_usage $snapid 0
> > _scratch_unmount
> > }
> >
> > @@ -284,13 +286,13 @@ delete_snapshot_ref()
> > echo "delete snapshot ref first"
> > prepare_snapshotted
> > rm $snap/f
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > - check_subvol_usage 257 0
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > + check_subvol_usage $snapid 0
> > rm $subv/f
> > - check_subvol_usage 256 $total_fill
> > - check_subvol_usage 257 0
> > - cycle_mount_check_subvol_usage 256 $total_fill
> > - check_subvol_usage 257 0
> > + check_subvol_usage $subvid $total_fill
> > + check_subvol_usage $snapid 0
> > + cycle_mount_check_subvol_usage $subvid $total_fill
> > + check_subvol_usage $snapid 0
> > _scratch_unmount
> > }
> >
> > @@ -300,18 +302,18 @@ delete_snapshot_src()
> > echo "delete snapshot src first"
> > prepare_snapshotted
> > $BTRFS_UTIL_PROG subvolume delete $subv >> $seqres.full
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > - check_subvol_usage 257 0
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > + check_subvol_usage $snapid 0
> > rm $snap/f
> > trigger_cleaner
> > - check_subvol_usage 256 $total_fill
> > - check_subvol_usage 257 0
> > + check_subvol_usage $subvid $total_fill
> > + check_subvol_usage $snapid 0
> > $BTRFS_UTIL_PROG subvolume delete $snap >> $seqres.full
> > trigger_cleaner
> > - check_subvol_usage 256 0
> > - check_subvol_usage 257 0
> > - cycle_mount_check_subvol_usage 256 0
> > - check_subvol_usage 257 0
> > + check_subvol_usage $subvid 0
> > + check_subvol_usage $snapid 0
> > + cycle_mount_check_subvol_usage $subvid 0
> > + check_subvol_usage $snapid 0
> > _scratch_unmount
> > }
> >
> > @@ -321,12 +323,12 @@ delete_snapshot()
> > echo "delete snapshot first"
> > prepare_snapshotted
> > $BTRFS_UTIL_PROG subvolume delete $snap >> $seqres.full
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > - check_subvol_usage 257 0
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > + check_subvol_usage $snapid 0
> > $BTRFS_UTIL_PROG subvolume delete $subv >> $seqres.full
> > trigger_cleaner
> > - check_subvol_usage 256 0
> > - check_subvol_usage 257 0
> > + check_subvol_usage $subvid 0
> > + check_subvol_usage $snapid 0
> > _scratch_unmount
> > }
> >
> > @@ -337,16 +339,16 @@ nested_accounting()
> > echo "nested accounting"
> > prepare_nested
> > rm $subv/f
> > - check_subvol_usage 256 $total_fill
> > - check_subvol_usage 257 $ext_sz
> > - local subv_usage=$(get_subvol_usage 256)
> > - local nested_usage=$(get_subvol_usage 257)
> > + check_subvol_usage $subvid $total_fill
> > + check_subvol_usage $snapid $ext_sz
> > + local subv_usage=$(get_subvol_usage $subvid)
> > + local nested_usage=$(get_subvol_usage $snapid)
> > check_qgroup_usage 1/100 $(($subv_usage + $nested_usage))
> > rm $nested/f
> > - check_subvol_usage 256 $total_fill
> > - check_subvol_usage 257 0
> > - subv_usage=$(get_subvol_usage 256)
> > - nested_usage=$(get_subvol_usage 257)
> > + check_subvol_usage $subvid $total_fill
> > + check_subvol_usage $snapid 0
> > + subv_usage=$(get_subvol_usage $subvid)
> > + nested_usage=$(get_subvol_usage $snapid)
> > check_qgroup_usage 1/100 $(($subv_usage + $nested_usage))
> > do_enospc_falloc $nested/large_falloc 2G
> > do_enospc_write $nested/large 2G
> > @@ -365,21 +367,21 @@ enable_mature()
> > # we did before enabling.
> > sync
> > enable_quota "s"
> > - set_subvol_limit 256 $limit
> > + set_subvol_limit $subvid $limit
> > _scratch_cycle_mount
> > - usage=$(get_subvol_usage 256)
> > + usage=$(get_subvol_usage $subvid)
> > [ $usage -lt $ext_sz ] || \
> > echo "captured usage from before enable $usage >= $ext_sz"
> > do_write $subv/g $ext_sz
> > - usage=$(get_subvol_usage 256)
> > + usage=$(get_subvol_usage $subvid)
> > [ $usage -lt $ext_sz ] && \
> > echo "failed to capture usage after enable $usage < $ext_sz"
> > - check_subvol_usage 256 $ext_sz
> > + check_subvol_usage $subvid $ext_sz
> > rm $subv/f
> > - check_subvol_usage 256 $ext_sz
> > + check_subvol_usage $subvid $ext_sz
> > _scratch_cycle_mount
> > rm $subv/g
> > - check_subvol_usage 256 0
> > + check_subvol_usage $subvid 0
> > _scratch_unmount
> > }
> >
> > @@ -394,7 +396,7 @@ reflink_accounting()
> > _cp_reflink $subv/f $subv/f.i
> > done
> > # Confirm that there is no additional data usage from the reflinks.
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > _scratch_unmount
> > }
> >
> > @@ -404,11 +406,11 @@ delete_reflink_src_ref()
> > echo "delete reflink src ref"
> > prepare
> > _cp_reflink $subv/f $subv/f.link
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > rm $subv/f
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > rm $subv/f.link
> > - check_subvol_usage 256 $(($total_fill))
> > + check_subvol_usage $subvid $(($total_fill))
> > _scratch_unmount
> > }
> >
> > @@ -418,11 +420,11 @@ delete_reflink_ref()
> > echo "delete reflink ref"
> > prepare
> > _cp_reflink $subv/f $subv/f.link
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > rm $subv/f.link
> > - check_subvol_usage 256 $(($total_fill + $ext_sz))
> > + check_subvol_usage $subvid $(($total_fill + $ext_sz))
> > rm $subv/f
> > - check_subvol_usage 256 $(($total_fill))
> > + check_subvol_usage $subvid $(($total_fill))
> > _scratch_unmount
> > }
> >
> > --
> > 2.42.0
> >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-11-20 19:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-20 18:45 [PATCH 0/2] btrfs/301: test fixes for squotas vs other features Boris Burkov
2023-11-20 18:45 ` [PATCH 1/2] btrfs/301: fix hardcoded subvolids Boris Burkov
2023-11-20 19:45 ` Filipe Manana
2023-11-20 19:53 ` Boris Burkov
2023-11-20 18:45 ` [PATCH 2/2] btrfs/301: require_no_compress Boris Burkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox