Hi Zorro,
Thank you for your review comments.
Zorro Lang <zlang@xxxxxxxxxx> 於 2022年8月23日 週二 晚上8:52寫道：
>
> On Mon, Aug 22, 2022 at 08:49:32AM +0800, bingjingc wrote:
> > From: BingJing Chang <bingjingc@xxxxxxxxxxxx>
> >
> > Normally btrfs stores file paths in an array of ref items. However, items
> > for the same parent directory can not exceed the size of a leaf. So btrfs
> > also store the rest of them in extended ref items alternatively.
> >
> > In this test, it creates a large number of links under a directory causing
> > the file paths stored in these two ways to be the parent snapshot. And it
> > deletes and recreates just an amount of them that can be stored within an
> > array of ref items to be the send snapshot. Test that an incremental send
> > operation correctly issues link/unlink operations only against new/deleted
> > paths, or the receive operation will fail due to a link on an existed path.
> >
> > This currently fails on btrfs but is fixed by a kernel patch with the
> > commit 3aa5bd367fa5a3 ("btrfs: send: fix sending link commands for
> > existing file paths")
> >
> > Signed-off-by: BingJing Chang <bingjingc@xxxxxxxxxxxx>
> > ---
> > tests/btrfs/272 | 97 +++++++++++++++++++++++++++++++++++++++++++++
> > tests/btrfs/272.out | 3 ++
> > 2 files changed, 100 insertions(+)
> > create mode 100755 tests/btrfs/272
> > create mode 100644 tests/btrfs/272.out
> >
> > diff --git a/tests/btrfs/272 b/tests/btrfs/272
> > new file mode 100755
> > index 00000000..e1986de9
> > --- /dev/null
> > +++ b/tests/btrfs/272
> > @@ -0,0 +1,97 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 BingJing Chang.
> > +#
> > +# FS QA Test No. btrfs/272
> > +#
> > +# Regression test for btrfs incremental send issue where a link instruction
> > +# is sent against an existing path, causing btrfs receive to fail.
> > +#
> > +# This issue is fixed by the following linux kernel btrfs patch:
> > +#
> > +# commit 3aa5bd367fa5a3 ("btrfs: send: fix sending link commands for
> > +# existing file paths")
>
> As Filipe said, welcome to use _fixed_by_kernel_commit and others related
> functions for a known regression test.
Thank you both for telling me this.
>
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick send
> > +
> > +# Override the default cleanup function.
> > +_cleanup()
> > +{
> > + cd /
> > + rm -fr $send_files_dir
>
> Will things in $send_files_dir take much disk space? If not, we can keep it
> in $TEST_DIR (then remove this specific _cleanup), generally we don't need
> to keep $TEST_DIR clean (except it will affect later testing).
>
No, in this test, it only generates many hard links to an empty file.
It takes only a little space to store the btrfs send stream.
I'm not familiar with the testing framework, so I added the _cleanup()
as other tests. It makes sense to me, so I will remove it in patch v3.
> > + rm -f $tmp.*
> > +}
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_fs btrfs
> > +_require_test
> > +_require_scratch
> > +_require_fssum
> > +
> > +send_files_dir=$TEST_DIR/btrfs-test-$seq
> > +
> > +rm -fr $send_files_dir
> > +mkdir $send_files_dir
> > +
> > +_scratch_mkfs > /dev/null 2>&1
>
> What kind of stdout/stderr you need to fill out? Generally we can output it
> into .full file for later debug. Anyway, I can't say this's wrong.
>
No. I should have outputted it to $seqres.ful.
Thanks for reminding me.
> > +_scratch_mount
> > +
> > +# Create a file and 2000 hard links to the same inode
> > +_run_btrfs_util_prog subvolume create $SCRATCH_MNT/vol
> > +touch $SCRATCH_MNT/vol/foo
> > +for i in {1..2000}; do
> > + link $SCRATCH_MNT/vol/foo $SCRATCH_MNT/vol/$i
> > +done
> > +
> > +# Create a snapshot for a full send operation
> > +_run_btrfs_util_prog subvolume snapshot -r $SCRATCH_MNT/vol $SCRATCH_MNT/snap1
> > +_run_btrfs_util_prog send -f $send_files_dir/1.snap $SCRATCH_MNT/snap1
> > +
> > +# Remove 2000 hard links and re-create the last 1000 links
> > +for i in {1..2000}; do
> > + rm $SCRATCH_MNT/vol/$i
> > +done
> > +for i in {1001..2000}; do
> > + link $SCRATCH_MNT/vol/foo $SCRATCH_MNT/vol/$i
> > +done
> > +
> > +# Create another snapshot for an incremental send operation
> > +_run_btrfs_util_prog subvolume snapshot -r $SCRATCH_MNT $SCRATCH_MNT/snap2
> > +_run_btrfs_util_prog send -p $SCRATCH_MNT/snap1 -f $send_files_dir/2.snap \
> > + $SCRATCH_MNT/snap2
> > +
> > +$FSSUM_PROG -A -f -w $send_files_dir/1.fssum $SCRATCH_MNT/snap1
> > +$FSSUM_PROG -A -f -w $send_files_dir/2.fssum \
> > + -x $SCRATCH_MNT/snap2/snap1 $SCRATCH_MNT/snap2
> > +
> > +# Recreate the filesystem by receiving both send streams and verify we get
> > +# the same content that the original filesystem had.
> > +_scratch_unmount
> > +_scratch_mkfs >>$seqres.full 2>&1
> > +_scratch_mount
> > +
> > +# Add the first snapshot to the new filesystem by applying the first send
> > +# stream.
> > +_run_btrfs_util_prog receive -f $send_files_dir/1.snap $SCRATCH_MNT
> > +
> > +# The incremental receive operation below used to fail with the following
> > +# error:
> > +#
> > +# ERROR: link 1238 -> foo failed: File exists
> > +#
> > +# This is because the path "1238" was stored as an extended ref item in the
> > +# original snapshot but as a normal ref item in the next snapshot. The send
> > +# operation cannot handle the duplicated paths, which are stored in
> > +# different ways, well, so it decides to issue a link operation for the
> > +# existing path. This results in the receiver to fail with the above error.
> > +_run_btrfs_util_prog receive -f $send_files_dir/2.snap $SCRATCH_MNT
> > +
> > +$FSSUM_PROG -r $send_files_dir/1.fssum $SCRATCH_MNT/snap1
> > +$FSSUM_PROG -r $send_files_dir/2.fssum $SCRATCH_MNT/snap2
> > +
> > +status=0
> > +exit
> > diff --git a/tests/btrfs/272.out b/tests/btrfs/272.out
> > new file mode 100644
> > index 00000000..b009b87a
> > --- /dev/null
> > +++ b/tests/btrfs/272.out
> > @@ -0,0 +1,3 @@
> > +QA output created by 272
> > +OK
> > +OK
> > --
> > 2.37.1
> >
>
