diff mbox

Btrfs-progs: add make test framework

Message ID 1378493456-5382-1-git-send-email-jbacik@fusionio.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Josef Bacik Sept. 6, 2013, 6:50 p.m. UTC
We need to start adding some sanity tests to btrfs-progs to make sure we aren't
breaking things with our patches.  The most important of these tools is btrfsck.
This patch gets things started by adding a basic btrfsck test that makes sure we
can fix a corruption problem we know we can fix.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fusionio.com>
---
 Makefile                         |    7 +++++++
 tests/bad-file-extent-bytenr.img |  Bin 0 -> 4096 bytes
 tests/fsck-tests.sh              |   34 ++++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 0 deletions(-)
 create mode 100644 tests/bad-file-extent-bytenr.img
 create mode 100644 tests/fsck-tests.sh

Comments

David Sterba Sept. 9, 2013, 4:32 p.m. UTC | #1
On Fri, Sep 06, 2013 at 02:50:56PM -0400, Josef Bacik wrote:
> We need to start adding some sanity tests to btrfs-progs to make sure we aren't
> breaking things with our patches.  The most important of these tools is btrfsck.
> This patch gets things started by adding a basic btrfsck test that makes sure we
> can fix a corruption problem we know we can fix.  Thanks,

That's great. I hope we're going to gather more tests so let's separate
them to more categories from the beginning (mkfs, options coverage,
corrupt-block, restore maybe, dunno what else).

This makes it possible to do a quick run a subset of the tests (eg. mkfs
related) after a fix is committed.

>  tests/bad-file-extent-bytenr.img |  Bin 0 -> 4096 bytes
>  tests/fsck-tests.sh              |   34 ++++++++++++++++++++++++++++++++++

I suggest to put them into a subdirectory (eg.) 'fsck' and prefix the
images with numbers (just a sequence number).

> --- /dev/null
> +++ b/tests/fsck-tests.sh
> @@ -0,0 +1,34 @@
> +#!/bin/bash
> +#
> +# loop through all of our bad images and make sure fsck repairs them properly
> +#
> +# It's GPL, same as everything else in this tree.
> +#
> +
> +TEST_DEV=""

I found the current test hard to use, eg. I can't just do 'make test'
from the toplevel dir. The script fsck-tests.sh expects TEST_DEV to be
set, but there's no way to do that, because it's set unconditionally to
empty string.

> +here=`pwd`
> +
> +_fail()
> +{
> +	echo "$*" | tee -a fsck-tests-results.txt
> +	exit 1
> +}
> +
> +[ -z $TEST_DEV ] && echo "Cannot run without a test device set" && exit 0
> +
> +rm -f fsck-tests-results.txt
> +
> +for i in $(find tests -name '*.img')
> +do
> +	echo "testing image $i" >> fsck-tests-results.txt
> +	$here/btrfs-image -r $i $TEST_DEV >> fsck-tests-results.txt 2>&1 \
> +		|| _fail "restore failed"
> +	$here/btrfsck $TEST_DEV >> fsck-test-results.txt 2>&1
> +	[ $? -eq 0 ] && _fail "btrfsck should have detected corruption"
> +
> +	$here/btrfsck --repair $TEST_DEV >> fsck-test-results.txt 2>&1 || \
> +		_fail "btrfsck should have repaired the image"
> +
> +	$here/btrfsck $TEST_DEV >> fsck-test-results.txt 2>&1 || \
> +		_fail "btrfsck did not correct corruption"
> +done
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josef Bacik Sept. 9, 2013, 5:13 p.m. UTC | #2
On Mon, Sep 09, 2013 at 06:32:04PM +0200, David Sterba wrote:
> On Fri, Sep 06, 2013 at 02:50:56PM -0400, Josef Bacik wrote:
> > We need to start adding some sanity tests to btrfs-progs to make sure we aren't
> > breaking things with our patches.  The most important of these tools is btrfsck.
> > This patch gets things started by adding a basic btrfsck test that makes sure we
> > can fix a corruption problem we know we can fix.  Thanks,
> 
> That's great. I hope we're going to gather more tests so let's separate
> them to more categories from the beginning (mkfs, options coverage,
> corrupt-block, restore maybe, dunno what else).
> 
> This makes it possible to do a quick run a subset of the tests (eg. mkfs
> related) after a fix is committed.
> 
> >  tests/bad-file-extent-bytenr.img |  Bin 0 -> 4096 bytes
> >  tests/fsck-tests.sh              |   34 ++++++++++++++++++++++++++++++++++
> 
> I suggest to put them into a subdirectory (eg.) 'fsck' and prefix the
> images with numbers (just a sequence number).

I'm not prefixing with numbers, I want to make it easy to tell what each image
is testing for.  I will move them into a fsck directory tho.

> 
> > --- /dev/null
> > +++ b/tests/fsck-tests.sh
> > @@ -0,0 +1,34 @@
> > +#!/bin/bash
> > +#
> > +# loop through all of our bad images and make sure fsck repairs them properly
> > +#
> > +# It's GPL, same as everything else in this tree.
> > +#
> > +
> > +TEST_DEV=""
> 
> I found the current test hard to use, eg. I can't just do 'make test'
> from the toplevel dir. The script fsck-tests.sh expects TEST_DEV to be
> set, but there's no way to do that, because it's set unconditionally to
> empty string.

Yeah I wasn't sure how I wanted to do this.  At first I thought about making the
fsck tester just make a loop device, but I didn't want to override something if
people were already using a loop device.  But maybe I'll just default to using
loop5 or something big like that and then if the user wants to change it they
can go into the script and change it themselves.  How does that sound?  Thanks,

Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sandeen Sept. 9, 2013, 7:07 p.m. UTC | #3
On 9/9/13 12:13 PM, Josef Bacik wrote:
> On Mon, Sep 09, 2013 at 06:32:04PM +0200, David Sterba wrote:
>> On Fri, Sep 06, 2013 at 02:50:56PM -0400, Josef Bacik wrote:
>>> We need to start adding some sanity tests to btrfs-progs to make sure we aren't
>>> breaking things with our patches.  The most important of these tools is btrfsck.
>>> This patch gets things started by adding a basic btrfsck test that makes sure we
>>> can fix a corruption problem we know we can fix.  Thanks,
>>
>> That's great. I hope we're going to gather more tests so let's separate
>> them to more categories from the beginning (mkfs, options coverage,
>> corrupt-block, restore maybe, dunno what else).
>>
>> This makes it possible to do a quick run a subset of the tests (eg. mkfs
>> related) after a fix is committed.
>>
>>>  tests/bad-file-extent-bytenr.img |  Bin 0 -> 4096 bytes
>>>  tests/fsck-tests.sh              |   34 ++++++++++++++++++++++++++++++++++
>>
>> I suggest to put them into a subdirectory (eg.) 'fsck' and prefix the
>> images with numbers (just a sequence number).
> 
> I'm not prefixing with numbers, I want to make it easy to tell what each image
> is testing for.  I will move them into a fsck directory tho.

David might have meant "001-bad-file-extent-bytenr.img" though.
 
>>
>>> --- /dev/null
>>> +++ b/tests/fsck-tests.sh
>>> @@ -0,0 +1,34 @@
>>> +#!/bin/bash
>>> +#
>>> +# loop through all of our bad images and make sure fsck repairs them properly
>>> +#
>>> +# It's GPL, same as everything else in this tree.
>>> +#
>>> +
>>> +TEST_DEV=""
>>
>> I found the current test hard to use, eg. I can't just do 'make test'
>> from the toplevel dir. The script fsck-tests.sh expects TEST_DEV to be
>> set, but there's no way to do that, because it's set unconditionally to
>> empty string.
> 
> Yeah I wasn't sure how I wanted to do this.  At first I thought about making the
> fsck tester just make a loop device, but I didn't want to override something if
> people were already using a loop device.  But maybe I'll just default to using
> loop5 or something big like that and then if the user wants to change it they
> can go into the script and change it themselves.  How does that sound?  Thanks,

Dumb question, can you just point btrfsck at the image itself w/o setting up
loopback?  i.e. do something like: 

# btrfs-image -r 001-bad-file-extent-bytenr.img test.img
# btrfsck --repair test.img

-Eric

> Josef
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josef Bacik Sept. 9, 2013, 7:57 p.m. UTC | #4
On Mon, Sep 09, 2013 at 02:07:43PM -0500, Eric Sandeen wrote:
> On 9/9/13 12:13 PM, Josef Bacik wrote:
> > On Mon, Sep 09, 2013 at 06:32:04PM +0200, David Sterba wrote:
> >> On Fri, Sep 06, 2013 at 02:50:56PM -0400, Josef Bacik wrote:
> >>> We need to start adding some sanity tests to btrfs-progs to make sure we aren't
> >>> breaking things with our patches.  The most important of these tools is btrfsck.
> >>> This patch gets things started by adding a basic btrfsck test that makes sure we
> >>> can fix a corruption problem we know we can fix.  Thanks,
> >>
> >> That's great. I hope we're going to gather more tests so let's separate
> >> them to more categories from the beginning (mkfs, options coverage,
> >> corrupt-block, restore maybe, dunno what else).
> >>
> >> This makes it possible to do a quick run a subset of the tests (eg. mkfs
> >> related) after a fix is committed.
> >>
> >>>  tests/bad-file-extent-bytenr.img |  Bin 0 -> 4096 bytes
> >>>  tests/fsck-tests.sh              |   34 ++++++++++++++++++++++++++++++++++
> >>
> >> I suggest to put them into a subdirectory (eg.) 'fsck' and prefix the
> >> images with numbers (just a sequence number).
> > 
> > I'm not prefixing with numbers, I want to make it easy to tell what each image
> > is testing for.  I will move them into a fsck directory tho.
> 
> David might have meant "001-bad-file-extent-bytenr.img" though.

Oh yeah that may be good then.
>  
> >>
> >>> --- /dev/null
> >>> +++ b/tests/fsck-tests.sh
> >>> @@ -0,0 +1,34 @@
> >>> +#!/bin/bash
> >>> +#
> >>> +# loop through all of our bad images and make sure fsck repairs them properly
> >>> +#
> >>> +# It's GPL, same as everything else in this tree.
> >>> +#
> >>> +
> >>> +TEST_DEV=""
> >>
> >> I found the current test hard to use, eg. I can't just do 'make test'
> >> from the toplevel dir. The script fsck-tests.sh expects TEST_DEV to be
> >> set, but there's no way to do that, because it's set unconditionally to
> >> empty string.
> > 
> > Yeah I wasn't sure how I wanted to do this.  At first I thought about making the
> > fsck tester just make a loop device, but I didn't want to override something if
> > people were already using a loop device.  But maybe I'll just default to using
> > loop5 or something big like that and then if the user wants to change it they
> > can go into the script and change it themselves.  How does that sound?  Thanks,
> 
> Dumb question, can you just point btrfsck at the image itself w/o setting up
> loopback?  i.e. do something like: 
> 
> # btrfs-image -r 001-bad-file-extent-bytenr.img test.img
> # btrfsck --repair test.img

Huh, I'm not sure...yes it looks like it, well that solves that problem.
Thanks,

Josef
> 
> -Eric
> 
> > Josef
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba Sept. 9, 2013, 11:31 p.m. UTC | #5
On Mon, Sep 09, 2013 at 03:57:17PM -0400, Josef Bacik wrote:
> On Mon, Sep 09, 2013 at 02:07:43PM -0500, Eric Sandeen wrote:
> > On 9/9/13 12:13 PM, Josef Bacik wrote:
> > David might have meant "001-bad-file-extent-bytenr.img" though.
> Oh yeah that may be good then.

Yes that's what I meant. I thought the number could be useful as a
shortcut additional to the human-readable part.

> > > Yeah I wasn't sure how I wanted to do this.  At first I thought about making the
> > > fsck tester just make a loop device, but I didn't want to override something if
> > > people were already using a loop device.  But maybe I'll just default to using
> > > loop5 or something big like that and then if the user wants to change it they
> > > can go into the script and change it themselves.  How does that sound?  Thanks,
> > 
> > Dumb question, can you just point btrfsck at the image itself w/o setting up
> > loopback?  i.e. do something like: 
> > 
> > # btrfs-image -r 001-bad-file-extent-bytenr.img test.img
> > # btrfsck --repair test.img
> 
> Huh, I'm not sure...yes it looks like it, well that solves that problem.
> Thanks,

If you need a blockdevice, you can use

$ losetup -f /the/file
(get a free loopdev)

$ losetup -j /the/file
/dev/loop2: ...

and parse the loop name from the output.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Makefile b/Makefile
index c43cb68..9fbe292 100644
--- a/Makefile
+++ b/Makefile
@@ -19,6 +19,7 @@  libbtrfs_headers = send-stream.h send-utils.h send.h rbtree.h btrfs-list.h \
 CHECKFLAGS= -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ -Wbitwise \
 	    -Wuninitialized -Wshadow -Wundef
 DEPFLAGS = -Wp,-MMD,$(@D)/.$(@F).d,-MT,$@
+TESTS = fsck-tests.sh
 
 INSTALL = install
 prefix ?= /usr/local
@@ -87,6 +88,12 @@  endif
 
 all: version.h $(progs) manpages
 
+test:
+	$(Q)for t in $(TESTS); do \
+		echo "     [TEST]    $$t"; \
+		bash tests/$$t || exit 1; \
+	done
+
 #
 # NOTE: For static compiles, you need to have all the required libs
 # 	static equivalent available
diff --git a/tests/bad-file-extent-bytenr.img b/tests/bad-file-extent-bytenr.img
new file mode 100644
index 0000000000000000000000000000000000000000..d2a05bb8f83d7f1bdc4be1441d3d8d35a89e142e
GIT binary patch
literal 4096
zcmeH}eK^y5AIJB=HZis#ld?QS9Mv4_Of8wM<gsInnB$m-GN*JVVaYO&p%sNVl8|UD
zhty$=hFxhUNzWdRrbKy4$Pwm9=f3MY*M0wcU+22+KG*O1d|$uM^}W8o-{<|u`};X{
zAnVA$h94RLe+6`T1+F~HfL<m5fVh(1e?)+2g=@ge=<<oLe=MheV!=lyHm&Soeq_MI
zmArqYvVvdJR|UQ*@Xt|z)jh=)R-FW@AtEWNxE}^T95kIuxTQO`GeSj$OAocZ8ZFG&
z<3eK!w-`IcHv_L`f(<;t>b+Xy{-7?nK7F$e6<y&8(6k1*npyVVPR;^XbD9d3T8KhU
z<dV1dX5Uut3yan!b4PzMp+~U;!K%0@F_3{e9)}18WH>QQb1IAX+fGlXZfMf;0!Vkj
zJvX_KWEOFo=%gw@5vC86umI%0Q@Kpn;Kos~t8AM#5}}Iev<%H_=M(YY>vRsP6|{X%
z%!njroW!5@!d|E;=0Fi)&zOQKQ0@W*x3i9*Rv@{&RVIlTf+h<_YkMV3gnNEEL9z3p
zoAlx;MA$Z_-~qsSxw3Ahviza+Re;u)K3iGc$~j&?N;%Nv`i;;v6-O4gW*uwxO^d(~
zAP}f+x~0LcT=VlI(lPD+zOh-}pmxLjOi`R-S(>&(wVug=T3~?ZI}5C#o@7U?*X+5p
zSUrGyGq61;#UvEa{#INBZWl_VA{R~xxV=5W{$IVJ6+btc4tYL~QqNO<y>PYQZLBAt
zC<$2z=O_MM)b$dAUq%p}K^Gmkgz}q5PxeG|&T7n~tDd$d>#M#tLmkvrwwYSX%o*Z5
z+F|Jnh^5>Opo?^LmBy7LW8t9;5*}(z#3LK5)c~TJ0DV^X7|ofXZljSOEI*iGOEL2>
zyms?y=YU$=G~M9wC9-C@A~MUPN5P+56asQb7^Kb=Q7Z>*cDq!<{D*BQ{L?YdZLE@^
zlS9Q&E9#b;iK&nKMun2`9=?4K)$NywboPAXv7djx@a$qA{=&uLHU~LJ5U#9qzTeZj
zJK&96z$-ri*Hbbb;gKF#C6XsLa}V*^F50xo@aeG9_-9^E@9%wht!YF}OK%@FS0B+R
zc$uMbpEWrt)AeGRAorUV2{?i}2`qJ<d41=`@p!ugx80fia_cF|>JNrvt2L1d`}P?6
zYa$V!<4#y{8v&m^d`7Fnl<yRCM#w6LyjS&gykUjdM(vX+LrC{K7*{u}eFr5qJ}zN6
zv-(f?Iu`R9Zew5{a`m^alfoYUWHg|5@>)`8aD9pCT(>!U7f!kml(o6f2y^P<Y&J(Y
zKk9v&KY2=7LM<>NT~YY<J5Dvo)hC1$!iT)zcN}1jiL0-eeRH{ezgM4pVZMA%tNT)Y
zb@`HC+o?w#s03C<bEv(~xlAEZe2%1JjH3AyjPng+j{7NSX^=zYYhYone++<{9W~?N
zb0I07Jbe}>+ea97ZMRO19VZd>M?@vlKv~U`g)kou6npII1s7=}eTYX5ijc4g@+Jx-
zf%ESm>VnD$gtGjkRt<5%s9vbfw^i%aJFSO0Uc6E<iW0B8Bj!3Y;Z6evW*#mw2gDg;
zS0jy?0W6IB<g{T24@~Pxb*h*KdNWNrYTu~{;^pvJO}uuyLEbDSetUiy)#Fg04FN^N
zRKwqOMVadiA7lThi%lz+&j#*WBJb$9;kIP&+F!^UwxqZhBhsUgDs9mzIlQ_ELE^72
zRiev#^0#MBEP0H+sUXj#0rT*UQ8*@Tb%(uJ|23n99bIX{w!~6`-C=!Zc{dnK&qYfP
zopegXhQ{jp#0e9#^RQz<ki!PQx7w(WkcsrXd+HpeE#bl4S9-$pOmjj3hwxmqI5<<w
z-1BA0oNy8++vZOm8x3|Xtpt~0)T(dW{d`C`sp}RZ%--vNW2ka$$g$;CTj|J4&&r}>
zu3DRFM{YHi4?g3j-fg&9^8xCi7J@kFeAusb)o(sKn}VyCj2+=IXV*A3q`ZApQt)V~
zUk7dri<xm|OS=40UwJKVklJ-jhNB;xAs?1?Dq?wqE#^&FzE{P=@jACD`KuX*!Dy~@
zc4pTsdbmRpKolO?yXolcsrgI2C+3p+A+?LctnQJUq0WIKu-GRP4Bsb^=eF&T{ko?<
zZ5>$z{O@KpF&_h?`x8Cj7BhGZxCLa0)J4Cr!X#UKG}<rC=+ldeo=99!+RAlw1!PH+
z#V1DW8ZUCYXf_E?@2od==}#z@$AIK=>u;!-A;xstUw-|!W8xtI0PL*!TUK{D%l!B{
zu<rWtqP2|LwwWi?S4uHf-Rx1c(OSAfiUp}yJ3!Fbn?Kptcj6K)|9-vi7E{l{51s`$
z=ygBq!a8*7oHiPVD8c5%@|X|@bb*5<N4q%(1r>2yqjR>4M(^gJ5T*El`%Mb|3IS)C
zAoI*Z)MtDBqP>lfb}WQHr8venClWZri%xBi2t+Tq$_+{kCwAAAJvFUN&Bv-rrV7~h
zCa8Q_YI`kO(|@`bPR4WBhuS9N0?eLd_gFO6_e{7+Euv;eb}K@u7(ynbeEvNKv<8hn
z!`U4a23A^~^yx<XLLs1!P3en!+Lu`xX#;0dfUw0#!m=YvtKiL81d-y6GjR(9*wW()
z4y9ABlN`RYlfaxf&7e`6?DMK7L3k&Q_;{D3F6PL5ba=L`VPOQvqfN=VRfD)JCSOJ)
wdAHD-c}bH<S;ReLS9EfZAgz)e^M5^=b9|?PO*Mu1f6h4e^-sPk@IO`HPtN?u?f?J)

literal 0
HcmV?d00001

diff --git a/tests/fsck-tests.sh b/tests/fsck-tests.sh
new file mode 100644
index 0000000..b164c4e
--- /dev/null
+++ b/tests/fsck-tests.sh
@@ -0,0 +1,34 @@ 
+#!/bin/bash
+#
+# loop through all of our bad images and make sure fsck repairs them properly
+#
+# It's GPL, same as everything else in this tree.
+#
+
+TEST_DEV=""
+here=`pwd`
+
+_fail()
+{
+	echo "$*" | tee -a fsck-tests-results.txt
+	exit 1
+}
+
+[ -z $TEST_DEV ] && echo "Cannot run without a test device set" && exit 0
+
+rm -f fsck-tests-results.txt
+
+for i in $(find tests -name '*.img')
+do
+	echo "testing image $i" >> fsck-tests-results.txt
+	$here/btrfs-image -r $i $TEST_DEV >> fsck-tests-results.txt 2>&1 \
+		|| _fail "restore failed"
+	$here/btrfsck $TEST_DEV >> fsck-test-results.txt 2>&1
+	[ $? -eq 0 ] && _fail "btrfsck should have detected corruption"
+
+	$here/btrfsck --repair $TEST_DEV >> fsck-test-results.txt 2>&1 || \
+		_fail "btrfsck should have repaired the image"
+
+	$here/btrfsck $TEST_DEV >> fsck-test-results.txt 2>&1 || \
+		_fail "btrfsck did not correct corruption"
+done