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
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
--
1.7.7.6
--
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
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
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
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
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.imgHuh, 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
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