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

(cherry picked from commit a518b0932c70bfd79143d402bb110cf6fd831466)
This commit is contained in:
liuh 2024-09-03 11:21:05 +08:00 committed by openeuler-sync-bot
parent cafebc3a44
commit 9e90dd065a
3 changed files with 323 additions and 1 deletions

View File

@ -0,0 +1,106 @@
From 631ee666394a36ec36abf1042956cdb2e6058b86 Mon Sep 17 00:00:00 2001
From: Qu Wenruo <wqu@suse.com>
Date: Fri, 12 Jan 2024 16:13:27 +1030
Subject: [PATCH] btrfs-progs: convert: for ext2, fix possible tree-checker
error when converting a large fs
[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
inserts those items first, then creates 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>
Signed-off-by: David Sterba <dsterba@suse.com>
---
convert/source-ext2.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/convert/source-ext2.c b/convert/source-ext2.c
index cfffc9e..ad7aeda 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)
--
2.43.0

View File

@ -0,0 +1,210 @@
From 517ba2d9e58aa50c4dbfe873ac4634cfd9c07ff2 Mon Sep 17 00:00:00 2001
From: Qu Wenruo <wqu@suse.com>
Date: Sat, 13 Jan 2024 19:07:06 +1030
Subject: [PATCH] btrfs-progs: convert: insert a dummy inode item before inode
ref for ext2/4
[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>
Signed-off-by: David Sterba <dsterba@suse.com>
---
check/mode-common.h | 15 ---------------
common/utils.h | 16 ++++++++++++++++
convert/source-ext2.c | 30 ++++++++++++++++++++----------
convert/source-fs.c | 20 ++++++++++++++++++++
4 files changed, 56 insertions(+), 25 deletions(-)
diff --git a/check/mode-common.h b/check/mode-common.h
index 894bbbb..80672e5 100644
--- a/check/mode-common.h
+++ b/check/mode-common.h
@@ -167,21 +167,6 @@ static inline bool is_valid_imode(u32 imode)
int recow_extent_buffer(struct btrfs_root *root, struct extent_buffer *eb);
-static inline u32 btrfs_type_to_imode(u8 type)
-{
- static u32 imode_by_btrfs_type[] = {
- [BTRFS_FT_REG_FILE] = S_IFREG,
- [BTRFS_FT_DIR] = S_IFDIR,
- [BTRFS_FT_CHRDEV] = S_IFCHR,
- [BTRFS_FT_BLKDEV] = S_IFBLK,
- [BTRFS_FT_FIFO] = S_IFIFO,
- [BTRFS_FT_SOCK] = S_IFSOCK,
- [BTRFS_FT_SYMLINK] = S_IFLNK,
- };
-
- return imode_by_btrfs_type[(type)];
-}
-
int get_extent_item_generation(u64 bytenr, u64 *gen_ret);
/*
diff --git a/common/utils.h b/common/utils.h
index e267814..85f086e 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -22,6 +22,7 @@
#include "kerncompat.h"
#include <stdbool.h>
#include <stddef.h>
+#include <sys/stat.h>
#include "kernel-lib/list.h"
#include "kernel-shared/volumes.h"
#include "common/fsfeatures.h"
@@ -40,6 +41,21 @@ enum exclusive_operation {
BTRFS_EXCLOP_UNKNOWN = -1,
};
+static inline u32 btrfs_type_to_imode(u8 type)
+{
+ static u32 imode_by_btrfs_type[] = {
+ [BTRFS_FT_REG_FILE] = S_IFREG,
+ [BTRFS_FT_DIR] = S_IFDIR,
+ [BTRFS_FT_CHRDEV] = S_IFCHR,
+ [BTRFS_FT_BLKDEV] = S_IFBLK,
+ [BTRFS_FT_FIFO] = S_IFIFO,
+ [BTRFS_FT_SOCK] = S_IFSOCK,
+ [BTRFS_FT_SYMLINK] = S_IFLNK,
+ };
+
+ return imode_by_btrfs_type[(type)];
+}
+
/* 2 for "0x", 2 for each byte, plus nul */
#define BTRFS_CSUM_STRING_LEN (2 + 2 * BTRFS_CSUM_SIZE + 1)
void btrfs_format_csum(u16 csum_type, const u8 *data, char *output);
diff --git a/convert/source-ext2.c b/convert/source-ext2.c
index ad7aeda..2186b25 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 to have 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 fe1ff7d..6656143 100644
--- a/convert/source-fs.c
+++ b/convert/source-fs.c
@@ -23,6 +23,8 @@
#include "kernel-shared/ctree.h"
#include "kernel-shared/disk-io.h"
#include "kernel-shared/volumes.h"
+#include "kernel-shared/transaction.h"
+#include "common/utils.h"
#include "common/internal.h"
#include "common/messages.h"
#include "common/extent-cache.h"
@@ -183,6 +185,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 +196,23 @@ int convert_insert_dirent(struct btrfs_trans_handle *trans,
dir, &location, file_type, index_cnt);
if (ret)
return ret;
+
+ btrfs_set_stack_inode_mode(&dummy_iitem, btrfs_type_to_imode(file_type));
+ btrfs_set_stack_inode_generation(&dummy_iitem, trans->transid);
+ btrfs_set_stack_inode_transid(&dummy_iitem, trans->transid);
+ /*
+ * 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, but we should still try to make it as valid as
+ * possible, or we may still trigger some tree checker.
+ */
+ 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)
--
2.43.0

View File

@ -1,6 +1,6 @@
Name: btrfs-progs
Version: 6.6.3
Release: 11
Release: 12
Summary: btrfs userspace programs
License: GPLv2 and GPL+ and LGPL-2.1+ and GPL-3.0+ and LGPL-2.1 and MIT
URL: https://btrfs.wiki.kernel.org/index.php/Main_Page
@ -16,6 +16,8 @@ Patch0007: 0007-btrfs-progs-fi-show-canonicalize-path-when-using-blk.patch
Patch0008: 0008-btrfs-progs-tune-fix-the-missing-close-of-filesystem.patch
Patch0009: 0009-btrfs-progs-error-out-immediately-if-an-unknown-back.patch
Patch0010: 0010-btrfs-progs-fix-the-conflicting-super-block-flags.patch
Patch0011: 0011-btrfs-progs-convert-for-ext2-fix-possible-tree-check.patch
Patch0012: 0012-btrfs-progs-convert-insert-a-dummy-inode-item-before.patch
BuildRequires: python3-devel >= 3.4
BuildRequires: libacl-devel, e2fsprogs-devel, libblkid-devel, libuuid-devel, zlib-devel, libzstd-devel, lzo-devel, systemd-devel
@ -81,6 +83,10 @@ make mandir=%{_mandir} bindir=%{_sbindir} libdir=%{_libdir} incdir=%{_includedir
%{_mandir}/man8/*.gz
%changelog
* Tue Sep 3 2024 liuh <liuhuan01@kylinos.cn> - 6.6.3-12
- btrfs-progs: convert: insert a dummy inode item before inode ref for ext2/4
- btrfs-progs: convert: for ext2, fix possible tree-checker error when converting a large fs
* Tue Aug 27 2024 liuh <liuhuan01@kylinos.cn> - 6.6.3-11
- btrfs-progs: fix the conflicting super block flags