[PATCH 1/3] Btrfs-progs: fsck: finish transaction commit if repair error out

[PATCH 1/3] Btrfs-progs: fsck: finish transaction commit if repair error out

[Wang Shilong]

If btrfsck fail to repair, we hit something like following:

Check tree block failed, want=29442048, have=0
Check tree block failed, want=29442048, have=0
Check tree block failed, want=29442048, have=0
Check tree block failed, want=29442048, have=0
Check tree block failed, want=29442048, have=0
read block failed check_tree_block
found 98304 bytes used err is 1
total csum bytes: 0
total tree bytes: 0
total fs tree bytes: 0
total extent tree bytes: 0
btree space waste bytes: 0
file data blocks allocated: 0
referenced 0
Btrfs v3.14.2-rc2-63-g3944f15
btrfs: transaction.h:38: btrfs_start_transaction: Assertion `!(root->commit_root)' failed.
Aborted (core dumped)

This is because under repair mode, we will start a transaction, and if we error out,
we don't finish this transaction. So in close_ctree(), it will try
to start and commit transaction which causes the above segmentation.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-check.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds-check.c b/cmds-check.c
index 5bceb43..00d37b8 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -6334,12 +6334,12 @@ again:
 	if (err && !ret)
 		ret = err;
 
+out:
 	if (trans) {
 		err = btrfs_commit_transaction(trans, root);
 		if (!ret)
 			ret = err;
 	}
-out:
 	if (repair) {
 		free_corrupt_blocks_tree(root->fs_info->corrupt_blocks);
 		root->fs_info->fsck_extent_cache = NULL;
-- 
1.9.0

[PATCH 2/3] Btrfs-progs: fsck: remove unfriendly BUG_ON() for searching tree failure

[Wang Shilong]

Now btrfsck would hit assertation failure for some searching tree failure.
It is true that filesystem may get some metadata block corrupted,
and btrfsck could not deal with these corruptings. But, Users really
don't want a BUG_ON() here, Instead, just return errors to caller.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-check.c | 84 +++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 58 insertions(+), 26 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index 00d37b8..c56da2a 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -899,7 +899,8 @@ static int is_child_root(struct btrfs_root *root, u64 parent_root_id,
 	key.offset = child_root_id;
 	ret = btrfs_search_slot(NULL, root->fs_info->tree_root, &key, &path,
 				0, 0);
-	BUG_ON(ret < 0);
+	if (ret < 0)
+		return ret;
 	btrfs_release_path(&path);
 	if (!ret)
 		return 1;
@@ -909,15 +910,14 @@ static int is_child_root(struct btrfs_root *root, u64 parent_root_id,
 	key.offset = 0;
 	ret = btrfs_search_slot(NULL, root->fs_info->tree_root, &key, &path,
 				0, 0);
-	BUG_ON(ret <= 0);
+	if (ret < 0)
+		goto out;
 
 	while (1) {
 		leaf = path.nodes[0];
 		if (path.slots[0] >= btrfs_header_nritems(leaf)) {
 			ret = btrfs_next_leaf(root->fs_info->tree_root, &path);
-			BUG_ON(ret < 0);
-
-			if (ret > 0)
+			if (ret)
 				break;
 			leaf = path.nodes[0];
 		}
@@ -936,8 +936,10 @@ static int is_child_root(struct btrfs_root *root, u64 parent_root_id,
 
 		path.slots[0]++;
 	}
-
+out:
 	btrfs_release_path(&path);
+	if (ret < 0)
+		return ret;
 	return has_parent? 0 : -1;
 }
 
@@ -1090,14 +1092,15 @@ static int process_inode_extref(struct extent_buffer *eb,
 
 }
 
-static u64 count_csum_range(struct btrfs_root *root, u64 start, u64 len)
+static int count_csum_range(struct btrfs_root *root, u64 start,
+			    u64 len, u64 *found)
 {
 	struct btrfs_key key;
 	struct btrfs_path path;
 	struct extent_buffer *leaf;
-	int ret ;
+	int ret;
 	size_t size;
-	u64 found = 0;
+	*found = 0;
 	u64 csum_end;
 	u16 csum_size = btrfs_super_csum_size(root->fs_info->super_copy);
 
@@ -1109,7 +1112,8 @@ static u64 count_csum_range(struct btrfs_root *root, u64 start, u64 len)
 
 	ret = btrfs_search_slot(NULL, root->fs_info->csum_root,
 				&key, &path, 0, 0);
-	BUG_ON(ret < 0);
+	if (ret < 0)
+		goto out;
 	if (ret > 0 && path.slots[0] > 0) {
 		leaf = path.nodes[0];
 		btrfs_item_key_to_cpu(leaf, &key, path.slots[0] - 1);
@@ -1122,9 +1126,10 @@ static u64 count_csum_range(struct btrfs_root *root, u64 start, u64 len)
 		leaf = path.nodes[0];
 		if (path.slots[0] >= btrfs_header_nritems(leaf)) {
 			ret = btrfs_next_leaf(root->fs_info->csum_root, &path);
-			BUG_ON(ret < 0);
 			if (ret > 0)
 				break;
+			else if (ret < 0)
+				goto out;
 			leaf = path.nodes[0];
 		}
 
@@ -1146,13 +1151,16 @@ static u64 count_csum_range(struct btrfs_root *root, u64 start, u64 len)
 			size = min(csum_end - start, len);
 			len -= size;
 			start += size;
-			found += size;
+			*found += size;
 		}
 
 		path.slots[0]++;
 	}
+out:
+	if (ret < 0)
+		return ret;
 	btrfs_release_path(&path);
-	return found;
+	return 0;
 }
 
 static int process_file_extent(struct btrfs_root *root,
@@ -1167,6 +1175,7 @@ static int process_file_extent(struct btrfs_root *root,
 	u64 extent_offset = 0;
 	u64 mask = root->sectorsize - 1;
 	int extent_type;
+	int ret;
 
 	rec = active_node->current;
 	BUG_ON(rec->ino != key->objectid || rec->refs > 1);
@@ -1221,7 +1230,9 @@ static int process_file_extent(struct btrfs_root *root,
 		else
 			disk_bytenr += extent_offset;
 
-		found = count_csum_range(root, disk_bytenr, num_bytes);
+		ret = count_csum_range(root, disk_bytenr, num_bytes, &found);
+		if (ret < 0)
+			return ret;
 		if (extent_type == BTRFS_FILE_EXTENT_REG) {
 			if (found > 0)
 				rec->found_csum_item = 1;
@@ -1242,7 +1253,6 @@ static int process_one_leaf(struct btrfs_root *root, struct extent_buffer *eb,
 	u32 nritems;
 	int i;
 	int ret = 0;
-	int error = 0;
 	struct cache_tree *inode_cache;
 	struct shared_node *active_node;
 
@@ -1292,10 +1302,8 @@ static int process_one_leaf(struct btrfs_root *root, struct extent_buffer *eb,
 		default:
 			break;
 		};
-		if (ret != 0)
-			error = 1;
 	}
-	return error;
+	return ret;
 }
 
 static void reada_walk_down(struct btrfs_root *root,
@@ -1366,6 +1374,8 @@ static int walk_down_tree(struct btrfs_root *root, struct btrfs_path *path,
 			break;
 		if (*level == 0) {
 			ret = process_one_leaf(root, cur, wc);
+			if (ret < 0)
+				err = ret;
 			break;
 		}
 		bytenr = btrfs_node_blockptr(cur, path->slots[*level]);
@@ -1759,6 +1769,7 @@ static int merge_root_recs(struct btrfs_root *root,
 	struct ptr_node *node;
 	struct inode_record *rec;
 	struct inode_backref *backref;
+	int ret = 0;
 
 	if (root->root_key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
 		free_inode_recs_tree(src_cache);
@@ -1774,7 +1785,10 @@ static int merge_root_recs(struct btrfs_root *root,
 		remove_cache_extent(src_cache, &node->cache);
 		free(node);
 
-		if (!is_child_root(root, root->objectid, rec->ino))
+		ret = is_child_root(root, root->objectid, rec->ino);
+		if (ret < 0)
+			break;
+		else if (ret == 0)
 			goto skip;
 
 		list_for_each_entry(backref, &rec->backrefs, list) {
@@ -1795,6 +1809,8 @@ static int merge_root_recs(struct btrfs_root *root,
 skip:
 		free_inode_rec(rec);
 	}
+	if (ret < 0)
+		return ret;
 	return 0;
 }
 
@@ -1953,6 +1969,7 @@ static int check_fs_root(struct btrfs_root *root,
 			 struct walk_control *wc)
 {
 	int ret = 0;
+	int err = 0;
 	int wret;
 	int level;
 	struct btrfs_path path;
@@ -1990,7 +2007,8 @@ static int check_fs_root(struct btrfs_root *root,
 		level = root_item->drop_level;
 		path.lowest_level = level;
 		wret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
-		BUG_ON(wret < 0);
+		if (wret < 0)
+			goto skip_walking;
 		btrfs_node_key(path.nodes[level], &found_key,
 				path.slots[level]);
 		WARN_ON(memcmp(&found_key, &root_item->drop_progress,
@@ -2010,9 +2028,12 @@ static int check_fs_root(struct btrfs_root *root,
 		if (wret != 0)
 			break;
 	}
+skip_walking:
 	btrfs_release_path(&path);
 
-	merge_root_recs(root, &root_node.root_cache, root_cache);
+	err = merge_root_recs(root, &root_node.root_cache, root_cache);
+	if (err < 0)
+		ret = err;
 
 	if (root_node.current) {
 		root_node.current->checked = 1;
@@ -2020,7 +2041,9 @@ static int check_fs_root(struct btrfs_root *root,
 				root_node.current);
 	}
 
-	ret = check_inode_recs(root, &root_node.inode_cache);
+	err = check_inode_recs(root, &root_node.inode_cache);
+	if (!ret)
+		ret = err;
 	return ret;
 }
 
@@ -2061,13 +2084,19 @@ static int check_fs_roots(struct btrfs_root *root,
 	key.objectid = 0;
 	key.type = BTRFS_ROOT_ITEM_KEY;
 	ret = btrfs_search_slot(NULL, tree_root, &key, &path, 0, 0);
-	BUG_ON(ret < 0);
+	if (ret < 0) {
+		err = 1;
+		goto out;
+	}
 	while (1) {
 		leaf = path.nodes[0];
 		if (path.slots[0] >= btrfs_header_nritems(leaf)) {
 			ret = btrfs_next_leaf(tree_root, &path);
-			if (ret != 0)
+			if (ret) {
+				if (ret < 0)
+					err = 1;
 				break;
+			}
 			leaf = path.nodes[0];
 		}
 		btrfs_item_key_to_cpu(leaf, &key, path.slots[0]);
@@ -2098,8 +2127,10 @@ static int check_fs_roots(struct btrfs_root *root,
 next:
 		path.slots[0]++;
 	}
+out:
 	btrfs_release_path(&path);
-
+	if (err)
+		free_extent_cache_tree(&wc.shared);
 	if (!cache_tree_empty(&wc.shared))
 		fprintf(stderr, "warning line %d\n", __LINE__);
 
@@ -6245,7 +6276,8 @@ again:
 	btrfs_set_key_type(&key, BTRFS_ROOT_ITEM_KEY);
 	ret = btrfs_search_slot(NULL, root->fs_info->tree_root,
 					&key, &path, 0, 0);
-	BUG_ON(ret < 0);
+	if (ret < 0)
+		goto out;
 	while(1) {
 		leaf = path.nodes[0];
 		slot = path.slots[0];
-- 
1.9.0

[PATCH 3/3] Btrfs-progs: fsck: add tests for extent tree rebuilding

[Wang Shilong]

We need test to verify extent tree rebuilding work, this test
create a strange filesystem with some snapshots, destroy
extent root node, and run fsck with "--init-extent-tree".

Since this tests need btrfs internal tool(btrfs-corrupt-block),so
i add this test into btrfs-progs.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 tests/fsck-tests.sh | 69 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 58 insertions(+), 11 deletions(-)

diff --git a/tests/fsck-tests.sh b/tests/fsck-tests.sh
index 25c390d..b783b78 100644
--- a/tests/fsck-tests.sh
+++ b/tests/fsck-tests.sh
@@ -6,27 +6,74 @@
 #
 
 here=`pwd`
+TEST_DEV=
+TEST_MNT=
+RESULT="fsck-tests-results.txt"
 
 _fail()
 {
-	echo "$*" | tee -a fsck-tests-results.txt
+	echo "$*" | tee -a $RESULT
 	exit 1
 }
 
-rm -f fsck-tests-results.txt
+run_check()
+{
+	echo "############### $@" >> $RESULT 2>&1
+	"$@" >> $RESULT 2>&1 || _fail "failed: $@"
+}
+
+rm -f $RESULT
+
+if [ -z $TEST_DEV ] || [ -z $TEST_MNT ];then
+	_fail "please set TEST_DEV and TEST_MNT"
+fi
+
+# test rely on corrupting blocks tool
+run_check make btrfs-corrupt-block
 
 for i in $(find $here/tests/fsck-tests -name '*.img')
 do
 	echo "     [TEST]    $(basename $i)"
-	echo "testing image $i" >> fsck-tests-results.txt
-	$here/btrfs-image -r $i test.img >> fsck-tests-results.txt 2>&1 \
-		|| _fail "restore failed"
-	$here/btrfsck test.img >> fsck-test-results.txt 2>&1
-	[ $? -eq 0 ] && _fail "btrfsck should have detected corruption"
+	echo "testing image $i" >> $RESULT
+
+	run_check $here/btrfs-image -r $i test.img
 
-	$here/btrfsck --repair test.img >> fsck-test-results.txt 2>&1 || \
-		_fail "btrfsck should have repaired the image"
+	$here/btrfsck test.img >> $RESULT 2>&1
+	[ $? -eq 0 ] && _fail "btrfsck should have detected corruption"
 
-	$here/btrfsck test.img >> fsck-test-results.txt 2>&1 || \
-		_fail "btrfsck did not correct corruption"
+	run_check $here/btrfsck --repair test.img
+	run_check $here/btrfsck test.img
 done
+
+# test whether fsck can rebuild a corrupted extent tree
+test_extent_tree_rebuild()
+{
+	echo "     [TEST]    extent tree rebuild"
+	$here/mkfs.btrfs -f $TEST_DEV >> /dev/null 2>&1 || _fail "fail to mkfs"
+
+	run_check mount $TEST_DEV $TEST_MNT
+	cp -aR /lib/modules/`uname -r`/ $TEST_MNT 2>&1
+
+	for i in `seq 1 100`;do
+		$here/btrfs sub snapshot $TEST_MNT \
+			$TEST_MNT/snapaaaaaaa_$i >& /dev/null
+	done
+	run_check umount $TEST_DEV
+
+	# get extent root bytenr
+	extent_root_bytenr=`$here/btrfs-debug-tree -r $TEST_DEV | grep extent | awk '{print $7}'`
+	if [ -z $extent_root_bytenr ];then
+		_fail "fail to get extent root bytenr"
+	fi
+
+	# corrupt extent root node block
+	run_check $here/btrfs-corrupt-block -l $extent_root_bytenr \
+		-b 4096 $TEST_DEV
+
+	$here/btrfs check $TEST_DEV >& /dev/null && \
+			_fail "fsck should detect failure"
+	run_check $here/btrfs check --init-extent-tree $TEST_DEV
+	run_check $here/btrfs check $TEST_DEV
+}
+
+test_extent_tree_rebuild
-- 
1.9.0