Re: [PATCH 2/2] btrfs-progs: misc-tests: add test case for receive --clone-fallback

[Qu Wenruo <wqu@suse.com>]

On 2021/9/30 18:30, Filipe Manana wrote:
> On Thu, Sep 30, 2021 at 11:18 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>>
>>
>>
>> On 2021/9/30 18:03, Filipe Manana wrote:
>>> On Thu, Sep 30, 2021 at 1:06 AM Qu Wenruo <wqu@suse.com> wrote:
>>>>
>>>> The new test case will create two send streams:
>>>>
>>>> - parent_stream
>>>>     A full send stream.
>>>>     Contains one file, as clone source.
>>>>
>>>> - clone_stream
>>>>     An incremental send stream.
>>>>     Contains one clone operation.
>>>>
>>>> Then we will receive the parent_stream with default mount options, while
>>>> try to receive the clone_stream with nodatasum mount option.
>>>>
>>>> This should result clone failure due to nodatasum flag mismatch.
>>>>
>>>> Then check if the receive can continue with --clone-fallback option.
>>>>
>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>> ---
>>>>    .../049-receive-clone-fallback/test.sh        | 60 +++++++++++++++++++
>>>>    1 file changed, 60 insertions(+)
>>>>    create mode 100755 tests/misc-tests/049-receive-clone-fallback/test.sh
>>>>
>>>> diff --git a/tests/misc-tests/049-receive-clone-fallback/test.sh b/tests/misc-tests/049-receive-clone-fallback/test.sh
>>>> new file mode 100755
>>>> index 000000000000..d383c0e08a68
>>>> --- /dev/null
>>>> +++ b/tests/misc-tests/049-receive-clone-fallback/test.sh
>>>> @@ -0,0 +1,60 @@
>>>> +#!/bin/bash
>>>> +# Verify that btrfs receive can fallback to buffered copy when clone
>>>> +# failed
>>>> +
>>>> +source "$TEST_TOP/common"
>>>> +
>>>> +check_prereq mkfs.btrfs
>>>> +check_prereq btrfs
>>>> +setup_root_helper
>>>> +prepare_test_dev
>>>> +
>>>> +tmp=$(mktemp -d --tmpdir btrfs-progs-send-stream.XXXXXX)
>>>> +
>>>> +# Create two send stream, one as the parent and the other as an incremental
>>>
>>> stream -> streams
>>>
>>>> +# stream with one clone operation.
>>>> +run_check_mkfs_test_dev
>>>> +run_check_mount_test_dev -o datacow,datasum
>>>> +run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "$TEST_MNT/subv"
>>>
>>> You can use the default subvolume and therefore avoid creating a
>>> subvolume and making the test longer than needed.
>>> Your call.
>>
>> I understand we can use the default fs tree, but I just can't find my
>> mind at peace when doing snapshoting of fs tree.
>>
>> It always remind me of the bad memories using hacky way to solve the
>> qgroup problem for such snapshot.
> 
> I don't get it.
> The fs tree is a subvolume like any other, it was always possible to
> create snapshots of it, and snapshotting it is done the same way as
> for any other subvolume (both in terms of api and at the
> implementation level).
> So I don't see anything "hacky" about it, and it feels very natural and common.

It's already way off-topic now, and it's mostly qgroup related problem.

The problem here is, when snapshotting fs tree, the destination dir is 
also in fs tree itself.

This means for qgroup, it has to do the snapshot dir entry creation 
differently, even they are happening inside the same transaction.
(like doing a mini-transaction commit inside a transaction)

Details can be found in qgroup_account_snapshot().

But that's all qgroup details, should not really trouble end users, just 
some really bad personal memories...

Thanks,
Qu

> 
> So I don't understand the relation with solving some qgroup related
> problems in a "hacky" way.
> 
> You can leave it though.
> 
>>
>> Thus I always try to avoid snapshotting fs tree.
>>
>>>
>>>> +run_check $SUDO_HELPER dd if=/dev/zero bs=1M count=1 of="$TEST_MNT/subv/file1"
>>>> +run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot -r "$TEST_MNT/subv" \
>>>> +       "$TEST_MNT/snap1"
>>>> +run_check $SUDO_HELPER cp --reflink=always "$TEST_MNT/subv/file1" \
>>>> +       "$TEST_MNT/subv/file2"
>>>> +run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot -r "$TEST_MNT/subv" \
>>>> +       "$TEST_MNT/snap2"
>>>> +
>>>> +run_check $SUDO_HELPER "$TOP/btrfs" send -f "$tmp/parent_stream" \
>>>> +       "$TEST_MNT/snap1"
>>>> +run_check $SUDO_HELPER "$TOP/btrfs" send -f "$tmp/clone_stream" \
>>>> +       -p "$TEST_MNT/snap1" "$TEST_MNT/snap2"
>>>> +
>>>> +run_check_umount_test_dev
>>>> +run_check_mkfs_test_dev
>>>> +
>>>> +# Now we have the needed stream, try to receive them with different mount
>>>
>>> Reading this is confusing, it mentions receiving them with different
>>> mount options, but they are the same for the first receive.
>>>
>>>> +# options
>>>> +run_check_mount_test_dev -o datacow -o datasum
>>>> +
>>>> +# Receiving the full stream should not fail
>>>> +run_check $SUDO_HELPER "$TOP/btrfs" receive -f "$tmp/parent_stream" "$TEST_MNT"
>>>> +
>>>> +# No remount helper, so here we manually unmoutn and re-mount with different
>>>> +# nodatasum option
>>>
>>> Seems pointless to mention there's a lack of a remount helper in the
>>> test framework.
>>> Just say that now we mount the filesystem with nodatasum so that the
>>> new file received through the incremental stream ends up with the
>>> nodatasum flag set.
>>
>> Or maybe I should just add run_check_remount_test().
>>
>> Thanks for the review,
>> Qu
>>
>>>
>>> Thanks.
>>>
>>>> +run_check_umount_test_dev
>>>> +run_check_mount_test_dev -o datacow,nodatasum
>>>> +
>>>> +# Receiving incremental send stream without --clone-fallback should fail.
>>>> +# As the clone source and destination have different NODATASUM flags
>>>> +run_mustfail "receiving clone with different NODATASUM should fail" \
>>>> +       $SUDO_HELPER "$TOP/btrfs" receive -f "$tmp/clone_stream" "$TEST_MNT"
>>>> +
>>>> +# Firstly we need to cleanup the partially received subvolume
>>>> +run_check $SUDO_HELPER "$TOP/btrfs" subvolume delete "$TEST_MNT/snap2"
>>>> +
>>>> +# With --clone-fallback specified, the receive should finish without problem
>>>> +run_check $SUDO_HELPER "$TOP/btrfs" receive --clone-fallback \
>>>> +       -f "$tmp/clone_stream" "$TEST_MNT"
>>>> +run_check_umount_test_dev
>>>> +
>>>> +rm -rf -- "$tmp"
>>>> --
>>>> 2.33.0
>>>>
>>>
>>>
> 
> 
>