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

[Filipe David Manana]

On Monday, February 17, 2014, Dave Chinner <david at 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 at 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 at 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
> 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.


Thanks for the detailed explanation Dave, very useful :)



>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david at fromorbit.com <javascript:;>
>


-- 
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."
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://oss.sgi.com/pipermail/xfs/attachments/20140217/65fa1bf6/attachment-0001.html>