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

[Filipe Manana <fdmanana@gmail.com>]

On Mon, Aug 30, 2021 at 4:01 PM Filipe Manana <fdmanana@gmail.com> 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"),

Actually, it's 4871c1588f92c6c13f4713a7009f25f217055807 ("Btrfs: use right
root when checking for hash collision"), mentioned in the reply for v1. My bad.

> 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.
>
> 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
> >
>
>
> --
> Filipe David Manana,
>
> “Whether you think you can, or you think you can't — you're right.”



-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”