[PATCH] xfstests: add regression test for btrfs incremental send

[PATCH] xfstests: add regression test for btrfs incremental send

[Filipe David Borba Manana]

Test for a btrfs incremental send issue where we end up sending a
wrong section of data from a file extent if the corresponding file
extent is compressed and the respective file extent item has a non
zero data offset.

Fixed by the following linux kernel btrfs patch:

   Btrfs: use right clone root offset for compressed extents

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 tests/btrfs/040     |  103 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/040.out |    1 +
 tests/btrfs/group   |    1 +
 3 files changed, 105 insertions(+)
 create mode 100755 tests/btrfs/040
 create mode 100644 tests/btrfs/040.out

diff --git a/tests/btrfs/040 b/tests/btrfs/040
new file mode 100755
index 0000000..c1f3d13
--- /dev/null
+++ b/tests/btrfs/040
@@ -0,0 +1,103 @@
+#! /bin/bash
+# FS QA Test No. btrfs/040
+#
+# Test for a btrfs incremental send issue where we end up sending a
+# wrong section of data from a file extent if the corresponding file
+# extent is compressed and the respective file extent item has a non
+# zero data offset.
+#
+# Fixed by the following linux kernel btrfs patch:
+#
+#   Btrfs: use right clone root offset for compressed extents
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Filipe Manana.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=`mktemp -d`
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    rm -fr $tmp
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+_need_to_be_root
+
+FSSUM_PROG=$here/src/fssum
+[ -x $FSSUM_PROG ] || _notrun "fssum not built"
+
+rm -f $seqres.full
+
+_scratch_mkfs >/dev/null 2>&1
+_scratch_mount "-o compress-force=lzo"
+
+run_check $XFS_IO_PROG -f -c "truncate 118811" $SCRATCH_MNT/foo
+$XFS_IO_PROG -c "fpunch 582007 864596" $SCRATCH_MNT/foo
+run_check $XFS_IO_PROG -c "pwrite -S 0x0d -b 39987 92267 39987" \
+	$SCRATCH_MNT/foo
+
+run_check $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
+	$SCRATCH_MNT/mysnap1
+
+run_check $XFS_IO_PROG -c "pwrite -S 0xe1 -b 38804 1119395 38804" \
+	$SCRATCH_MNT/foo
+run_check $XFS_IO_PROG -c "pwrite -S 0x0e -b 41125 80802 41125" \
+	$SCRATCH_MNT/foo
+
+run_check $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
+	$SCRATCH_MNT/mysnap2
+
+run_check $FSSUM_PROG -A -f -w $tmp/1.fssum $SCRATCH_MNT/mysnap1
+run_check $FSSUM_PROG -A -f -w $tmp/2.fssum -x $SCRATCH_MNT/mysnap2/mysnap1 \
+	$SCRATCH_MNT/mysnap2
+
+run_check $BTRFS_UTIL_PROG send $SCRATCH_MNT/mysnap1 -f $tmp/1.snap
+run_check $BTRFS_UTIL_PROG send -p $SCRATCH_MNT/mysnap1 $SCRATCH_MNT/mysnap2 \
+	-f $tmp/2.snap
+
+_scratch_unmount
+_check_btrfs_filesystem $SCRATCH_DEV
+
+_scratch_mkfs >/dev/null 2>&1
+_scratch_mount
+
+run_check $BTRFS_UTIL_PROG receive $SCRATCH_MNT -f $tmp/1.snap
+run_check $FSSUM_PROG -r $tmp/1.fssum $SCRATCH_MNT/mysnap1 2>> $seqres.full
+
+run_check $BTRFS_UTIL_PROG receive $SCRATCH_MNT -f $tmp/2.snap
+run_check $FSSUM_PROG -r $tmp/2.fssum $SCRATCH_MNT/mysnap2 2>> $seqres.full
+
+_scratch_unmount
+_check_btrfs_filesystem $SCRATCH_DEV
+
+status=0
+exit
diff --git a/tests/btrfs/040.out b/tests/btrfs/040.out
new file mode 100644
index 0000000..7740549
--- /dev/null
+++ b/tests/btrfs/040.out
@@ -0,0 +1 @@
+QA output created by 040
diff --git a/tests/btrfs/group b/tests/btrfs/group
index 2ca2225..a687634 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -38,3 +38,4 @@
 033 auto quick
 034 auto quick
 036 auto quick
+040 auto quick
-- 
1.7.9.5

[PATCH v2] xfstests: add regression test for btrfs incremental send

[Filipe David Borba Manana]

Test for a btrfs incremental send issue where we end up sending a
wrong section of data from a file extent if the corresponding file
extent is compressed and the respective file extent item has a non
zero data offset.

Fixed by the following linux kernel btrfs patch:

   Btrfs: use right clone root offset for compressed extents

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---

V2: Made the test more reliable. Now it doesn't depend anymore of btrfs'
    hole punch implementation leaving hole file extent items when we punch
    beyond the file's current size.

 tests/btrfs/040     |  115 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/040.out |    1 +
 tests/btrfs/group   |    1 +
 3 files changed, 117 insertions(+)
 create mode 100755 tests/btrfs/040
 create mode 100644 tests/btrfs/040.out

diff --git a/tests/btrfs/040 b/tests/btrfs/040
new file mode 100755
index 0000000..d6b37bf
--- /dev/null
+++ b/tests/btrfs/040
@@ -0,0 +1,115 @@
+#! /bin/bash
+# FS QA Test No. btrfs/040
+#
+# Test for a btrfs incremental send issue where we end up sending a
+# wrong section of data from a file extent if the corresponding file
+# extent is compressed and the respective file extent item has a non
+# zero data offset.
+#
+# Fixed by the following linux kernel btrfs patch:
+#
+#   Btrfs: use right clone root offset for compressed extents
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Filipe Manana.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=`mktemp -d`
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    rm -fr $tmp
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+_need_to_be_root
+
+FSSUM_PROG=$here/src/fssum
+[ -x $FSSUM_PROG ] || _notrun "fssum not built"
+
+rm -f $seqres.full
+
+_scratch_mkfs >/dev/null 2>&1
+_scratch_mount "-o compress-force=lzo"
+
+run_check $XFS_IO_PROG -f -c "truncate 118811" $SCRATCH_MNT/foo
+run_check $XFS_IO_PROG -c "pwrite -S 0x0d -b 39987 92267 39987" \
+	$SCRATCH_MNT/foo
+
+run_check $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
+	$SCRATCH_MNT/mysnap1
+
+run_check $XFS_IO_PROG -c "pwrite -S 0x3e -b 80000 200000 80000" \
+	$SCRATCH_MNT/foo
+run_check $BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT
+run_check $XFS_IO_PROG -c "pwrite -S 0xdc -b 10000 250000 10000" \
+	$SCRATCH_MNT/foo
+run_check $XFS_IO_PROG -c "pwrite -S 0xff -b 10000 300000 10000" \
+	$SCRATCH_MNT/foo
+
+# will be used for incremental send to be able to issue clone operations
+run_check $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
+	$SCRATCH_MNT/clones_snap
+
+run_check $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
+	$SCRATCH_MNT/mysnap2
+
+run_check $FSSUM_PROG -A -f -w $tmp/1.fssum $SCRATCH_MNT/mysnap1
+run_check $FSSUM_PROG -A -f -w $tmp/2.fssum -x $SCRATCH_MNT/mysnap2/mysnap1 \
+	-x $SCRATCH_MNT/mysnap2/clones_snap $SCRATCH_MNT/mysnap2
+run_check $FSSUM_PROG -A -f -w $tmp/clones.fssum $SCRATCH_MNT/clones_snap \
+	-x $SCRATCH_MNT/clones_snap/mysnap1 -x $SCRATCH_MNT/clones_snap/mysnap2
+
+run_check $BTRFS_UTIL_PROG send $SCRATCH_MNT/mysnap1 -f $tmp/1.snap
+run_check $BTRFS_UTIL_PROG send $SCRATCH_MNT/clones_snap -f $tmp/clones.snap
+run_check $BTRFS_UTIL_PROG send -p $SCRATCH_MNT/mysnap1 \
+	-c $SCRATCH_MNT/clones_snap $SCRATCH_MNT/mysnap2 -f $tmp/2.snap
+
+_scratch_unmount
+_check_btrfs_filesystem $SCRATCH_DEV
+
+_scratch_mkfs >/dev/null 2>&1
+_scratch_mount
+
+run_check $BTRFS_UTIL_PROG receive $SCRATCH_MNT -f $tmp/1.snap
+run_check $FSSUM_PROG -r $tmp/1.fssum $SCRATCH_MNT/mysnap1 2>> $seqres.full
+
+run_check $BTRFS_UTIL_PROG receive $SCRATCH_MNT -f $tmp/clones.snap
+run_check $FSSUM_PROG -r $tmp/clones.fssum $SCRATCH_MNT/clones_snap 2>> $seqres.full
+
+run_check $BTRFS_UTIL_PROG receive $SCRATCH_MNT -f $tmp/2.snap
+run_check $FSSUM_PROG -r $tmp/2.fssum $SCRATCH_MNT/mysnap2 2>> $seqres.full
+
+_scratch_unmount
+_check_btrfs_filesystem $SCRATCH_DEV
+
+status=0
+exit
diff --git a/tests/btrfs/040.out b/tests/btrfs/040.out
new file mode 100644
index 0000000..7740549
--- /dev/null
+++ b/tests/btrfs/040.out
@@ -0,0 +1 @@
+QA output created by 040
diff --git a/tests/btrfs/group b/tests/btrfs/group
index 2ca2225..a687634 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -38,3 +38,4 @@
 033 auto quick
 034 auto quick
 036 auto quick
+040 auto quick
-- 
1.7.9.5

Re: [PATCH v2] xfstests: add regression test for btrfs incremental send

[Dave Chinner]

On Sat, Feb 15, 2014 at 03:36:13PM +0000, Filipe David Borba Manana wrote:
> Test for a btrfs incremental send issue where we end up sending a
> wrong section of data from a file extent if the corresponding file
> extent is compressed and the respective file extent item has a non
> zero data offset.
> 
> Fixed by the following linux kernel btrfs patch:
> 
>    Btrfs: use right clone root offset for compressed extents
> 
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> ---
> 
> V2: Made the test more reliable. Now it doesn't depend anymore of btrfs'
>     hole punch implementation leaving hole file extent items when we punch
>     beyond the file's current size.
> 
>  tests/btrfs/040     |  115 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/040.out |    1 +
>  tests/btrfs/group   |    1 +
>  3 files changed, 117 insertions(+)
>  create mode 100755 tests/btrfs/040
>  create mode 100644 tests/btrfs/040.out
> 
> diff --git a/tests/btrfs/040 b/tests/btrfs/040
> new file mode 100755
> index 0000000..d6b37bf
> --- /dev/null
> +++ b/tests/btrfs/040
> @@ -0,0 +1,115 @@
> +#! /bin/bash
> +# FS QA Test No. btrfs/040
> +#
> +# Test for a btrfs incremental send issue where we end up sending a
> +# wrong section of data from a file extent if the corresponding file
> +# extent is compressed and the respective file extent item has a non
> +# zero data offset.
> +#
> +# Fixed by the following linux kernel btrfs patch:
> +#
> +#   Btrfs: use right clone root offset for compressed extents
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2014 Filipe Manana.  All Rights Reserved.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=`mktemp -d`
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +    rm -fr $tmp
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +_need_to_be_root
> +
> +FSSUM_PROG=$here/src/fssum
> +[ -x $FSSUM_PROG ] || _notrun "fssum not built"
> +
> +rm -f $seqres.full
> +
> +_scratch_mkfs >/dev/null 2>&1
> +_scratch_mount "-o compress-force=lzo"
> +
> +run_check $XFS_IO_PROG -f -c "truncate 118811" $SCRATCH_MNT/foo
> +run_check $XFS_IO_PROG -c "pwrite -S 0x0d -b 39987 92267 39987" \
> +	$SCRATCH_MNT/foo

Ugh. filter the output, don't use run_check.

$XFS_IO_PROG -f -c "truncate 118811" $SCRATCH_MNT/foo
$XFS_IO_PROG -c "pwrite -S 0x0d -b 39987 92267 39987" \
	$SCRATCH_MNT/foo | _filter_xfs_io

If something fails, we still want the test to continue running, even
if all it does is exercise error handling paths. run_check simply
terminates the test at the first failure.

> +run_check $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
> +	$SCRATCH_MNT/mysnap1
> +
> +run_check $XFS_IO_PROG -c "pwrite -S 0x3e -b 80000 200000 80000" \
> +	$SCRATCH_MNT/foo
> +run_check $BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT

Why a special btrfs sync here? Why isn't "sync" sufficient, or even
a synchronous write or write plus fsync like:

$XFS_IO_PROG -c "pwrite -S 0x3e -b 80000 200000 80000" -c "fsync" \
	$SCRATCH_MNT/foo | _filter_xfs_io

Tests need to be documented the same way code is documented....

> +run_check $XFS_IO_PROG -c "pwrite -S 0xdc -b 10000 250000 10000" \
> +	$SCRATCH_MNT/foo
> +run_check $XFS_IO_PROG -c "pwrite -S 0xff -b 10000 300000 10000" \
> +	$SCRATCH_MNT/foo

I'm getting to the point where I'm starting to consider "run_check"
as being harmful....

I know you are trying to work around the fact that the btrfs
progs commands have inconsistent output and so are difficult to
match. However, given that this is leading to bad habits like using
run_check for everything.

I'd suggest that we need a set of $BTRFS_UTIL_PROG specific handlers
to deal with these differences rather than continuing to pollute the
tests with run_check. e.g.

_run_btrfs_util_prog()
{
	run_check $BTRFS_UTIL_PROG $*
}

would be a good start because it gets that run_check pattern out of
the main test scripts and hence out of the heads of test writers.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v2] xfstests: add regression test for btrfs incremental send

[Filipe David Manana]

On Sun, Feb 16, 2014 at 11:08 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Sat, Feb 15, 2014 at 03:36:13PM +0000, Filipe David Borba Manana wrote:
>> Test for a btrfs incremental send issue where we end up sending a
>> wrong section of data from a file extent if the corresponding file
>> extent is compressed and the respective file extent item has a non
>> zero data offset.
>>
>> Fixed by the following linux kernel btrfs patch:
>>
>>    Btrfs: use right clone root offset for compressed extents
>>
>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>> ---
>>
>> V2: Made the test more reliable. Now it doesn't depend anymore of btrfs'
>>     hole punch implementation leaving hole file extent items when we punch
>>     beyond the file's current size.
>>
>>  tests/btrfs/040     |  115 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/btrfs/040.out |    1 +
>>  tests/btrfs/group   |    1 +
>>  3 files changed, 117 insertions(+)
>>  create mode 100755 tests/btrfs/040
>>  create mode 100644 tests/btrfs/040.out
>>
>> diff --git a/tests/btrfs/040 b/tests/btrfs/040
>> new file mode 100755
>> index 0000000..d6b37bf
>> --- /dev/null
>> +++ b/tests/btrfs/040
>> @@ -0,0 +1,115 @@
>> +#! /bin/bash
>> +# FS QA Test No. btrfs/040
>> +#
>> +# Test for a btrfs incremental send issue where we end up sending a
>> +# wrong section of data from a file extent if the corresponding file
>> +# extent is compressed and the respective file extent item has a non
>> +# zero data offset.
>> +#
>> +# Fixed by the following linux kernel btrfs patch:
>> +#
>> +#   Btrfs: use right clone root offset for compressed extents
>> +#
>> +#-----------------------------------------------------------------------
>> +# Copyright (c) 2014 Filipe Manana.  All Rights Reserved.
>> +#
>> +# This program is free software; you can redistribute it and/or
>> +# modify it under the terms of the GNU General Public License as
>> +# published by the Free Software Foundation.
>> +#
>> +# This program is distributed in the hope that it would be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program; if not, write the Free Software Foundation,
>> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>> +#-----------------------------------------------------------------------
>> +#
>> +
>> +seq=`basename $0`
>> +seqres=$RESULT_DIR/$seq
>> +echo "QA output created by $seq"
>> +
>> +here=`pwd`
>> +tmp=`mktemp -d`
>> +status=1     # failure is the default!
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +_cleanup()
>> +{
>> +    rm -fr $tmp
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/filter
>> +
>> +# real QA test starts here
>> +_supported_fs btrfs
>> +_supported_os Linux
>> +_require_scratch
>> +_need_to_be_root
>> +
>> +FSSUM_PROG=$here/src/fssum
>> +[ -x $FSSUM_PROG ] || _notrun "fssum not built"
>> +
>> +rm -f $seqres.full
>> +
>> +_scratch_mkfs >/dev/null 2>&1
>> +_scratch_mount "-o compress-force=lzo"
>> +
>> +run_check $XFS_IO_PROG -f -c "truncate 118811" $SCRATCH_MNT/foo
>> +run_check $XFS_IO_PROG -c "pwrite -S 0x0d -b 39987 92267 39987" \
>> +     $SCRATCH_MNT/foo
>
> Ugh. filter the output, don't use run_check.
>
> $XFS_IO_PROG -f -c "truncate 118811" $SCRATCH_MNT/foo
> $XFS_IO_PROG -c "pwrite -S 0x0d -b 39987 92267 39987" \
>         $SCRATCH_MNT/foo | _filter_xfs_io
>
> If something fails, we still want the test to continue running, even
> if all it does is exercise error handling paths. run_check simply
> terminates the test at the first failure.

What's the point of continuing? The test will fail anyway, all of the
xfs_io calls are necessary to trigger the bug.

>
>> +run_check $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
>> +     $SCRATCH_MNT/mysnap1
>> +
>> +run_check $XFS_IO_PROG -c "pwrite -S 0x3e -b 80000 200000 80000" \
>> +     $SCRATCH_MNT/foo
>> +run_check $BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT
>
> Why a special btrfs sync here? Why isn't "sync" sufficient, or even
> a synchronous write or write plus fsync like:
>
> $XFS_IO_PROG -c "pwrite -S 0x3e -b 80000 200000 80000" -c "fsync" \
>         $SCRATCH_MNT/foo | _filter_xfs_io
>
> Tests need to be documented the same way code is documented....
>
>> +run_check $XFS_IO_PROG -c "pwrite -S 0xdc -b 10000 250000 10000" \
>> +     $SCRATCH_MNT/foo
>> +run_check $XFS_IO_PROG -c "pwrite -S 0xff -b 10000 300000 10000" \
>> +     $SCRATCH_MNT/foo
>
> I'm getting to the point where I'm starting to consider "run_check"
> as being harmful....

Ok...

>
> I know you are trying to work around the fact that the btrfs
> progs commands have inconsistent output and so are difficult to
> match. However, given that this is leading to bad habits like using
> run_check for everything.
>
> I'd suggest that we need a set of $BTRFS_UTIL_PROG specific handlers
> to deal with these differences rather than continuing to pollute the
> tests with run_check. e.g.
>
> _run_btrfs_util_prog()
> {
>         run_check $BTRFS_UTIL_PROG $*
> }
>
> would be a good start because it gets that run_check pattern out of
> the main test scripts and hence out of the heads of test writers.

Well, will get rid of those run_check calls, but that will imply
adding some | _filter_scratch in many places. So shortening lines is
not a great argument :)

thanks

>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

[PATCH v3] xfstests: add regression test for btrfs incremental send

[Filipe David Borba Manana]

Test for a btrfs incremental send issue where we end up sending a
wrong section of data from a file extent if the corresponding file
extent is compressed and the respective file extent item has a non
zero data offset.

Fixed by the following linux kernel btrfs patch:

   Btrfs: use right clone root offset for compressed extents

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---

V2: Made the test more reliable. Now it doesn't depend anymore of btrfs'
    hole punch implementation leaving hole file extent items when we punch
    beyond the file's current size.
V3: Filter xfs_io output and make less use of the run_check function, as
    suggested by Dave Chinner.

 common/rc           |    5 +++
 tests/btrfs/040     |  119 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/040.out |    9 ++++
 tests/btrfs/group   |    1 +
 4 files changed, 134 insertions(+)
 create mode 100755 tests/btrfs/040
 create mode 100644 tests/btrfs/040.out

diff --git a/common/rc b/common/rc
index e91568b..27be009 100644
--- a/common/rc
+++ b/common/rc
@@ -2207,6 +2207,11 @@ run_check()
 	"$@" >> $seqres.full 2>&1 || _fail "failed: '$@'"
 }
 
+_run_btrfs_util_prog()
+{
+	run_check $BTRFS_UTIL_PROG $*
+}
+
 init_rc()
 {
 	if [ "$iam" == new ]
diff --git a/tests/btrfs/040 b/tests/btrfs/040
new file mode 100755
index 0000000..9037782
--- /dev/null
+++ b/tests/btrfs/040
@@ -0,0 +1,119 @@
+#! /bin/bash
+# FS QA Test No. btrfs/040
+#
+# Test for a btrfs incremental send issue where we end up sending a
+# wrong section of data from a file extent if the corresponding file
+# extent is compressed and the respective file extent item has a non
+# zero data offset.
+#
+# Fixed by the following linux kernel btrfs patch:
+#
+#   Btrfs: use right clone root offset for compressed extents
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Filipe Manana.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=`mktemp -d`
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    rm -fr $tmp
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+_need_to_be_root
+
+FSSUM_PROG=$here/src/fssum
+[ -x $FSSUM_PROG ] || _notrun "fssum not built"
+
+rm -f $seqres.full
+
+_scratch_mkfs >/dev/null 2>&1
+_scratch_mount "-o compress-force=lzo"
+
+$XFS_IO_PROG -f -c "truncate 118811" $SCRATCH_MNT/foo
+$XFS_IO_PROG -c "pwrite -S 0x0d -b 39987 92267 39987" \
+	$SCRATCH_MNT/foo | _filter_xfs_io
+
+_run_btrfs_util_prog subvolume snapshot -r $SCRATCH_MNT \
+	$SCRATCH_MNT/mysnap1
+
+$XFS_IO_PROG -c "pwrite -S 0x3e -b 80000 200000 80000" \
+	$SCRATCH_MNT/foo | _filter_xfs_io
+
+# Sync to avoid btrfs merging file extent items, which would make the test
+# succeed when it should fail.
+_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
+
+$XFS_IO_PROG -c "pwrite -S 0xdc -b 10000 250000 10000" \
+	$SCRATCH_MNT/foo | _filter_xfs_io
+$XFS_IO_PROG -c "pwrite -S 0xff -b 10000 300000 10000" \
+	$SCRATCH_MNT/foo | _filter_xfs_io
+
+# will be used for incremental send to be able to issue clone operations
+_run_btrfs_util_prog subvolume snapshot -r $SCRATCH_MNT \
+	$SCRATCH_MNT/clones_snap
+
+_run_btrfs_util_prog subvolume snapshot -r $SCRATCH_MNT \
+	$SCRATCH_MNT/mysnap2
+
+run_check $FSSUM_PROG -A -f -w $tmp/1.fssum $SCRATCH_MNT/mysnap1
+run_check $FSSUM_PROG -A -f -w $tmp/2.fssum -x $SCRATCH_MNT/mysnap2/mysnap1 \
+	-x $SCRATCH_MNT/mysnap2/clones_snap $SCRATCH_MNT/mysnap2
+run_check $FSSUM_PROG -A -f -w $tmp/clones.fssum $SCRATCH_MNT/clones_snap \
+	-x $SCRATCH_MNT/clones_snap/mysnap1 -x $SCRATCH_MNT/clones_snap/mysnap2
+
+_run_btrfs_util_prog send $SCRATCH_MNT/mysnap1 -f $tmp/1.snap
+_run_btrfs_util_prog send $SCRATCH_MNT/clones_snap -f $tmp/clones.snap
+_run_btrfs_util_prog send -p $SCRATCH_MNT/mysnap1 \
+	-c $SCRATCH_MNT/clones_snap $SCRATCH_MNT/mysnap2 -f $tmp/2.snap
+
+_scratch_unmount
+_check_btrfs_filesystem $SCRATCH_DEV
+
+_scratch_mkfs >/dev/null 2>&1
+_scratch_mount
+
+_run_btrfs_util_prog receive $SCRATCH_MNT -f $tmp/1.snap
+run_check $FSSUM_PROG -r $tmp/1.fssum $SCRATCH_MNT/mysnap1 2>> $seqres.full
+
+_run_btrfs_util_prog receive $SCRATCH_MNT -f $tmp/clones.snap
+run_check $FSSUM_PROG -r $tmp/clones.fssum $SCRATCH_MNT/clones_snap 2>> $seqres.full
+
+_run_btrfs_util_prog receive $SCRATCH_MNT -f $tmp/2.snap
+run_check $FSSUM_PROG -r $tmp/2.fssum $SCRATCH_MNT/mysnap2 2>> $seqres.full
+
+_scratch_unmount
+_check_btrfs_filesystem $SCRATCH_DEV
+
+status=0
+exit
diff --git a/tests/btrfs/040.out b/tests/btrfs/040.out
new file mode 100644
index 0000000..cc77aed
--- /dev/null
+++ b/tests/btrfs/040.out
@@ -0,0 +1,9 @@
+QA output created by 040
+wrote 39987/39987 bytes at offset 92267
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 80000/80000 bytes at offset 200000
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 10000/10000 bytes at offset 250000
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 10000/10000 bytes at offset 300000
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/btrfs/group b/tests/btrfs/group
index 2ca2225..a687634 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -38,3 +38,4 @@
 033 auto quick
 034 auto quick
 036 auto quick
+040 auto quick
-- 
1.7.9.5

Re: [PATCH v2] xfstests: add regression test for btrfs incremental send

[Dave Chinner]

On Sun, Feb 16, 2014 at 11:43:17PM +0000, Filipe David Manana wrote:
> On Sun, Feb 16, 2014 at 11:08 PM, Dave Chinner <david@fromorbit.com> wrote:
> > On Sat, Feb 15, 2014 at 03:36:13PM +0000, Filipe David Borba Manana wrote:
> >> Test for a btrfs incremental send issue where we end up sending a
> >> wrong section of data from a file extent if the corresponding file
> >> extent is compressed and the respective file extent item has a non
> >> zero data offset.
> >>
> >> Fixed by the following linux kernel btrfs patch:
> >>
> >>    Btrfs: use right clone root offset for compressed extents
> >>
> >> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> >> ---
> >>
> >> V2: Made the test more reliable. Now it doesn't depend anymore of btrfs'
> >>     hole punch implementation leaving hole file extent items when we punch
> >>     beyond the file's current size.
> >>
> >>  tests/btrfs/040     |  115 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  tests/btrfs/040.out |    1 +
> >>  tests/btrfs/group   |    1 +
> >>  3 files changed, 117 insertions(+)
> >>  create mode 100755 tests/btrfs/040
> >>  create mode 100644 tests/btrfs/040.out
> >>
> >> diff --git a/tests/btrfs/040 b/tests/btrfs/040
> >> new file mode 100755
> >> index 0000000..d6b37bf
> >> --- /dev/null
> >> +++ b/tests/btrfs/040
> >> @@ -0,0 +1,115 @@
> >> +#! /bin/bash
> >> +# FS QA Test No. btrfs/040
> >> +#
> >> +# Test for a btrfs incremental send issue where we end up sending a
> >> +# wrong section of data from a file extent if the corresponding file
> >> +# extent is compressed and the respective file extent item has a non
> >> +# zero data offset.
> >> +#
> >> +# Fixed by the following linux kernel btrfs patch:
> >> +#
> >> +#   Btrfs: use right clone root offset for compressed extents
> >> +#
> >> +#-----------------------------------------------------------------------
> >> +# Copyright (c) 2014 Filipe Manana.  All Rights Reserved.
> >> +#
> >> +# This program is free software; you can redistribute it and/or
> >> +# modify it under the terms of the GNU General Public License as
> >> +# published by the Free Software Foundation.
> >> +#
> >> +# This program is distributed in the hope that it would be useful,
> >> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> +# GNU General Public License for more details.
> >> +#
> >> +# You should have received a copy of the GNU General Public License
> >> +# along with this program; if not, write the Free Software Foundation,
> >> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> >> +#-----------------------------------------------------------------------
> >> +#
> >> +
> >> +seq=`basename $0`
> >> +seqres=$RESULT_DIR/$seq
> >> +echo "QA output created by $seq"
> >> +
> >> +here=`pwd`
> >> +tmp=`mktemp -d`
> >> +status=1     # failure is the default!
> >> +trap "_cleanup; exit \$status" 0 1 2 3 15
> >> +
> >> +_cleanup()
> >> +{
> >> +    rm -fr $tmp
> >> +}
> >> +
> >> +# get standard environment, filters and checks
> >> +. ./common/rc
> >> +. ./common/filter
> >> +
> >> +# real QA test starts here
> >> +_supported_fs btrfs
> >> +_supported_os Linux
> >> +_require_scratch
> >> +_need_to_be_root
> >> +
> >> +FSSUM_PROG=$here/src/fssum
> >> +[ -x $FSSUM_PROG ] || _notrun "fssum not built"
> >> +
> >> +rm -f $seqres.full
> >> +
> >> +_scratch_mkfs >/dev/null 2>&1
> >> +_scratch_mount "-o compress-force=lzo"
> >> +
> >> +run_check $XFS_IO_PROG -f -c "truncate 118811" $SCRATCH_MNT/foo
> >> +run_check $XFS_IO_PROG -c "pwrite -S 0x0d -b 39987 92267 39987" \
> >> +     $SCRATCH_MNT/foo
> >
> > Ugh. filter the output, don't use run_check.
> >
> > $XFS_IO_PROG -f -c "truncate 118811" $SCRATCH_MNT/foo
> > $XFS_IO_PROG -c "pwrite -S 0x0d -b 39987 92267 39987" \
> >         $SCRATCH_MNT/foo | _filter_xfs_io
> >
> > If something fails, we still want the test to continue running, even
> > if all it does is exercise error handling paths. run_check simply
> > terminates the test at the first failure.
> 
> What's the point of continuing? The test will fail anyway, all of the
> xfs_io calls are necessary to trigger the bug.

Users don't stop doing doing stuff on a filesystem when a single
failure occurs, so why should the tests? If you stop the moment a
single failure occurs then you aren't ever going to stress the error
handling paths, are you?

> > _run_btrfs_util_prog()
> > {
> >         run_check $BTRFS_UTIL_PROG $*
> > }
> >
> > would be a good start because it gets that run_check pattern out of
> > the main test scripts and hence out of the heads of test writers.
> 
> Well, will get rid of those run_check calls, but that will imply
> adding some | _filter_scratch in many places. So shortening lines is
> not a great argument :)

I'm not talking about shortening lines here. I'm talking about the
correct principles and conceptsi being in the forefront of a test
writer's mind. Having the concept of "need to filter the output" in
the head of test writers is *exactly* the right mindset to have.

Indeed, if you have a block of code that needs common filtering,
that's easy to do:

do_test()
{
	# put test in function
}

do_test | _filter_scratch

Will apply that filter to the entire output of the test, and so
you don't need it on every command.

Remember - an xfstest is not a "pass/fail" test. It's a "run this
set of commands, and then check the entire output matches the known
good output" test. i.e. we are testing the entire set of commands as
a whole - we are not testing each individual command that is run.
It's a very different principle to the "test every command that can
fail" method of writing tests.

_fail should only be used if the test cannot possibly be continued
(e.g. scratch filesystem corrupted and cannot be mounted).  If one
of the early commands fails, then that's fine - the test will fail -
but we still want to run the other commands if we can so as to get
the best test coverage we can get even on failed tests.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v3] xfstests: add regression test for btrfs incremental send

[Dave Chinner]

On Mon, Feb 17, 2014 at 12:20:38AM +0000, Filipe David Borba Manana wrote:
> Test for a btrfs incremental send issue where we end up sending a
> wrong section of data from a file extent if the corresponding file
> extent is compressed and the respective file extent item has a non
> zero data offset.
> 
> Fixed by the following linux kernel btrfs patch:
> 
>    Btrfs: use right clone root offset for compressed extents
> 
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> ---
> 
> V2: Made the test more reliable. Now it doesn't depend anymore of btrfs'
>     hole punch implementation leaving hole file extent items when we punch
>     beyond the file's current size.
> V3: Filter xfs_io output and make less use of the run_check function, as
>     suggested by Dave Chinner.

Awesome. Thanks for the quick turn around.

>  common/rc           |    5 +++
>  tests/btrfs/040     |  119 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/040.out |    9 ++++
>  tests/btrfs/group   |    1 +
>  4 files changed, 134 insertions(+)
>  create mode 100755 tests/btrfs/040
>  create mode 100644 tests/btrfs/040.out
> 
> diff --git a/common/rc b/common/rc
> index e91568b..27be009 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2207,6 +2207,11 @@ run_check()
>  	"$@" >> $seqres.full 2>&1 || _fail "failed: '$@'"
>  }
>  
> +_run_btrfs_util_prog()
> +{
> +	run_check $BTRFS_UTIL_PROG $*
> +}

Can you do a cleanup of all the other btrfs tests that can use this?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v2] xfstests: add regression test for btrfs incremental send

[Filipe David Manana]

On Mon, Feb 17, 2014 at 1:17 AM, Dave Chinner <david@fromorbit.com> wrote:
> On Sun, Feb 16, 2014 at 11:43:17PM +0000, Filipe David Manana wrote:
>> On Sun, Feb 16, 2014 at 11:08 PM, Dave Chinner <david@fromorbit.com> wrote:
>> > On Sat, Feb 15, 2014 at 03:36:13PM +0000, Filipe David Borba Manana wrote:
>> >> Test for a btrfs incremental send issue where we end up sending a
>> >> wrong section of data from a file extent if the corresponding file
>> >> extent is compressed and the respective file extent item has a non
>> >> zero data offset.
>> >>
>> >> Fixed by the following linux kernel btrfs patch:
>> >>
>> >>    Btrfs: use right clone root offset for compressed extents
>> >>
>> >> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>> >> ---
>> >>
>> >> V2: Made the test more reliable. Now it doesn't depend anymore of btrfs'
>> >>     hole punch implementation leaving hole file extent items when we punch
>> >>     beyond the file's current size.
>> >>
>> >>  tests/btrfs/040     |  115 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  tests/btrfs/040.out |    1 +
>> >>  tests/btrfs/group   |    1 +
>> >>  3 files changed, 117 insertions(+)
>> >>  create mode 100755 tests/btrfs/040
>> >>  create mode 100644 tests/btrfs/040.out
>> >>
>> >> diff --git a/tests/btrfs/040 b/tests/btrfs/040
>> >> new file mode 100755
>> >> index 0000000..d6b37bf
>> >> --- /dev/null
>> >> +++ b/tests/btrfs/040
>> >> @@ -0,0 +1,115 @@
>> >> +#! /bin/bash
>> >> +# FS QA Test No. btrfs/040
>> >> +#
>> >> +# Test for a btrfs incremental send issue where we end up sending a
>> >> +# wrong section of data from a file extent if the corresponding file
>> >> +# extent is compressed and the respective file extent item has a non
>> >> +# zero data offset.
>> >> +#
>> >> +# Fixed by the following linux kernel btrfs patch:
>> >> +#
>> >> +#   Btrfs: use right clone root offset for compressed extents
>> >> +#
>> >> +#-----------------------------------------------------------------------
>> >> +# Copyright (c) 2014 Filipe Manana.  All Rights Reserved.
>> >> +#
>> >> +# This program is free software; you can redistribute it and/or
>> >> +# modify it under the terms of the GNU General Public License as
>> >> +# published by the Free Software Foundation.
>> >> +#
>> >> +# This program is distributed in the hope that it would be useful,
>> >> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> >> +# GNU General Public License for more details.
>> >> +#
>> >> +# You should have received a copy of the GNU General Public License
>> >> +# along with this program; if not, write the Free Software Foundation,
>> >> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>> >> +#-----------------------------------------------------------------------
>> >> +#
>> >> +
>> >> +seq=`basename $0`
>> >> +seqres=$RESULT_DIR/$seq
>> >> +echo "QA output created by $seq"
>> >> +
>> >> +here=`pwd`
>> >> +tmp=`mktemp -d`
>> >> +status=1     # failure is the default!
>> >> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> >> +
>> >> +_cleanup()
>> >> +{
>> >> +    rm -fr $tmp
>> >> +}
>> >> +
>> >> +# get standard environment, filters and checks
>> >> +. ./common/rc
>> >> +. ./common/filter
>> >> +
>> >> +# real QA test starts here
>> >> +_supported_fs btrfs
>> >> +_supported_os Linux
>> >> +_require_scratch
>> >> +_need_to_be_root
>> >> +
>> >> +FSSUM_PROG=$here/src/fssum
>> >> +[ -x $FSSUM_PROG ] || _notrun "fssum not built"
>> >> +
>> >> +rm -f $seqres.full
>> >> +
>> >> +_scratch_mkfs >/dev/null 2>&1
>> >> +_scratch_mount "-o compress-force=lzo"
>> >> +
>> >> +run_check $XFS_IO_PROG -f -c "truncate 118811" $SCRATCH_MNT/foo
>> >> +run_check $XFS_IO_PROG -c "pwrite -S 0x0d -b 39987 92267 39987" \
>> >> +     $SCRATCH_MNT/foo
>> >
>> > Ugh. filter the output, don't use run_check.
>> >
>> > $XFS_IO_PROG -f -c "truncate 118811" $SCRATCH_MNT/foo
>> > $XFS_IO_PROG -c "pwrite -S 0x0d -b 39987 92267 39987" \
>> >         $SCRATCH_MNT/foo | _filter_xfs_io
>> >
>> > If something fails, we still want the test to continue running, even
>> > if all it does is exercise error handling paths. run_check simply
>> > terminates the test at the first failure.
>>
>> What's the point of continuing? The test will fail anyway, all of the
>> xfs_io calls are necessary to trigger the bug.
>
> Users don't stop doing doing stuff on a filesystem when a single
> failure occurs, so why should the tests? If you stop the moment a
> single failure occurs then you aren't ever going to stress the error
> handling paths, are you?
>
>> > _run_btrfs_util_prog()
>> > {
>> >         run_check $BTRFS_UTIL_PROG $*
>> > }
>> >
>> > would be a good start because it gets that run_check pattern out of
>> > the main test scripts and hence out of the heads of test writers.
>>
>> Well, will get rid of those run_check calls, but that will imply
>> adding some | _filter_scratch in many places. So shortening lines is
>> not a great argument :)
>
> I'm not talking about shortening lines here. I'm talking about the
> correct principles and conceptsi being in the forefront of a test
> writer's mind. Having the concept of "need to filter the output" in
> the head of test writers is *exactly* the right mindset to have.
>
> Indeed, if you have a block of code that needs common filtering,
> that's easy to do:
>
> do_test()
> {
>         # put test in function
> }
>
> do_test | _filter_scratch
>
> Will apply that filter to the entire output of the test, and so
> you don't need it on every command.
>
> Remember - an xfstest is not a "pass/fail" test. It's a "run this
> set of commands, and then check the entire output matches the known
> good output" test. i.e. we are testing the entire set of commands as
> a whole - we are not testing each individual command that is run.
> It's a very different principle to the "test every command that can
> fail" method of writing tests.
>
> _fail should only be used if the test cannot possibly be continued
> (e.g. scratch filesystem corrupted and cannot be mounted).  If one
> of the early commands fails, then that's fine - the test will fail -
> but we still want to run the other commands if we can so as to get
> the best test coverage we can get even on failed tests.

Ok, thanks for the very detailed explanation, very helpful :)


>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

Re: [PATCH v3] xfstests: add regression test for btrfs incremental send

[Filipe David Manana]

On Mon, Feb 17, 2014 at 1:19 AM, Dave Chinner <david@fromorbit.com> wrote:
> On Mon, Feb 17, 2014 at 12:20:38AM +0000, Filipe David Borba Manana wrote:
>> Test for a btrfs incremental send issue where we end up sending a
>> wrong section of data from a file extent if the corresponding file
>> extent is compressed and the respective file extent item has a non
>> zero data offset.
>>
>> Fixed by the following linux kernel btrfs patch:
>>
>>    Btrfs: use right clone root offset for compressed extents
>>
>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>> ---
>>
>> V2: Made the test more reliable. Now it doesn't depend anymore of btrfs'
>>     hole punch implementation leaving hole file extent items when we punch
>>     beyond the file's current size.
>> V3: Filter xfs_io output and make less use of the run_check function, as
>>     suggested by Dave Chinner.
>
> Awesome. Thanks for the quick turn around.
>
>>  common/rc           |    5 +++
>>  tests/btrfs/040     |  119 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/btrfs/040.out |    9 ++++
>>  tests/btrfs/group   |    1 +
>>  4 files changed, 134 insertions(+)
>>  create mode 100755 tests/btrfs/040
>>  create mode 100644 tests/btrfs/040.out
>>
>> diff --git a/common/rc b/common/rc
>> index e91568b..27be009 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -2207,6 +2207,11 @@ run_check()
>>       "$@" >> $seqres.full 2>&1 || _fail "failed: '$@'"
>>  }
>>
>> +_run_btrfs_util_prog()
>> +{
>> +     run_check $BTRFS_UTIL_PROG $*
>> +}
>
> Can you do a cleanup of all the other btrfs tests that can use this?

I just did that for all the non-merged test cases (4 including this
one), as you probably have noticed already by now.
I'll see if I can do the same for the ones already in the git repository soon.

thanks

>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

Re: [PATCH] xfstests: add regression test for btrfs incremental send

[Dave Chinner]

On Mon, Feb 17, 2014 at 01:36:02AM +0000, Filipe David Manana wrote:
> On Monday, February 17, 2014, Dave Chinner <david@fromorbit.com> wrote:
> 
> > On Mon, Feb 17, 2014 at 12:20:38AM +0000, Filipe David Borba Manana wrote:
> > > Test for a btrfs incremental send issue where we end up sending a
> > > wrong section of data from a file extent if the corresponding file
> > > extent is compressed and the respective file extent item has a non
> > > zero data offset.
> > >
> > > Fixed by the following linux kernel btrfs patch:
> > >
> > >    Btrfs: use right clone root offset for compressed extents
> > >
> > > Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com<javascript:;>
> > >
> > > ---
> > >
> > > V2: Made the test more reliable. Now it doesn't depend anymore of btrfs'
> > >     hole punch implementation leaving hole file extent items when we
> > punch
> > >     beyond the file's current size.
> > > V3: Filter xfs_io output and make less use of the run_check function, as
> > >     suggested by Dave Chinner.
> >
> > Awesome. Thanks for the quick turn around.
> >
> > >  common/rc           |    5 +++
> > >  tests/btrfs/040     |  119
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/btrfs/040.out |    9 ++++
> > >  tests/btrfs/group   |    1 +
> > >  4 files changed, 134 insertions(+)
> > >  create mode 100755 tests/btrfs/040
> > >  create mode 100644 tests/btrfs/040.out
> > >
> > > diff --git a/common/rc b/common/rc
> > > index e91568b..27be009 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -2207,6 +2207,11 @@ run_check()
> > >       "$@" >> $seqres.full 2>&1 || _fail "failed: '$@'"
> > >  }
> > >
> > > +_run_btrfs_util_prog()
> > > +{
> > > +     run_check $BTRFS_UTIL_PROG $*
> > > +}
> >
> > Can you do a cleanup of all the other btrfs tests that can use this?
> 
> 
> Ok. I just did that for all the test cases not yet merged, as you probably
> noticed already.

Yes, I did ;)

> For the ones already in the repository, I'll see if I can do it soon this
> coming week.

Great! Thanks for doing this.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com