[PATCH 0/3] btrfs: add couple tests to check fiemap correctly reports extent sharedness

[PATCH 0/3] btrfs: add couple tests to check fiemap correctly reports extent sharedness

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

This adds two new btrfs specific tests to verify fiemap correctly reports
extent sharedness in a couple of scenarios recently fixed in kernel 6.1-rc2.
More details on the test files.

Filipe Manana (3):
  common/punch: fix flags printing for filter _filter_fiemap_flags
  btrfs: test that fiemap reports extent as not shared after deleting file
  btrfs: test fiemap reports extent as not shared after COWing it in snapshot

 common/punch        |  2 ++
 tests/btrfs/279     | 82 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/279.out | 39 +++++++++++++++++++++
 tests/btrfs/280     | 64 +++++++++++++++++++++++++++++++++++
 tests/btrfs/280.out | 21 ++++++++++++
 5 files changed, 208 insertions(+)
 create mode 100755 tests/btrfs/279
 create mode 100644 tests/btrfs/279.out
 create mode 100755 tests/btrfs/280
 create mode 100644 tests/btrfs/280.out

-- 
2.35.1

[PATCH 1/3] common/punch: fix flags printing for filter _filter_fiemap_flags

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

In the filter _filter_fiemap_flags, if we get a flags field that only has
the 'last' flag set, we end up printing the string "nonelast", which is
ugly and not intuitive.

For example:

  $XFS_IO_PROG -f -c "pwrite 0 64K" $SCRATCH_MNT/foo > /dev/null
  $XFS_IO_PROG -c "fiemap -v" $SCRATCH_MNT/foo | _filter_fiemap_flags

Gives:

  0: [0..127]: nonelast

So fix this by updating the filter's awk code to reset the flags string to
an empty string if we have the "last" flag set and we haven't updated the
flags string before. So now the same test gives the following result:

  0: [0..127]: last

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 common/punch | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/punch b/common/punch
index 47a29c92..94599c35 100644
--- a/common/punch
+++ b/common/punch
@@ -130,6 +130,8 @@ _filter_fiemap_flags()
 			if (and(flags, 0x1)) {
 				if (set) {
 					flag_str = flag_str"|";
+				} else {
+					flag_str = "";
 				}
 				flag_str = flag_str"last";
 			}
-- 
2.35.1

[PATCH 2/3] btrfs: test that fiemap reports extent as not shared after deleting file

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Test that if we have two files with shared extents, after removing one of
the files, if we do a fiemap against the other file, it does not report
extents as shared anymore.

This exercises the processing of delayed references for data extents in
the backref walking code, used by fiemap to determine if an extent is
shared.

This used to fail until very recently and was fixed by the following
kernel commit that landed in 6.1-rc2:

  4fc7b5722824 (""btrfs: fix processing of delayed data refs during backref walking")

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 tests/btrfs/279     | 82 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/279.out | 39 +++++++++++++++++++++
 2 files changed, 121 insertions(+)
 create mode 100755 tests/btrfs/279
 create mode 100644 tests/btrfs/279.out

diff --git a/tests/btrfs/279 b/tests/btrfs/279
new file mode 100755
index 00000000..5b5824fd
--- /dev/null
+++ b/tests/btrfs/279
@@ -0,0 +1,82 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
+#
+# FS QA Test 279
+#
+# Test that if we have two files with shared extents, after removing one of the
+# files, if we do a fiemap against the other file, it does not report extents as
+# shared anymore.
+#
+# This exercises the processing of delayed references for data extents in the
+# backref walking code, used by fiemap to determine if an extent is shared.
+#
+. ./common/preamble
+_begin_fstest auto quick subvol fiemap clone
+
+. ./common/filter
+. ./common/reflink
+. ./common/punch # for _filter_fiemap_flags
+
+_supported_fs btrfs
+_require_scratch_reflink
+_require_cp_reflink
+_require_xfs_io_command "fiemap"
+
+_fixed_by_kernel_commit 4fc7b5722824 \
+	"btrfs: fix processing of delayed data refs during backref walking"
+
+run_test()
+{
+	local file_path_1=$1
+	local file_path_2=$2
+	local do_sync=$3
+
+	$XFS_IO_PROG -f -c "pwrite 0 64K" $file_path_1 | _filter_xfs_io
+	_cp_reflink $file_path_1 $file_path_2
+
+	if [ $do_sync -eq 1 ]; then
+		sync
+	fi
+
+	echo "Fiemap of $file_path_1 before deleting $file_path_2:" | \
+		_filter_scratch
+	$XFS_IO_PROG -c "fiemap -v" $file_path_1 | _filter_fiemap_flags
+
+	rm -f $file_path_2
+
+	echo "Fiemap of $file_path_1 after deleting $file_path_2:" | \
+		_filter_scratch
+	$XFS_IO_PROG -c "fiemap -v" $file_path_1 | _filter_fiemap_flags
+}
+
+_scratch_mkfs >> $seqres.full 2>&1
+_scratch_mount
+
+# Create two test subvolumes, we'll reflink files between them.
+$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subv1 | _filter_scratch
+$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subv2 | _filter_scratch
+
+echo
+echo "Testing with same subvolume and without transaction commit"
+echo
+run_test "$SCRATCH_MNT/subv1/f1" "$SCRATCH_MNT/subv1/f2" 0
+
+echo
+echo "Testing with same subvolume and with transaction commit"
+echo
+run_test "$SCRATCH_MNT/subv1/f3" "$SCRATCH_MNT/subv1/f4" 1
+
+echo
+echo "Testing with different subvolumes and without transaction commit"
+echo
+run_test "$SCRATCH_MNT/subv1/f5" "$SCRATCH_MNT/subv2/f6" 0
+
+echo
+echo "Testing with different subvolumes and with transaction commit"
+echo
+run_test "$SCRATCH_MNT/subv1/f7" "$SCRATCH_MNT/subv2/f8" 1
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/279.out b/tests/btrfs/279.out
new file mode 100644
index 00000000..a959a86d
--- /dev/null
+++ b/tests/btrfs/279.out
@@ -0,0 +1,39 @@
+QA output created by 279
+Create subvolume 'SCRATCH_MNT/subv1'
+Create subvolume 'SCRATCH_MNT/subv2'
+
+Testing with same subvolume and without transaction commit
+
+wrote 65536/65536 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Fiemap of SCRATCH_MNT/subv1/f1 before deleting SCRATCH_MNT/subv1/f2:
+0: [0..127]: shared|last
+Fiemap of SCRATCH_MNT/subv1/f1 after deleting SCRATCH_MNT/subv1/f2:
+0: [0..127]: last
+
+Testing with same subvolume and with transaction commit
+
+wrote 65536/65536 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Fiemap of SCRATCH_MNT/subv1/f3 before deleting SCRATCH_MNT/subv1/f4:
+0: [0..127]: shared|last
+Fiemap of SCRATCH_MNT/subv1/f3 after deleting SCRATCH_MNT/subv1/f4:
+0: [0..127]: last
+
+Testing with different subvolumes and without transaction commit
+
+wrote 65536/65536 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Fiemap of SCRATCH_MNT/subv1/f5 before deleting SCRATCH_MNT/subv2/f6:
+0: [0..127]: shared|last
+Fiemap of SCRATCH_MNT/subv1/f5 after deleting SCRATCH_MNT/subv2/f6:
+0: [0..127]: last
+
+Testing with different subvolumes and with transaction commit
+
+wrote 65536/65536 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Fiemap of SCRATCH_MNT/subv1/f7 before deleting SCRATCH_MNT/subv2/f8:
+0: [0..127]: shared|last
+Fiemap of SCRATCH_MNT/subv1/f7 after deleting SCRATCH_MNT/subv2/f8:
+0: [0..127]: last
-- 
2.35.1

[PATCH 3/3] btrfs: test fiemap reports extent as not shared after COWing it in snapshot

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Test that if we have a large file, with file extent items spanning several
leaves in the fs tree, and that is shared due to a snapshot, if we COW one
of the extents, doing a fiemap will report the respective file range as
not shared.

This exercises the processing of delayed references for metadata extents
in the backref walking code, used by fiemap to determine if an extent is
shared.

This used to fail until very recently and was fixed by the following
kernel commit that landed in 6.1-rc2:

  943553ef9b51 (""btrfs: fix processing of delayed tree block refs during backref walking")

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 tests/btrfs/280     | 64 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/280.out | 21 +++++++++++++++
 2 files changed, 85 insertions(+)
 create mode 100755 tests/btrfs/280
 create mode 100644 tests/btrfs/280.out

diff --git a/tests/btrfs/280 b/tests/btrfs/280
new file mode 100755
index 00000000..06ef221e
--- /dev/null
+++ b/tests/btrfs/280
@@ -0,0 +1,64 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
+#
+# FS QA Test 280
+#
+# Test that if we have a large file, with file extent items spanning several
+# leaves in the fs tree, and that is shared due to a snapshot, if we COW one of
+# the extents, doing a fiemap will report the respective file range as not
+# shared.
+#
+# This exercises the processing of delayed references for metadata extents in
+# the backref walking code, used by fiemap to determine if an extent is shared.
+#
+. ./common/preamble
+_begin_fstest auto quick compress snapshot fiemap
+
+. ./common/filter
+. ./common/punch # for _filter_fiemap_flags
+
+_supported_fs btrfs
+_require_scratch
+_require_xfs_io_command "fiemap"
+
+_fixed_by_kernel_commit 943553ef9b51 \
+	"btrfs: fix processing of delayed tree block refs during backref walking"
+
+_scratch_mkfs >> $seqres.full 2>&1
+# We use compression because it's a very quick way to create a file with a very
+# large number of extents (compression limits the maximum extent size to 128K)
+# and while using very little disk space.
+_scratch_mount -o compress
+
+# A 128M file full of compressed extents results in a fs tree with a heigth
+# of 2 (root at level 1), so we'll end up with tree block references in the
+# extent tree (if the root was a leaf, we would have only data references).
+$XFS_IO_PROG -f -c "pwrite -b 1M 0 128M" $SCRATCH_MNT/foo | _filter_xfs_io
+
+# Create a RW snapshot of the default subvolume.
+$BTRFS_UTIL_PROG subvolume snapshot $SCRATCH_MNT $SCRATCH_MNT/snap | _filter_scratch
+
+echo
+echo "File foo fiemap before COWing extent:"
+echo
+$XFS_IO_PROG -c "fiemap -v" $SCRATCH_MNT/foo | _filter_fiemap_flags
+
+echo
+echo "Overwriting file range [120M, 120M + 128K) in the snapshot"
+echo
+$XFS_IO_PROG -c "pwrite -b 128K 120M 128K" $SCRATCH_MNT/snap/foo | _filter_xfs_io
+# Now fsync the file to force COWing the extent and the path from the root of
+# the snapshot tree down to the leaf where the extent is at.
+$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/snap/foo
+
+echo
+echo "File foo fiemap after COWing extent in the snapshot:"
+echo
+# Now we should have all extents marked as shared except the 128K extent in the
+# file range [120M, 120M + 128K).
+$XFS_IO_PROG -c "fiemap -v" $SCRATCH_MNT/foo | _filter_fiemap_flags
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/280.out b/tests/btrfs/280.out
new file mode 100644
index 00000000..c3f82966
--- /dev/null
+++ b/tests/btrfs/280.out
@@ -0,0 +1,21 @@
+QA output created by 280
+wrote 134217728/134217728 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Create a snapshot of 'SCRATCH_MNT' in 'SCRATCH_MNT/snap'
+
+File foo fiemap before COWing extent:
+
+0: [0..261887]: shared
+1: [261888..262143]: shared|last
+
+Overwriting file range [120M, 120M + 128K) in the snapshot
+
+wrote 131072/131072 bytes at offset 125829120
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+File foo fiemap after COWing extent in the snapshot:
+
+0: [0..245759]: shared
+1: [245760..246015]: none
+2: [246016..261887]: shared
+3: [261888..262143]: shared|last
-- 
2.35.1

Re: [PATCH 1/3] common/punch: fix flags printing for filter _filter_fiemap_flags

[Zorro Lang]

On Mon, Oct 31, 2022 at 11:11:19AM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> In the filter _filter_fiemap_flags, if we get a flags field that only has
> the 'last' flag set, we end up printing the string "nonelast", which is
> ugly and not intuitive.
> 
> For example:
> 
>   $XFS_IO_PROG -f -c "pwrite 0 64K" $SCRATCH_MNT/foo > /dev/null
>   $XFS_IO_PROG -c "fiemap -v" $SCRATCH_MNT/foo | _filter_fiemap_flags
> 
> Gives:
> 
>   0: [0..127]: nonelast
> 
> So fix this by updating the filter's awk code to reset the flags string to
> an empty string if we have the "last" flag set and we haven't updated the
> flags string before. So now the same test gives the following result:
> 
>   0: [0..127]: last
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  common/punch | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/common/punch b/common/punch
> index 47a29c92..94599c35 100644
> --- a/common/punch
> +++ b/common/punch
> @@ -130,6 +130,8 @@ _filter_fiemap_flags()
>  			if (and(flags, 0x1)) {
>  				if (set) {
>  					flag_str = flag_str"|";
> +				} else {
> +					flag_str = "";

Looks good to me.

We have two cases (g/352, g/353) call this function, I just gave them a test,
looks like this change doesn't affect them.

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

>  				}
>  				flag_str = flag_str"last";
>  			}
> -- 
> 2.35.1
> 

Re: [PATCH 2/3] btrfs: test that fiemap reports extent as not shared after deleting file

[Zorro Lang]

On Mon, Oct 31, 2022 at 11:11:20AM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Test that if we have two files with shared extents, after removing one of
> the files, if we do a fiemap against the other file, it does not report
> extents as shared anymore.
> 
> This exercises the processing of delayed references for data extents in
> the backref walking code, used by fiemap to determine if an extent is
> shared.
> 
> This used to fail until very recently and was fixed by the following
> kernel commit that landed in 6.1-rc2:
> 
>   4fc7b5722824 (""btrfs: fix processing of delayed data refs during backref walking")
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---

Looks good to me. As this's not a genericcase, I'm not sure if you need
_require_congruent_file_oplen helper.

Thanks,
Zorro

>  tests/btrfs/279     | 82 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/279.out | 39 +++++++++++++++++++++
>  2 files changed, 121 insertions(+)
>  create mode 100755 tests/btrfs/279
>  create mode 100644 tests/btrfs/279.out
> 
> diff --git a/tests/btrfs/279 b/tests/btrfs/279
> new file mode 100755
> index 00000000..5b5824fd
> --- /dev/null
> +++ b/tests/btrfs/279
> @@ -0,0 +1,82 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
> +#
> +# FS QA Test 279
> +#
> +# Test that if we have two files with shared extents, after removing one of the
> +# files, if we do a fiemap against the other file, it does not report extents as
> +# shared anymore.
> +#
> +# This exercises the processing of delayed references for data extents in the
> +# backref walking code, used by fiemap to determine if an extent is shared.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick subvol fiemap clone
> +
> +. ./common/filter
> +. ./common/reflink
> +. ./common/punch # for _filter_fiemap_flags
> +
> +_supported_fs btrfs
> +_require_scratch_reflink
> +_require_cp_reflink
> +_require_xfs_io_command "fiemap"
> +
> +_fixed_by_kernel_commit 4fc7b5722824 \
> +	"btrfs: fix processing of delayed data refs during backref walking"
> +
> +run_test()
> +{
> +	local file_path_1=$1
> +	local file_path_2=$2
> +	local do_sync=$3
> +
> +	$XFS_IO_PROG -f -c "pwrite 0 64K" $file_path_1 | _filter_xfs_io
> +	_cp_reflink $file_path_1 $file_path_2
> +
> +	if [ $do_sync -eq 1 ]; then
> +		sync
> +	fi
> +
> +	echo "Fiemap of $file_path_1 before deleting $file_path_2:" | \
> +		_filter_scratch
> +	$XFS_IO_PROG -c "fiemap -v" $file_path_1 | _filter_fiemap_flags
> +
> +	rm -f $file_path_2
> +
> +	echo "Fiemap of $file_path_1 after deleting $file_path_2:" | \
> +		_filter_scratch
> +	$XFS_IO_PROG -c "fiemap -v" $file_path_1 | _filter_fiemap_flags
> +}
> +
> +_scratch_mkfs >> $seqres.full 2>&1
> +_scratch_mount
> +
> +# Create two test subvolumes, we'll reflink files between them.
> +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subv1 | _filter_scratch
> +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subv2 | _filter_scratch
> +
> +echo
> +echo "Testing with same subvolume and without transaction commit"
> +echo
> +run_test "$SCRATCH_MNT/subv1/f1" "$SCRATCH_MNT/subv1/f2" 0
> +
> +echo
> +echo "Testing with same subvolume and with transaction commit"
> +echo
> +run_test "$SCRATCH_MNT/subv1/f3" "$SCRATCH_MNT/subv1/f4" 1
> +
> +echo
> +echo "Testing with different subvolumes and without transaction commit"
> +echo
> +run_test "$SCRATCH_MNT/subv1/f5" "$SCRATCH_MNT/subv2/f6" 0
> +
> +echo
> +echo "Testing with different subvolumes and with transaction commit"
> +echo
> +run_test "$SCRATCH_MNT/subv1/f7" "$SCRATCH_MNT/subv2/f8" 1
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/279.out b/tests/btrfs/279.out
> new file mode 100644
> index 00000000..a959a86d
> --- /dev/null
> +++ b/tests/btrfs/279.out
> @@ -0,0 +1,39 @@
> +QA output created by 279
> +Create subvolume 'SCRATCH_MNT/subv1'
> +Create subvolume 'SCRATCH_MNT/subv2'
> +
> +Testing with same subvolume and without transaction commit
> +
> +wrote 65536/65536 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Fiemap of SCRATCH_MNT/subv1/f1 before deleting SCRATCH_MNT/subv1/f2:
> +0: [0..127]: shared|last
> +Fiemap of SCRATCH_MNT/subv1/f1 after deleting SCRATCH_MNT/subv1/f2:
> +0: [0..127]: last
> +
> +Testing with same subvolume and with transaction commit
> +
> +wrote 65536/65536 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Fiemap of SCRATCH_MNT/subv1/f3 before deleting SCRATCH_MNT/subv1/f4:
> +0: [0..127]: shared|last
> +Fiemap of SCRATCH_MNT/subv1/f3 after deleting SCRATCH_MNT/subv1/f4:
> +0: [0..127]: last
> +
> +Testing with different subvolumes and without transaction commit
> +
> +wrote 65536/65536 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Fiemap of SCRATCH_MNT/subv1/f5 before deleting SCRATCH_MNT/subv2/f6:
> +0: [0..127]: shared|last
> +Fiemap of SCRATCH_MNT/subv1/f5 after deleting SCRATCH_MNT/subv2/f6:
> +0: [0..127]: last
> +
> +Testing with different subvolumes and with transaction commit
> +
> +wrote 65536/65536 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Fiemap of SCRATCH_MNT/subv1/f7 before deleting SCRATCH_MNT/subv2/f8:
> +0: [0..127]: shared|last
> +Fiemap of SCRATCH_MNT/subv1/f7 after deleting SCRATCH_MNT/subv2/f8:
> +0: [0..127]: last
> -- 
> 2.35.1
> 

Re: [PATCH 2/3] btrfs: test that fiemap reports extent as not shared after deleting file

[Filipe Manana]

On Mon, Oct 31, 2022 at 4:41 PM Zorro Lang <zlang@redhat.com> wrote:
>
> On Mon, Oct 31, 2022 at 11:11:20AM +0000, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > Test that if we have two files with shared extents, after removing one of
> > the files, if we do a fiemap against the other file, it does not report
> > extents as shared anymore.
> >
> > This exercises the processing of delayed references for data extents in
> > the backref walking code, used by fiemap to determine if an extent is
> > shared.
> >
> > This used to fail until very recently and was fixed by the following
> > kernel commit that landed in 6.1-rc2:
> >
> >   4fc7b5722824 (""btrfs: fix processing of delayed data refs during backref walking")
> >
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
>
> Looks good to me. As this's not a genericcase, I'm not sure if you need
> _require_congruent_file_oplen helper.

Hum? Why would it be needed?
That helper was introduced because of xfs's realtime config.
On btrfs reflinking always works for any multiple of the sector size.

>
> Thanks,
> Zorro
>
> >  tests/btrfs/279     | 82 +++++++++++++++++++++++++++++++++++++++++++++
> >  tests/btrfs/279.out | 39 +++++++++++++++++++++
> >  2 files changed, 121 insertions(+)
> >  create mode 100755 tests/btrfs/279
> >  create mode 100644 tests/btrfs/279.out
> >
> > diff --git a/tests/btrfs/279 b/tests/btrfs/279
> > new file mode 100755
> > index 00000000..5b5824fd
> > --- /dev/null
> > +++ b/tests/btrfs/279
> > @@ -0,0 +1,82 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
> > +#
> > +# FS QA Test 279
> > +#
> > +# Test that if we have two files with shared extents, after removing one of the
> > +# files, if we do a fiemap against the other file, it does not report extents as
> > +# shared anymore.
> > +#
> > +# This exercises the processing of delayed references for data extents in the
> > +# backref walking code, used by fiemap to determine if an extent is shared.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick subvol fiemap clone
> > +
> > +. ./common/filter
> > +. ./common/reflink
> > +. ./common/punch # for _filter_fiemap_flags
> > +
> > +_supported_fs btrfs
> > +_require_scratch_reflink
> > +_require_cp_reflink
> > +_require_xfs_io_command "fiemap"
> > +
> > +_fixed_by_kernel_commit 4fc7b5722824 \
> > +     "btrfs: fix processing of delayed data refs during backref walking"
> > +
> > +run_test()
> > +{
> > +     local file_path_1=$1
> > +     local file_path_2=$2
> > +     local do_sync=$3
> > +
> > +     $XFS_IO_PROG -f -c "pwrite 0 64K" $file_path_1 | _filter_xfs_io
> > +     _cp_reflink $file_path_1 $file_path_2
> > +
> > +     if [ $do_sync -eq 1 ]; then
> > +             sync
> > +     fi
> > +
> > +     echo "Fiemap of $file_path_1 before deleting $file_path_2:" | \
> > +             _filter_scratch
> > +     $XFS_IO_PROG -c "fiemap -v" $file_path_1 | _filter_fiemap_flags
> > +
> > +     rm -f $file_path_2
> > +
> > +     echo "Fiemap of $file_path_1 after deleting $file_path_2:" | \
> > +             _filter_scratch
> > +     $XFS_IO_PROG -c "fiemap -v" $file_path_1 | _filter_fiemap_flags
> > +}
> > +
> > +_scratch_mkfs >> $seqres.full 2>&1
> > +_scratch_mount
> > +
> > +# Create two test subvolumes, we'll reflink files between them.
> > +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subv1 | _filter_scratch
> > +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subv2 | _filter_scratch
> > +
> > +echo
> > +echo "Testing with same subvolume and without transaction commit"
> > +echo
> > +run_test "$SCRATCH_MNT/subv1/f1" "$SCRATCH_MNT/subv1/f2" 0
> > +
> > +echo
> > +echo "Testing with same subvolume and with transaction commit"
> > +echo
> > +run_test "$SCRATCH_MNT/subv1/f3" "$SCRATCH_MNT/subv1/f4" 1
> > +
> > +echo
> > +echo "Testing with different subvolumes and without transaction commit"
> > +echo
> > +run_test "$SCRATCH_MNT/subv1/f5" "$SCRATCH_MNT/subv2/f6" 0
> > +
> > +echo
> > +echo "Testing with different subvolumes and with transaction commit"
> > +echo
> > +run_test "$SCRATCH_MNT/subv1/f7" "$SCRATCH_MNT/subv2/f8" 1
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/btrfs/279.out b/tests/btrfs/279.out
> > new file mode 100644
> > index 00000000..a959a86d
> > --- /dev/null
> > +++ b/tests/btrfs/279.out
> > @@ -0,0 +1,39 @@
> > +QA output created by 279
> > +Create subvolume 'SCRATCH_MNT/subv1'
> > +Create subvolume 'SCRATCH_MNT/subv2'
> > +
> > +Testing with same subvolume and without transaction commit
> > +
> > +wrote 65536/65536 bytes at offset 0
> > +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > +Fiemap of SCRATCH_MNT/subv1/f1 before deleting SCRATCH_MNT/subv1/f2:
> > +0: [0..127]: shared|last
> > +Fiemap of SCRATCH_MNT/subv1/f1 after deleting SCRATCH_MNT/subv1/f2:
> > +0: [0..127]: last
> > +
> > +Testing with same subvolume and with transaction commit
> > +
> > +wrote 65536/65536 bytes at offset 0
> > +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > +Fiemap of SCRATCH_MNT/subv1/f3 before deleting SCRATCH_MNT/subv1/f4:
> > +0: [0..127]: shared|last
> > +Fiemap of SCRATCH_MNT/subv1/f3 after deleting SCRATCH_MNT/subv1/f4:
> > +0: [0..127]: last
> > +
> > +Testing with different subvolumes and without transaction commit
> > +
> > +wrote 65536/65536 bytes at offset 0
> > +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > +Fiemap of SCRATCH_MNT/subv1/f5 before deleting SCRATCH_MNT/subv2/f6:
> > +0: [0..127]: shared|last
> > +Fiemap of SCRATCH_MNT/subv1/f5 after deleting SCRATCH_MNT/subv2/f6:
> > +0: [0..127]: last
> > +
> > +Testing with different subvolumes and with transaction commit
> > +
> > +wrote 65536/65536 bytes at offset 0
> > +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > +Fiemap of SCRATCH_MNT/subv1/f7 before deleting SCRATCH_MNT/subv2/f8:
> > +0: [0..127]: shared|last
> > +Fiemap of SCRATCH_MNT/subv1/f7 after deleting SCRATCH_MNT/subv2/f8:
> > +0: [0..127]: last
> > --
> > 2.35.1
> >
>

Re: [PATCH 2/3] btrfs: test that fiemap reports extent as not shared after deleting file

[Zorro Lang]

On Mon, Oct 31, 2022 at 04:51:06PM +0000, Filipe Manana wrote:
> On Mon, Oct 31, 2022 at 4:41 PM Zorro Lang <zlang@redhat.com> wrote:
> >
> > On Mon, Oct 31, 2022 at 11:11:20AM +0000, fdmanana@kernel.org wrote:
> > > From: Filipe Manana <fdmanana@suse.com>
> > >
> > > Test that if we have two files with shared extents, after removing one of
> > > the files, if we do a fiemap against the other file, it does not report
> > > extents as shared anymore.
> > >
> > > This exercises the processing of delayed references for data extents in
> > > the backref walking code, used by fiemap to determine if an extent is
> > > shared.
> > >
> > > This used to fail until very recently and was fixed by the following
> > > kernel commit that landed in 6.1-rc2:
> > >
> > >   4fc7b5722824 (""btrfs: fix processing of delayed data refs during backref walking")
> > >
> > > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > > ---
> >
> > Looks good to me. As this's not a genericcase, I'm not sure if you need
> > _require_congruent_file_oplen helper.
> 
> Hum? Why would it be needed?
> That helper was introduced because of xfs's realtime config.
> On btrfs reflinking always works for any multiple of the sector size.

Sure, I was not asking you to use that, as it's not a generic case, so I just
tried to double check with you, to make sure btrfs doesn't need it. Thanks
for you confirm that, this patchset looks good to me.

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

Thanks,
Zorro

> 
> >
> > Thanks,
> > Zorro
> >
> > >  tests/btrfs/279     | 82 +++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/btrfs/279.out | 39 +++++++++++++++++++++
> > >  2 files changed, 121 insertions(+)
> > >  create mode 100755 tests/btrfs/279
> > >  create mode 100644 tests/btrfs/279.out
> > >
> > > diff --git a/tests/btrfs/279 b/tests/btrfs/279
> > > new file mode 100755
> > > index 00000000..5b5824fd
> > > --- /dev/null
> > > +++ b/tests/btrfs/279
> > > @@ -0,0 +1,82 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
> > > +#
> > > +# FS QA Test 279
> > > +#
> > > +# Test that if we have two files with shared extents, after removing one of the
> > > +# files, if we do a fiemap against the other file, it does not report extents as
> > > +# shared anymore.
> > > +#
> > > +# This exercises the processing of delayed references for data extents in the
> > > +# backref walking code, used by fiemap to determine if an extent is shared.
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto quick subvol fiemap clone
> > > +
> > > +. ./common/filter
> > > +. ./common/reflink
> > > +. ./common/punch # for _filter_fiemap_flags
> > > +
> > > +_supported_fs btrfs
> > > +_require_scratch_reflink
> > > +_require_cp_reflink
> > > +_require_xfs_io_command "fiemap"
> > > +
> > > +_fixed_by_kernel_commit 4fc7b5722824 \
> > > +     "btrfs: fix processing of delayed data refs during backref walking"
> > > +
> > > +run_test()
> > > +{
> > > +     local file_path_1=$1
> > > +     local file_path_2=$2
> > > +     local do_sync=$3
> > > +
> > > +     $XFS_IO_PROG -f -c "pwrite 0 64K" $file_path_1 | _filter_xfs_io
> > > +     _cp_reflink $file_path_1 $file_path_2
> > > +
> > > +     if [ $do_sync -eq 1 ]; then
> > > +             sync
> > > +     fi
> > > +
> > > +     echo "Fiemap of $file_path_1 before deleting $file_path_2:" | \
> > > +             _filter_scratch
> > > +     $XFS_IO_PROG -c "fiemap -v" $file_path_1 | _filter_fiemap_flags
> > > +
> > > +     rm -f $file_path_2
> > > +
> > > +     echo "Fiemap of $file_path_1 after deleting $file_path_2:" | \
> > > +             _filter_scratch
> > > +     $XFS_IO_PROG -c "fiemap -v" $file_path_1 | _filter_fiemap_flags
> > > +}
> > > +
> > > +_scratch_mkfs >> $seqres.full 2>&1
> > > +_scratch_mount
> > > +
> > > +# Create two test subvolumes, we'll reflink files between them.
> > > +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subv1 | _filter_scratch
> > > +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subv2 | _filter_scratch
> > > +
> > > +echo
> > > +echo "Testing with same subvolume and without transaction commit"
> > > +echo
> > > +run_test "$SCRATCH_MNT/subv1/f1" "$SCRATCH_MNT/subv1/f2" 0
> > > +
> > > +echo
> > > +echo "Testing with same subvolume and with transaction commit"
> > > +echo
> > > +run_test "$SCRATCH_MNT/subv1/f3" "$SCRATCH_MNT/subv1/f4" 1
> > > +
> > > +echo
> > > +echo "Testing with different subvolumes and without transaction commit"
> > > +echo
> > > +run_test "$SCRATCH_MNT/subv1/f5" "$SCRATCH_MNT/subv2/f6" 0
> > > +
> > > +echo
> > > +echo "Testing with different subvolumes and with transaction commit"
> > > +echo
> > > +run_test "$SCRATCH_MNT/subv1/f7" "$SCRATCH_MNT/subv2/f8" 1
> > > +
> > > +# success, all done
> > > +status=0
> > > +exit
> > > diff --git a/tests/btrfs/279.out b/tests/btrfs/279.out
> > > new file mode 100644
> > > index 00000000..a959a86d
> > > --- /dev/null
> > > +++ b/tests/btrfs/279.out
> > > @@ -0,0 +1,39 @@
> > > +QA output created by 279
> > > +Create subvolume 'SCRATCH_MNT/subv1'
> > > +Create subvolume 'SCRATCH_MNT/subv2'
> > > +
> > > +Testing with same subvolume and without transaction commit
> > > +
> > > +wrote 65536/65536 bytes at offset 0
> > > +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > > +Fiemap of SCRATCH_MNT/subv1/f1 before deleting SCRATCH_MNT/subv1/f2:
> > > +0: [0..127]: shared|last
> > > +Fiemap of SCRATCH_MNT/subv1/f1 after deleting SCRATCH_MNT/subv1/f2:
> > > +0: [0..127]: last
> > > +
> > > +Testing with same subvolume and with transaction commit
> > > +
> > > +wrote 65536/65536 bytes at offset 0
> > > +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > > +Fiemap of SCRATCH_MNT/subv1/f3 before deleting SCRATCH_MNT/subv1/f4:
> > > +0: [0..127]: shared|last
> > > +Fiemap of SCRATCH_MNT/subv1/f3 after deleting SCRATCH_MNT/subv1/f4:
> > > +0: [0..127]: last
> > > +
> > > +Testing with different subvolumes and without transaction commit
> > > +
> > > +wrote 65536/65536 bytes at offset 0
> > > +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > > +Fiemap of SCRATCH_MNT/subv1/f5 before deleting SCRATCH_MNT/subv2/f6:
> > > +0: [0..127]: shared|last
> > > +Fiemap of SCRATCH_MNT/subv1/f5 after deleting SCRATCH_MNT/subv2/f6:
> > > +0: [0..127]: last
> > > +
> > > +Testing with different subvolumes and with transaction commit
> > > +
> > > +wrote 65536/65536 bytes at offset 0
> > > +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > > +Fiemap of SCRATCH_MNT/subv1/f7 before deleting SCRATCH_MNT/subv2/f8:
> > > +0: [0..127]: shared|last
> > > +Fiemap of SCRATCH_MNT/subv1/f7 after deleting SCRATCH_MNT/subv2/f8:
> > > +0: [0..127]: last
> > > --
> > > 2.35.1
> > >
> >
> 

Re: [PATCH 3/3] btrfs: test fiemap reports extent as not shared after COWing it in snapshot

[Zorro Lang]

On Mon, Oct 31, 2022 at 11:11:21AM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Test that if we have a large file, with file extent items spanning several
> leaves in the fs tree, and that is shared due to a snapshot, if we COW one
> of the extents, doing a fiemap will report the respective file range as
> not shared.
> 
> This exercises the processing of delayed references for metadata extents
> in the backref walking code, used by fiemap to determine if an extent is
> shared.
> 
> This used to fail until very recently and was fixed by the following
> kernel commit that landed in 6.1-rc2:
> 
>   943553ef9b51 (""btrfs: fix processing of delayed tree block refs during backref walking")
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/btrfs/280     | 64 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/280.out | 21 +++++++++++++++
>  2 files changed, 85 insertions(+)
>  create mode 100755 tests/btrfs/280
>  create mode 100644 tests/btrfs/280.out
> 
> diff --git a/tests/btrfs/280 b/tests/btrfs/280
> new file mode 100755
> index 00000000..06ef221e
> --- /dev/null
> +++ b/tests/btrfs/280
> @@ -0,0 +1,64 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
> +#
> +# FS QA Test 280
> +#
> +# Test that if we have a large file, with file extent items spanning several
> +# leaves in the fs tree, and that is shared due to a snapshot, if we COW one of
> +# the extents, doing a fiemap will report the respective file range as not
> +# shared.
> +#
> +# This exercises the processing of delayed references for metadata extents in
> +# the backref walking code, used by fiemap to determine if an extent is shared.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick compress snapshot fiemap
> +
> +. ./common/filter
> +. ./common/punch # for _filter_fiemap_flags
> +
> +_supported_fs btrfs
> +_require_scratch
> +_require_xfs_io_command "fiemap"
> +
> +_fixed_by_kernel_commit 943553ef9b51 \
> +	"btrfs: fix processing of delayed tree block refs during backref walking"
> +
> +_scratch_mkfs >> $seqres.full 2>&1
> +# We use compression because it's a very quick way to create a file with a very
> +# large number of extents (compression limits the maximum extent size to 128K)
> +# and while using very little disk space.
> +_scratch_mount -o compress
> +
> +# A 128M file full of compressed extents results in a fs tree with a heigth
> +# of 2 (root at level 1), so we'll end up with tree block references in the
> +# extent tree (if the root was a leaf, we would have only data references).
> +$XFS_IO_PROG -f -c "pwrite -b 1M 0 128M" $SCRATCH_MNT/foo | _filter_xfs_io
> +
> +# Create a RW snapshot of the default subvolume.
> +$BTRFS_UTIL_PROG subvolume snapshot $SCRATCH_MNT $SCRATCH_MNT/snap | _filter_scratch
> +
> +echo
> +echo "File foo fiemap before COWing extent:"
> +echo
> +$XFS_IO_PROG -c "fiemap -v" $SCRATCH_MNT/foo | _filter_fiemap_flags
> +
> +echo
> +echo "Overwriting file range [120M, 120M + 128K) in the snapshot"
> +echo
> +$XFS_IO_PROG -c "pwrite -b 128K 120M 128K" $SCRATCH_MNT/snap/foo | _filter_xfs_io
> +# Now fsync the file to force COWing the extent and the path from the root of
> +# the snapshot tree down to the leaf where the extent is at.
> +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/snap/foo
> +
> +echo
> +echo "File foo fiemap after COWing extent in the snapshot:"
> +echo
> +# Now we should have all extents marked as shared except the 128K extent in the
> +# file range [120M, 120M + 128K).
> +$XFS_IO_PROG -c "fiemap -v" $SCRATCH_MNT/foo | _filter_fiemap_flags
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/280.out b/tests/btrfs/280.out
> new file mode 100644
> index 00000000..c3f82966
> --- /dev/null
> +++ b/tests/btrfs/280.out
> @@ -0,0 +1,21 @@
> +QA output created by 280
> +wrote 134217728/134217728 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Create a snapshot of 'SCRATCH_MNT' in 'SCRATCH_MNT/snap'
> +
> +File foo fiemap before COWing extent:
> +
> +0: [0..261887]: shared
> +1: [261888..262143]: shared|last
> +
> +Overwriting file range [120M, 120M + 128K) in the snapshot
> +
> +wrote 131072/131072 bytes at offset 125829120
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +File foo fiemap after COWing extent in the snapshot:
> +
> +0: [0..245759]: shared
> +1: [245760..246015]: none
> +2: [246016..261887]: shared
> +3: [261888..262143]: shared|last
> -- 
> 2.35.1
>