Re: [PATCH V2 2/2] btrfs: Add test for rename/exchange behavior between subvolumes

[Nikolay Borisov <nborisov@suse.com>]

On 30.08.21 г. 18:01, Filipe Manana wrote:
> On Mon, Aug 30, 2021 at 1:23 PM Nikolay Borisov <nborisov@suse.com> wrote:
>>
>> This tests ensures that renames/exchanges across subvolumes work only
>> for other subvolumes and are otherwise forbidden and fail.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>> Changes in V2:
>>  * Added cross-subvol rename tests
>>  * Added cross-subvol subvolume rename test
>>  * Added ordinary volume rename test
>>  * Removed explicit sync
>>
>>  tests/btrfs/246     | 43 +++++++++++++++++++++++++++++++++++++++++++
>>  tests/btrfs/246.out | 27 +++++++++++++++++++++++++++
>>  2 files changed, 70 insertions(+)
>>  create mode 100755 tests/btrfs/246
>>  create mode 100644 tests/btrfs/246.out
>>
>> diff --git a/tests/btrfs/246 b/tests/btrfs/246
>> new file mode 100755
>> index 000000000000..eeb12bb457a8
>> --- /dev/null
>> +++ b/tests/btrfs/246
>> @@ -0,0 +1,43 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2021 SUSE Linux Products GmbH.  All Rights Reserved.
>> +#
>> +# FS QA Test 246
>> +#
>> +# Tests rename exchange behavior when subvolumes are involved. This is also a
>> +# regression test for 3f79f6f6247c ("btrfs: prevent rename2 from exchanging a
>> +# subvol with a directory from different parents").
> 
> And it's also a regression test for commit 3f79f6f6247c83 ("btrfs:
> prevent rename2 from exchanging a subvol with a directory from
> different parents"),
> which is actually the fix that motivated v1 of this test case.
> 
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto quick rename subvol
>> +
>> +# Import common functions.
>> + . ./common/renameat2
>> +
>> +# real QA test starts here
>> +
>> +# Modify as appropriate.
>> +_supported_fs btrfs
>> +_require_renameat2 exchange
>> +_require_scratch
>> +
>> +_scratch_mkfs >> $seqres.full 2>&1
>> +_scratch_mount
>> +
>> +# Create 2 subvols to use as parents for the rename ops
>> +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subvol1 1>/dev/null
>> +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subvol2 1>/dev/null
>> +
>> +# Ensure cross subvol ops are forbidden
>> +_rename_tests_source_dest $SCRATCH_MNT/subvol1/src $SCRATCH_MNT/subvol2/dst "cross-subvol" "-x"
>> +
>> +# Prepare a subvolume and a directory whose parents are different subvolumes
>> +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subvol1/sub-subvol 1>/dev/null
>> +mkdir $SCRATCH_MNT/subvol2/dir
>> +
>> +# Ensure exchanging a subvol with a dir when both parents are different fails
>> +$here/src/renameat2 -x $SCRATCH_MNT/subvol1/sub-subvol $SCRATCH_MNT/subvol2/dir
> 
> Where is the test for renaming a subvolume and the test(s) for regular renames?
> The v2 changelog mentions those tests were added, but all the tests I
> see here are doing rename exchanges only.
> 
> I must have missed something subtle.

Nope, I missed committing my changes...

> 
> Thanks.
> 
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/btrfs/246.out b/tests/btrfs/246.out
>> new file mode 100644
>> index 000000000000..d50dc28b1b40
>> --- /dev/null
>> +++ b/tests/btrfs/246.out
>> @@ -0,0 +1,27 @@
>> +QA output created by 246
>> +cross-subvol none/none -> No such file or directory
>> +cross-subvol none/regu -> No such file or directory
>> +cross-subvol none/symb -> No such file or directory
>> +cross-subvol none/dire -> No such file or directory
>> +cross-subvol none/tree -> No such file or directory
>> +cross-subvol regu/none -> No such file or directory
>> +cross-subvol regu/regu -> Invalid cross-device link
>> +cross-subvol regu/symb -> Invalid cross-device link
>> +cross-subvol regu/dire -> Invalid cross-device link
>> +cross-subvol regu/tree -> Invalid cross-device link
>> +cross-subvol symb/none -> No such file or directory
>> +cross-subvol symb/regu -> Invalid cross-device link
>> +cross-subvol symb/symb -> Invalid cross-device link
>> +cross-subvol symb/dire -> Invalid cross-device link
>> +cross-subvol symb/tree -> Invalid cross-device link
>> +cross-subvol dire/none -> No such file or directory
>> +cross-subvol dire/regu -> Invalid cross-device link
>> +cross-subvol dire/symb -> Invalid cross-device link
>> +cross-subvol dire/dire -> Invalid cross-device link
>> +cross-subvol dire/tree -> Invalid cross-device link
>> +cross-subvol tree/none -> No such file or directory
>> +cross-subvol tree/regu -> Invalid cross-device link
>> +cross-subvol tree/symb -> Invalid cross-device link
>> +cross-subvol tree/dire -> Invalid cross-device link
>> +cross-subvol tree/tree -> Invalid cross-device link
>> +Invalid cross-device link
>> --
>> 2.17.1
>>
> 
>