[PATCH] btfs-progs: fsck-tests: corrupt nlink value test

[PATCH] btfs-progs: fsck-tests: corrupt nlink value test

[Lakshmipathi.G]

Signed-off-by: Lakshmipathi.G <Lakshmipathi.G@giis.co.in>
---
 tests/fsck-tests/025-wrong-inode-nlink/test.sh | 37 ++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100755 tests/fsck-tests/025-wrong-inode-nlink/test.sh

diff --git a/tests/fsck-tests/025-wrong-inode-nlink/test.sh b/tests/fsck-tests/025-wrong-inode-nlink/test.sh
new file mode 100755
index 0000000..15f1b84
--- /dev/null
+++ b/tests/fsck-tests/025-wrong-inode-nlink/test.sh
@@ -0,0 +1,37 @@
+#!/bin/bash
+
+source $TOP/tests/common
+
+check_prereq btrfs-corrupt-block
+check_prereq mkfs.btrfs
+check_prereq btrfs
+
+setup_root_helper
+prepare_test_dev 512M
+
+# test whether fsck can fix a corrupted inode nlink
+test_inode_nlink_field()
+{
+	run_check $SUDO_HELPER $TOP/mkfs.btrfs -f $TEST_DEV
+
+	run_check_mount_test_dev
+	run_check $SUDO_HELPER touch $TEST_MNT/test_nlink.txt
+
+	run_check_umount_test_dev
+
+	# find inode_item id
+	inode_item=`$SUDO_HELPER $TOP/btrfs-debug-tree -t FS_TREE $TEST_DEV | \
+	grep -B3 "test_nlink.txt" |  grep INODE_ITEM | \
+	cut -f2 -d'(' | cut -f1 -d' ' | head -n1`
+
+	# corrupt nlink field of inode object:257
+        run_check $SUDO_HELPER $TOP/btrfs-corrupt-block -i $inode_item \
+		-f nlink $TEST_DEV
+
+	$SUDO_HELPER $TOP/btrfs check $TEST_DEV >& /dev/null && \
+			_fail "btrfs check failed to detect nlink corruption"
+	run_check $SUDO_HELPER $TOP/btrfs check --repair $TEST_DEV
+	run_check $SUDO_HELPER $TOP/btrfs check $TEST_DEV
+}
+
+test_inode_nlink_field
-- 
2.7.4

Re: [PATCH] btfs-progs: fsck-tests: corrupt nlink value test

[lakshmipathi.g]

> What about submitting a btrfs-image and use generic test load?

Okay, how to share the corrupted btrfs-image? using github?  And also do 
you have references for this kind of
setup under btrfs-progs/tests/?  So that  I can follow its model.

Cheers.
Lakshmipathi.G

Re: [PATCH] btfs-progs: fsck-tests: corrupt nlink value test

[Qu Wenruo]

At 01/11/2017 01:36 AM, lakshmipathi.g@giis.co.in wrote:
>> What about submitting a btrfs-image and use generic test load?
>
> Okay, how to share the corrupted btrfs-image? using github?  And also do
> you have references for this kind of
> setup under btrfs-progs/tests/?  So that  I can follow its model.

At least I follow the following steps to create image:

- Create a 256M file
   The default size of image.
   All zero content is recommended for high compression ratio.
   Both hole or fallocate is OK.

- Make btrfs on that file

- Fill the fs with minimum content
   Just enough files/dirs for next step.
   The smaller space it takes the better

- Corrupt the fs
   Either using btrfs-corrupt-block or manually

- Take the dump
   Either by btrfs-image or just use the raw image.

   It's recommended to use btrfs-image and with -c9 option, which
   can reduce the file size dramatically compared to xz raw image.
   But we must ensure the recovered image still has the same
   corruption, or we must use raw image.

   For raw image, xz it just like other tests.

   Normally submitted images are less than 100K, and that's small enough
   to send as patch.

Thanks,
Qu

>
> Cheers.
> Lakshmipathi.G
>
>
>

Re: [PATCH] btfs-progs: fsck-tests: corrupt nlink value test

[lakshmipathi.g]

Hi Qu,

Thanks for the details, I'll create image using these steps. Never used 
btrfs-image before will try it out.  in the mean time: I was thinking 
about these two approaches:

--
I agree on the dependency issue with btrfs-debug-tree. Test script will 
break if output is changed or jumbled.
If btrfs-debug-tree is the only issue, possible solution can be :

(a) when btrfs-debug-tree breaks the scripts add a backward captiable 
option (--print-old-format) for outputs. (or)
(b) Use 'ls -i' to find the inode object id?  I guess that will be more 
reliable than parsing btrfs-debug-tree output.  (or)
(c) may be use hard-corded id, if i'm not wrong, first object id will be 
always 256+1 = 257

I slightly prefer script over image because:
* It might be more easier to have script to add new functionality (ex: 
corrupt nlink field from hardlink)
* It allows option to inject corruption sequentitally.
ex: clean image->injection corrupt-x -> run btrfsck -> fixed image -> 
inject corruption 'variant of' x  -> repeat the process.
* It might also be more easier to deal with possible RAID-based 
corruption test scripts.
--

To my (QA) eyes, creating new image from 'mkfs.btrfs' for a each 
btrfs-progs version, injecting corruption (using ls -i output) and then 
verifying looks better. This way we can even catch any (rare) changes 
made to mkfs.btrfs. Script makes btrfsck heavily dependent on 
'mkfs.btrfs' which is a good thing,imo.

Let me know your thoughts. thanks!

Cheers.
Lakshmipathi.G

Re: [PATCH] btfs-progs: fsck-tests: corrupt nlink value test

[Qu Wenruo]

At 01/12/2017 10:37 PM, lakshmipathi.g@giis.co.in wrote:
> Hi Qu,
>
> Thanks for the details, I'll create image using these steps. Never used
> btrfs-image before will try it out.  in the mean time: I was thinking
> about these two approaches:
>
> --
> I agree on the dependency issue with btrfs-debug-tree. Test script will
> break if output is changed or jumbled.
> If btrfs-debug-tree is the only issue, possible solution can be :
>
> (a) when btrfs-debug-tree breaks the scripts add a backward captiable
> option (--print-old-format) for outputs. (or)
> (b) Use 'ls -i' to find the inode object id?  I guess that will be more
> reliable than parsing btrfs-debug-tree output.  (or)
> (c) may be use hard-corded id, if i'm not wrong, first object id will be
> always 256+1 = 257
>
> I slightly prefer script over image because:
> * It might be more easier to have script to add new functionality (ex:
> corrupt nlink field from hardlink)

Right, but btrfs-corrupt-block is no longer default installed and its 
options are badly arranged, with little documentation.

Quite a lot of its code is old and its output is quite hard to catch.

In short, it's just in bad shape.

> * It allows option to inject corruption sequentitally.
> ex: clean image->injection corrupt-x -> run btrfsck -> fixed image ->
> inject corruption 'variant of' x  -> repeat the process.
> * It might also be more easier to deal with possible RAID-based
> corruption test scripts.

Yes, RAID test indeed needs btrfs-corrupt-block support, just as fstests 
guys pointed out, but we really need to refact it before introducing new 
features.

Before we have a really good btrfs-corrupt-block, current image method 
seems to be best yet.

If btrfs-corrupt-block has the following feature, then I'm OK to convert 
to script based tests:

1) Able to corrupt things CoW or noCoW
    Only part of corruptions are noCow now.
    NoCow corruption is especially important for shared node/leaf case.

2) Ability to corrupt node/leaf header (Already supported)

3) Ability to corrupt data structure pinpointly
    This means, we should be able to corrupt given fields of specified
    data structures, at least covers the current fsck test cases.

    I see patches enhancing them, but 5) still makes them quite hard to
    use.

4) Better error handling and output.

5) Documentation
    No document for current btrfs-currupt-block, and this makes the
    options of it quite chaos and hard to use.
    (So normally I just hard code it to create test images)

But current one is far from those objectives, which makes it unpredictable.

And that's why we are still using binary images, because it's reliable 
and makes error always reproducible.

> --
>
> To my (QA) eyes, creating new image from 'mkfs.btrfs' for a each
> btrfs-progs version, injecting corruption (using ls -i output) and then
> verifying looks better. This way we can even catch any (rare) changes
> made to mkfs.btrfs. Script makes btrfsck heavily dependent on
> 'mkfs.btrfs' which is a good thing,imo.

IIRC current fsck-tests acts more or less like regression or corner-case 
tests.

And in this case, btrfsck dependent on mkfs.btrfs(or 
btrfs-corrupt-block) behavior makes us hard to trigger corner-case or 
regression which only happens on certain cases.

Thanks,
Qu

>
> Let me know your thoughts. thanks!
>
> Cheers.
> Lakshmipathi.G
>
>

Re: [PATCH] btfs-progs: fsck-tests: corrupt nlink value test

[lakshmipathi.g]

If btrfs-corrupt-block is in bad shape, then corruption scripts around 
them won't help in long term.

Yes, documentation for btrfs-corrupt-block needs improvement.  imo, 
re-arranged priority will be like : (5), (1)/(3) then (4).  Agree that 
some corner cases, having static image is best option, i think the 
corruption test-case needs to be mixture of both static-images and 
scripts.

For the time being, I'll create static-images to cover corruption cases. 
thanks.

Cheers,
Lakshmipathi.G

Re: [PATCH] btfs-progs: fsck-tests: corrupt nlink value test

[David Sterba]

On Mon, Jan 16, 2017 at 09:35:52AM -0700, lakshmipathi.g@giis.co.in wrote:
> If btrfs-corrupt-block is in bad shape, then corruption scripts around 
> them won't help in long term.
> 
> Yes, documentation for btrfs-corrupt-block needs improvement.  imo, 
> re-arranged priority will be like : (5), (1)/(3) then (4).  Agree that 
> some corner cases, having static image is best option, i think the 
> corruption test-case needs to be mixture of both static-images and 
> scripts.

Both approaches have their pros and cons so I'll accept both. The
functionality provided by the corrupt block utility can be used, any
changes to the command line UI will be also applied to the test scripts.

Re: [PATCH] btfs-progs: fsck-tests: corrupt nlink value test

[Lakshmipathi.G]

> Both approaches have their pros and cons so I'll accept both. The
> functionality provided by the corrupt block utility can be used, any
> changes to the command line UI will be also applied to the test scripts.

okay, I'll continue mixing both approaches.