diff mbox series

btrfs-progs: convert-ext2: insert a dummy inode item before inode ref

Message ID 6f4ac3afeeef5410d70713bb2fe07245f5817fb6.1705104367.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: convert-ext2: insert a dummy inode item before inode ref | expand

Commit Message

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

  Create btrfs metadata
  corrupt leaf: root=5 block=5001931145216 slot=1 ino=89911763, invalid previous key objectid, have 89911762 expect 89911763
  leaf 5001931145216 items 336 free space 7 generation 90 owner FS_TREE
  leaf 5001931145216 flags 0x1(WRITTEN) backref revision 1
  fs uuid 8b69f018-37c3-4b30-b859-42ccfcbe2449
  chunk uuid 448ce78c-ea41-49f6-99dc-46ad80b93da9
          item 0 key (89911762 INODE_REF 3858733) itemoff 16222 itemsize 61
                  index 171 namelen 51 name: [FILENAME1]
          item 1 key (89911763 INODE_REF 3858733) itemoff 16161 itemsize 61
                  index 103 namelen 51 name: [FILENAME2]

[CAUSE]
When iterating a directory, btrfs-convert would insert the DIR_ITEMs,
along with the INODE_REF of that inode.

This leads to above stray INODE_REFs, and trigger the tree-checker.

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]
Insert a dummy INODE_ITEM for the INODE_REF first, the inode items would
be updated when iterating the child inode of the directory.

Issue: #731
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/source-ext2.c | 30 ++++++++++++++++++++----------
 convert/source-fs.c   | 13 +++++++++++++
 2 files changed, 33 insertions(+), 10 deletions(-)

--
2.43.0
diff mbox series

Patch

diff --git a/convert/source-ext2.c b/convert/source-ext2.c
index 7e93b0901489..f56d79734715 100644
--- a/convert/source-ext2.c
+++ b/convert/source-ext2.c
@@ -857,6 +857,10 @@  static int ext2_copy_single_inode(struct btrfs_trans_handle *trans,
 	struct btrfs_key inode_key;
 	struct btrfs_path path = { 0 };

+	inode_key.objectid = objectid;
+	inode_key.type = BTRFS_INODE_ITEM_KEY;
+	inode_key.offset = 0;
+
 	if (ext2_inode->i_links_count == 0)
 		return 0;

@@ -878,13 +882,23 @@  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).
+	 * The inode may already be created (with dummy contents), in that
+	 * case we don't need to do anything yet.
+	 * The inode item would be updated at the end anyway.
 	 */
-	ret = btrfs_insert_inode(trans, root, objectid, &btrfs_inode);
-	if (ret < 0)
-		return ret;
+	ret = btrfs_lookup_inode(trans, root, &path, &inode_key, 1);
+	btrfs_release_path(&path);
+	if (ret > 0) {
+		/*
+		 * No inode item yet, 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:
@@ -917,10 +931,6 @@  static int ext2_copy_single_inode(struct btrfs_trans_handle *trans,
 	 * 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;
diff --git a/convert/source-fs.c b/convert/source-fs.c
index fe1ff7d0d795..ff6912dfa21f 100644
--- a/convert/source-fs.c
+++ b/convert/source-fs.c
@@ -183,6 +183,7 @@  int convert_insert_dirent(struct btrfs_trans_handle *trans,
 {
 	int ret;
 	u64 inode_size;
+	struct btrfs_inode_item dummy_iitem = { 0 };
 	struct btrfs_key location = {
 		.objectid = objectid,
 		.offset = 0,
@@ -193,6 +194,18 @@  int convert_insert_dirent(struct btrfs_trans_handle *trans,
 				    dir, &location, file_type, index_cnt);
 	if (ret)
 		return ret;
+	/*
+	 * We must have an INOTE_ITEM before INODE_REF, or tree-checker won't
+	 * be happy.
+	 * The content of the INODE_ITEM would be properly updated when iterating
+	 * that child inode.
+	 */
+	ret = btrfs_insert_inode(trans, root, objectid, &dummy_iitem);
+	/* The inode item is already there, just skip it. */
+	if (ret == -EEXIST)
+		ret = 0;
+	if (ret < 0)
+		return ret;
 	ret = btrfs_insert_inode_ref(trans, root, name, name_len,
 				     objectid, dir, index_cnt);
 	if (ret)