[PATCH] btrfs: test that we can not delete a subvolume with an active swap file

[PATCH] btrfs: test that we can not delete a subvolume with an active swap file

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Verify that we can not delete a subvolume that has an active swap file,
and that after disabling the swap file, we can delete it.

This tests a fix done by kernel commit 60021bd754c6ca ("btrfs: prevent
subvol with swapfile from being deleted"), which landed in kernel 5.18.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 tests/btrfs/274     | 51 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/274.out |  6 ++++++
 2 files changed, 57 insertions(+)
 create mode 100755 tests/btrfs/274
 create mode 100644 tests/btrfs/274.out

diff --git a/tests/btrfs/274 b/tests/btrfs/274
new file mode 100755
index 00000000..0309c118
--- /dev/null
+++ b/tests/btrfs/274
@@ -0,0 +1,51 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
+#
+# FS QA Test 274
+#
+# Test that we can not delete a subvolume that has an active swap file.
+#
+. ./common/preamble
+_begin_fstest auto quick swap subvol
+
+. ./common/filter
+
+_supported_fs btrfs
+_fixed_by_kernel_commit 60021bd754c6ca \
+    "btrfs: prevent subvol with swapfile from being deleted"
+_require_scratch_swapfile
+
+_scratch_mkfs >> $seqres.full 2>&1
+_scratch_mount
+
+$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subvol | _filter_scratch
+swap_file="$SCRATCH_MNT/subvol/swap"
+
+echo "Creating and activating swap file..."
+_format_swapfile $swap_file $(($(get_page_size) * 32)) >> $seqres.full
+_swapon_file $swap_file
+
+echo "Attempting to delete subvolume with swap file enabled..."
+# Output differs with different btrfs-progs versions and some display multiple
+# lines on failure like this for example:
+#
+#   ERROR: Could not destroy subvolume/snapshot: Operation not permitted
+#   WARNING: deletion failed with EPERM, send may be in progress
+#   Delete subvolume (no-commit): '/home/fdmanana/btrfs-tests/scratch_1/subvol'
+#
+# So just redirect all output to the .full file and check the command's exit
+# status instead.
+$BTRFS_UTIL_PROG subvolume delete $SCRATCH_MNT/subvol >> $seqres.full 2>&1 && \
+    echo "subvolume deletion successful, expected failure!"
+
+echo "Disabling swap file..."
+swapoff $swap_file
+
+echo "Attempting to delete subvolume after disabling swap file..."
+$BTRFS_UTIL_PROG subvolume delete $SCRATCH_MNT/subvol >> $seqres.full 2>&1 || \
+   echo "subvolume deletion failure, expected success!"
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/274.out b/tests/btrfs/274.out
new file mode 100644
index 00000000..66e0de25
--- /dev/null
+++ b/tests/btrfs/274.out
@@ -0,0 +1,6 @@
+QA output created by 274
+Create subvolume 'SCRATCH_MNT/subvol'
+Creating and activating swap file...
+Attempting to delete subvolume with swap file enabled...
+Disabling swap file...
+Attempting to delete subvolume after disabling swap file...
-- 
2.35.1

Re: [PATCH] btrfs: test that we can not delete a subvolume with an active swap file

[Zorro Lang]

On Thu, Sep 01, 2022 at 05:12:11PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Verify that we can not delete a subvolume that has an active swap file,
> and that after disabling the swap file, we can delete it.
> 
> This tests a fix done by kernel commit 60021bd754c6ca ("btrfs: prevent
> subvol with swapfile from being deleted"), which landed in kernel 5.18.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  tests/btrfs/274     | 51 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/274.out |  6 ++++++
>  2 files changed, 57 insertions(+)
>  create mode 100755 tests/btrfs/274
>  create mode 100644 tests/btrfs/274.out
> 
> diff --git a/tests/btrfs/274 b/tests/btrfs/274
> new file mode 100755
> index 00000000..0309c118
> --- /dev/null
> +++ b/tests/btrfs/274
> @@ -0,0 +1,51 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
> +#
> +# FS QA Test 274
> +#
> +# Test that we can not delete a subvolume that has an active swap file.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick swap subvol
> +
> +. ./common/filter
> +
> +_supported_fs btrfs
> +_fixed_by_kernel_commit 60021bd754c6ca \
> +    "btrfs: prevent subvol with swapfile from being deleted"
> +_require_scratch_swapfile
> +
> +_scratch_mkfs >> $seqres.full 2>&1
> +_scratch_mount
> +
> +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subvol | _filter_scratch
> +swap_file="$SCRATCH_MNT/subvol/swap"
> +
> +echo "Creating and activating swap file..."
> +_format_swapfile $swap_file $(($(get_page_size) * 32)) >> $seqres.full
> +_swapon_file $swap_file
> +
> +echo "Attempting to delete subvolume with swap file enabled..."
> +# Output differs with different btrfs-progs versions and some display multiple
> +# lines on failure like this for example:
> +#
> +#   ERROR: Could not destroy subvolume/snapshot: Operation not permitted
> +#   WARNING: deletion failed with EPERM, send may be in progress
> +#   Delete subvolume (no-commit): '/home/fdmanana/btrfs-tests/scratch_1/subvol'
> +#
> +# So just redirect all output to the .full file and check the command's exit
> +# status instead.
> +$BTRFS_UTIL_PROG subvolume delete $SCRATCH_MNT/subvol >> $seqres.full 2>&1 && \
> +    echo "subvolume deletion successful, expected failure!"
> +
> +echo "Disabling swap file..."
> +swapoff $swap_file

Better to make sure we swapoff this file in _cleanup() too, to avoid it affect
later testing if something suddently kill the testing. Others looks good to me,
welcome more review from btrfs.

Thanks,
Zorro

> +
> +echo "Attempting to delete subvolume after disabling swap file..."
> +$BTRFS_UTIL_PROG subvolume delete $SCRATCH_MNT/subvol >> $seqres.full 2>&1 || \
> +   echo "subvolume deletion failure, expected success!"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/274.out b/tests/btrfs/274.out
> new file mode 100644
> index 00000000..66e0de25
> --- /dev/null
> +++ b/tests/btrfs/274.out
> @@ -0,0 +1,6 @@
> +QA output created by 274
> +Create subvolume 'SCRATCH_MNT/subvol'
> +Creating and activating swap file...
> +Attempting to delete subvolume with swap file enabled...
> +Disabling swap file...
> +Attempting to delete subvolume after disabling swap file...
> -- 
> 2.35.1
> 

Re: [PATCH] btrfs: test that we can not delete a subvolume with an active swap file

[David Sterba]

On Fri, Sep 02, 2022 at 10:11:37AM +0800, Zorro Lang wrote:
> Better to make sure we swapoff this file in _cleanup() too, to avoid it affect
> later testing if something suddently kill the testing. Others looks good to me,
> welcome more review from btrfs.

You can assume an implicit ack for any btrfs tests when it's sent by a
person with history of existing fstests and/or btrfs commits. A review
would be desirable but I'd say it would hardly find any problems,
typically the one who writes the kernel patch has a test script that is
then transformed to the test case so you can also expect a real testing.

Speaking for the btrfs developer group, we'd rather see slightly
imperfect or not formally reviewed patch in fstests suite as we'd catch
any breakage once it's pushed to git. We have daily update and test
loops and we'll notice, also we know how to revert commits or disable
tests if needed. Holding back sent patches is counterproductive. Thanks.