[PATCH v3] fstests: btrfs: test incremental send for changed reference paths

[PATCH v3] fstests: btrfs: test incremental send for changed reference paths

[bingjingc]

From: BingJing Chang <bingjingc@synology.com>

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@synology.com>
---
 tests/btrfs/272     | 91 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/272.out |  3 ++
 2 files changed, 94 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..57dd065c
--- /dev/null
+++ b/tests/btrfs/272
@@ -0,0 +1,91 @@
+#! /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")
+#
+. ./common/preamble
+_begin_fstest auto quick send
+
+# Import common functions.
+. ./common/filter
+
+# real QA test starts here
+_supported_fs btrfs
+_fixed_by_kernel_commit 3aa5bd367fa5a3 \
+	"btrfs: send: fix sending link commands for existing file paths"
+_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 >>$seqres.full 2>&1
+_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/vol $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

Re: [PATCH v3] fstests: btrfs: test incremental send for changed reference paths

[Filipe Manana]

On Wed, Aug 24, 2022 at 06:42:02PM +0800, bingjingc wrote:
> From: BingJing Chang <bingjingc@synology.com>
> 
> 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@synology.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks a lot for doing this BingJing!

> ---
>  tests/btrfs/272     | 91 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/272.out |  3 ++
>  2 files changed, 94 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..57dd065c
> --- /dev/null
> +++ b/tests/btrfs/272
> @@ -0,0 +1,91 @@
> +#! /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")
> +#
> +. ./common/preamble
> +_begin_fstest auto quick send
> +
> +# Import common functions.
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_fixed_by_kernel_commit 3aa5bd367fa5a3 \
> +	"btrfs: send: fix sending link commands for existing file paths"
> +_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 >>$seqres.full 2>&1
> +_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/vol $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
> 

Re: [PATCH v3] fstests: btrfs: test incremental send for changed reference paths

[Zorro Lang]

On Wed, Aug 24, 2022 at 06:42:02PM +0800, bingjingc wrote:
> From: BingJing Chang <bingjingc@synology.com>
> 
> 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@synology.com>
> ---
>  tests/btrfs/272     | 91 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/272.out |  3 ++
>  2 files changed, 94 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..57dd065c
> --- /dev/null
> +++ b/tests/btrfs/272
> @@ -0,0 +1,91 @@
> +#! /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")
> +#
> +. ./common/preamble
> +_begin_fstest auto quick send
> +
> +# Import common functions.
> +. ./common/filter

I think this case doesn't use any helpers in common/filter, if so you can
remove this line. Others looks good to me.

Reviewed-by: Zorro Lang <zlang@redhat.com>

> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_fixed_by_kernel_commit 3aa5bd367fa5a3 \
> +	"btrfs: send: fix sending link commands for existing file paths"
> +_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 >>$seqres.full 2>&1
> +_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/vol $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
> 

Re: [PATCH v3] fstests: btrfs: test incremental send for changed reference paths

[bingjing chang]

Hi Zorro & Filipe,

Zorro Lang <zlang@redhat.com> 於 2022年8月24日 週三 晚上9:41寫道：
>
> On Wed, Aug 24, 2022 at 06:42:02PM +0800, bingjingc wrote:
> > From: BingJing Chang <bingjingc@synology.com>
> >
> > 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@synology.com>
> > ---
> >  tests/btrfs/272     | 91 +++++++++++++++++++++++++++++++++++++++++++++
> >  tests/btrfs/272.out |  3 ++
> >  2 files changed, 94 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..57dd065c
> > --- /dev/null
> > +++ b/tests/btrfs/272
> > @@ -0,0 +1,91 @@
> > +#! /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")
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick send
> > +
> > +# Import common functions.
> > +. ./common/filter
>
> I think this case doesn't use any helpers in common/filter, if so you can
> remove this line. Others looks good to me.
>

I removed the redundant import of common/filter and tested that the patch
can work. So I remove the line and provide the change log in patch v4.

> Reviewed-by: Zorro Lang <zlang@redhat.com>
>

Thank you both for your review.

> > +
> > +# real QA test starts here
> > +_supported_fs btrfs
> > +_fixed_by_kernel_commit 3aa5bd367fa5a3 \
> > +     "btrfs: send: fix sending link commands for existing file paths"
> > +_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 >>$seqres.full 2>&1
> > +_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/vol $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
> >
>