diff mbox

[RFC,03/16] btrfs-progs: fsck: Introduce function to query tree block level

Message ID 1461642543-4621-4-git-send-email-quwenruo@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Qu Wenruo April 26, 2016, 3:48 a.m. UTC
From: Lu Fengqi <lufq.fnst@cn.fujitsu.com>

Introduce function query_tree_block_level() to resolve tree block level
by reading out the tree block.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 cmds-check.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Josef Bacik April 28, 2016, 2:13 p.m. UTC | #1
On 04/25/2016 11:48 PM, Qu Wenruo wrote:
> From: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>
> Introduce function query_tree_block_level() to resolve tree block level
> by reading out the tree block.
>
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---

This skips the check_tree_block if the transid passes, but we could have 
a matching transid but a corrupt item.  You need to fix 
read_tree_block_fs_info to always call check_block so we can be sure 
that our btrfs_header_level() is valid here.  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
Qu Wenruo April 29, 2016, 3:35 a.m. UTC | #2
Josef Bacik wrote on 2016/04/28 10:13 -0400:
> On 04/25/2016 11:48 PM, Qu Wenruo wrote:
>> From: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>>
>> Introduce function query_tree_block_level() to resolve tree block level
>> by reading out the tree block.
>>
>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> ---
>
> This skips the check_tree_block if the transid passes, but we could have
> a matching transid but a corrupt item.  You need to fix
> read_tree_block_fs_info to always call check_block so we can be sure
> that our btrfs_header_level() is valid here.  Thanks,
>
> Josef
>
>
read_tree_block_fs_info(or read_tree_block) will call check_tree_block() 
and verify_parent_transid() if it doesn't find a cached extent buffer.

So I don't think we need to do further modification to 
read_tree_block_fs_info().

But your comment reminds me that, we did miss the generation check, as 
we can get generation from backref, and when we can, we should also 
check transid.

We will add transid check from metadata backref.

Thanks,
Qu


--
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 April 29, 2016, 1:51 p.m. UTC | #3
On 04/28/2016 11:35 PM, Qu Wenruo wrote:
>
>
> Josef Bacik wrote on 2016/04/28 10:13 -0400:
>> On 04/25/2016 11:48 PM, Qu Wenruo wrote:
>>> From: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>>>
>>> Introduce function query_tree_block_level() to resolve tree block level
>>> by reading out the tree block.
>>>
>>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>>> ---
>>
>> This skips the check_tree_block if the transid passes, but we could have
>> a matching transid but a corrupt item.  You need to fix
>> read_tree_block_fs_info to always call check_block so we can be sure
>> that our btrfs_header_level() is valid here.  Thanks,
>>
>> Josef
>>
>>
> read_tree_block_fs_info(or read_tree_block) will call check_tree_block()
> and verify_parent_transid() if it doesn't find a cached extent buffer.
>
> So I don't think we need to do further modification to
> read_tree_block_fs_info().
>
> But your comment reminds me that, we did miss the generation check, as
> we can get generation from backref, and when we can, we should also
> check transid.
>
> We will add transid check from metadata backref.
>

Sigh I was looking at an older version of btrfs-progs, you are right. 
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
diff mbox

Patch

diff --git a/cmds-check.c b/cmds-check.c
index d097edd..6633b6e 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -8716,6 +8716,26 @@  error:
 	return err;
 }
 
+/*
+ * Get real tree block level for case like shared block
+ * Return >= 0 as tree level
+ * Return <0 for error
+ */
+static int query_tree_block_level(struct btrfs_fs_info *fs_info, u64 bytenr)
+{
+	struct extent_buffer *eb;
+	u32 nodesize = btrfs_super_nodesize(fs_info->super_copy);
+	int ret = -EIO;
+
+	eb = read_tree_block_fs_info(fs_info, bytenr, nodesize, 0);
+	if (!extent_buffer_uptodate(eb))
+		goto out;
+	ret = btrfs_header_level(eb);
+out:
+	free_extent_buffer(eb);
+	return ret;
+}
+
 static int btrfs_fsck_reinit_root(struct btrfs_trans_handle *trans,
 			   struct btrfs_root *root, int overwrite)
 {