diff mbox series

btrfs-progs: convert-ext2: fix possible tree-checker error when converting a large fs

Message ID f238f09e92c4043d5e6892c322234515b260df20.1705038162.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: convert-ext2: fix possible tree-checker error when converting a large fs | expand

Commit Message

Qu Wenruo Jan. 12, 2024, 5:43 a.m. UTC
[BUG]
There is a report about failed btrfs-convert, which shows the following
error:

 corrupt leaf: root=5 block=5001928998912 slot=1 ino=89911763, invalid previous key objectid, have 89911762 expect 89911763
 ERROR: failed to copy ext2 inode 89911320: -5
 ERROR: error during copy_inodes -5
 WARNING: error during conversion, the original filesystem is not modified

[CAUSE]
Above error is triggered when checking the following items inside a
subvolume:

- inode ref
- dir item/index
- file extent
- xattr

This is to make sure these items have correct previous key.

However btrfs-convert is not following this requirement, it always
insert those items first, then create a btrfs_inode for it.

Thus it can lead to the error.

This can only happen for large fs, as for most cases we have all these
modified tree blocks cached, thus tree-checker won't be triggered.
But when the tree block cache is not hit, and we have to read from disk,
then such behavior can lead to above tree-checker error.

[FIX]
Make sure we insert the inode item first, then the file extents/dir
items/xattrs.
And after the file extents/dir items/xattrs inserted, we update the
existing inode (to update its size and bytes).

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/source-ext2.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

Comments

David Sterba Jan. 12, 2024, 3:37 p.m. UTC | #1
On Fri, Jan 12, 2024 at 04:13:27PM +1030, Qu Wenruo wrote:
> [BUG]
> There is a report about failed btrfs-convert, which shows the following
> error:
> 
>  corrupt leaf: root=5 block=5001928998912 slot=1 ino=89911763, invalid previous key objectid, have 89911762 expect 89911763
>  ERROR: failed to copy ext2 inode 89911320: -5
>  ERROR: error during copy_inodes -5
>  WARNING: error during conversion, the original filesystem is not modified
> 
> [CAUSE]
> Above error is triggered when checking the following items inside a
> subvolume:
> 
> - inode ref
> - dir item/index
> - file extent
> - xattr
> 
> This is to make sure these items have correct previous key.
> 
> However btrfs-convert is not following this requirement, it always
> insert those items first, then create a btrfs_inode for it.
> 
> Thus it can lead to the error.
> 
> This can only happen for large fs, as for most cases we have all these
> modified tree blocks cached, thus tree-checker won't be triggered.
> But when the tree block cache is not hit, and we have to read from disk,
> then such behavior can lead to above tree-checker error.
> 
> [FIX]
> Make sure we insert the inode item first, then the file extents/dir
> items/xattrs.
> And after the file extents/dir items/xattrs inserted, we update the
> existing inode (to update its size and bytes).
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Added to devel, thanks.
diff mbox series

Patch

diff --git a/convert/source-ext2.c b/convert/source-ext2.c
index cfffc9e21b2d..7e93b0901489 100644
--- a/convert/source-ext2.c
+++ b/convert/source-ext2.c
@@ -854,6 +854,8 @@  static int ext2_copy_single_inode(struct btrfs_trans_handle *trans,
 	int ret;
 	int s_inode_size;
 	struct btrfs_inode_item btrfs_inode;
+	struct btrfs_key inode_key;
+	struct btrfs_path path = { 0 };
 
 	if (ext2_inode->i_links_count == 0)
 		return 0;
@@ -875,6 +877,15 @@  static int ext2_copy_single_inode(struct btrfs_trans_handle *trans,
 	}
 	ext2_convert_inode_flags(&btrfs_inode, ext2_inode);
 
+	/*
+	 * The inode item must be inserted before any file extents/dir items/xattrs,
+	 * or we may trigger tree-checker. (File extents/dir items/xattrs require
+	 * the previous item has the same key objectid).
+	 */
+	ret = btrfs_insert_inode(trans, root, objectid, &btrfs_inode);
+	if (ret < 0)
+		return ret;
+
 	switch (ext2_inode->i_mode & S_IFMT) {
 	case S_IFREG:
 		ret = ext2_create_file_extents(trans, root, objectid,
@@ -901,7 +912,25 @@  static int ext2_copy_single_inode(struct btrfs_trans_handle *trans,
 		if (ret)
 			return ret;
 	}
-	return btrfs_insert_inode(trans, root, objectid, &btrfs_inode);
+
+	/*
+	 * Update the inode item, as above insert never updates the inode's
+	 * nbytes and size.
+	 */
+	inode_key.objectid = objectid;
+	inode_key.type = BTRFS_INODE_ITEM_KEY;
+	inode_key.offset = 0;
+
+	ret = btrfs_lookup_inode(trans, root, &path, &inode_key, 1);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0)
+		return ret;
+	write_extent_buffer(path.nodes[0], &btrfs_inode,
+			    btrfs_item_ptr_offset(path.nodes[0], path.slots[0]),
+			    sizeof(btrfs_inode));
+	btrfs_release_path(&path);
+	return 0;
 }
 
 static bool ext2_is_special_inode(ext2_filsys ext2_fs, ext2_ino_t ino)