Skip to contentSkip to conversation

btrfs-progs: Use more loose open ctree flags for dump-tree and restore

PATCH In review Qu Wenruo 3 participants 5 messages Apr 6, 2018 – Apr 10, 2018 Btrfs file system

lore
Share & tools
lore

Qu Wenruo

author
#0 lore raw

PATCH 1/1 btrfs-progs: Use more loose open ctree flags for dump-tree and restore

Corrupted extent tree (either the root node or leaf) can normally block us from open the fs.
As normally open_ctree() has the following call chain:
__open_ctree_fd()
|- btrfs_setup_all_roots()
|- btrfs_read_block_groups()
And we will search block group items in extent tree.

And considering how block group items are scattered around the whole extent tree, any error would block the fs from being mounted.

Fortunately, we already have OPEN_CTREE_NO_BLOCK_GROUPS flags to disable block group items search, which will not only allow us to open some fs, but also hugely speed up open time.

Currently dump-tree and btrfs-restore is ensured that they care nothing about block group items. So specify OPEN_CTREE_NO_BLOCK_GROUPS flag as default.

Also fix a typo where dump-tree is using OPEN_CTREE_FS_PARTIAL, which should be OPEN_CTREE_PARTIAL.
This makes dump-tree do more check and can sometimes fail to open certain filesystems.

Fixes 8698a2b9ba89 ("btrfs-progs: Allow inspect dump-tree to show specified tree block even some tree roots are corrupted")
---
 cmds-inspect-dump-tree.c | 4 +++-
 cmds-restore.c           | 3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)
cmds-inspect-dump-tree.c +3 -1
index 7defb7164a49..8be976041543 100644
@@ -303,7 +303,9 @@ int cmd_inspect_dump_tree(int argc, char **argv)
 	int uuid_tree_only = 0;
 	int roots_only = 0;
 	int root_backups = 0;
-	unsigned open_ctree_flags = OPEN_CTREE_FS_PARTIAL;
+	/* Speed up open_ctree() and continue if extent tree is corrupted */
+	unsigned open_ctree_flags = OPEN_CTREE_PARTIAL |
+				    OPEN_CTREE_NO_BLOCK_GROUPS;
 	u64 block_bytenr;
 	struct btrfs_root *tree_root_scan;
 	u64 tree_id = 0;
cmds-restore.c +2 -1
index ade35f0f880f..b43bd2ac6502 100644
@@ -1282,7 +1282,8 @@ static struct btrfs_root *open_fs(const char *dev, u64 root_location,
 	for (i = super_mirror; i < BTRFS_SUPER_MIRROR_MAX; i++) {
 		bytenr = btrfs_sb_offset(i);
 		fs_info = open_ctree_fs_info(dev, bytenr, root_location, 0,
-					     OPEN_CTREE_PARTIAL);
+					     OPEN_CTREE_PARTIAL |
+					     OPEN_CTREE_NO_BLOCK_GROUPS);
 		if (fs_info)
 			break;
 		fprintf(stderr, "Could not open root, trying backup super\n");
Signature · 1 line
2.17.0
#1 lore raw
Quoted 15 lines · Qu Wenruo

Corrupted extent tree (either the root node or leaf) can normally block us from open the fs.
As normally open_ctree() has the following call chain:
__open_ctree_fd()
|- btrfs_setup_all_roots()
|- btrfs_read_block_groups()
And we will search block group items in extent tree.

And considering how block group items are scattered around the whole extent tree, any error would block the fs from being mounted.

Fortunately, we already have OPEN_CTREE_NO_BLOCK_GROUPS flags to disable block group items search, which will not only allow us to open some fs, but also hugely speed up open time.

Quoted 8 lines

Currently dump-tree and btrfs-restore is ensured that they care nothing about block group items. So specify OPEN_CTREE_NO_BLOCK_GROUPS flag as default.

Also fix a typo where dump-tree is using OPEN_CTREE_FS_PARTIAL, which should be OPEN_CTREE_PARTIAL.
This makes dump-tree do more check and can sometimes fail to open certain filesystems.

Oh, that's too easy to get wrong, we should rename one or the other.

Quoted 13 lines
Fixes 8698a2b9ba89 ("btrfs-progs: Allow inspect dump-tree to show specified tree block even some tree roots are corrupted")
---
 cmds-inspect-dump-tree.c | 4 +++-
 cmds-restore.c           | 3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)
cmds-inspect-dump-tree.c +0 -0
index 7defb7164a49..8be976041543 100644

cmds-inspect-dump-tree.c int cmd_inspect_dump_tree(int argc, char **argv) · line 308

Quoted 7 lines
cmds-inspect-dump-tree.c +3 -1
@@ -303,7 +303,9 @@ int cmd_inspect_dump_tree(int argc, char **argv)
 	int uuid_tree_only = 0;
 	int roots_only = 0;
 	int root_backups = 0;
-	unsigned open_ctree_flags = OPEN_CTREE_FS_PARTIAL;
+	/* Speed up open_ctree() and continue if extent tree is corrupted */
+	unsigned open_ctree_flags = OPEN_CTREE_PARTIAL |
+				    OPEN_CTREE_NO_BLOCK_GROUPS;

We could consider that separatelly, whether to allow dumping a partially created filesystem, ie. OPEN_CTREE_FS_PARTIAL added by a new option. This would be really for debugging/testing purposes, but could be useful when the filesystem is prefilled with files or if we enable the runtime features and the final write fails for some reason.

Quoted 17 lines
cmds-inspect-dump-tree.c +0 -0
 	u64 block_bytenr;
 	struct btrfs_root *tree_root_scan;
 	u64 tree_id = 0;
cmds-restore.c +2 -1
index ade35f0f880f..b43bd2ac6502 100644
@@ -1282,7 +1282,8 @@ static struct btrfs_root *open_fs(const char *dev, u64 root_location,
 	for (i = super_mirror; i < BTRFS_SUPER_MIRROR_MAX; i++) {
 		bytenr = btrfs_sb_offset(i);
 		fs_info = open_ctree_fs_info(dev, bytenr, root_location, 0,
-					     OPEN_CTREE_PARTIAL);
+					     OPEN_CTREE_PARTIAL |
+					     OPEN_CTREE_NO_BLOCK_GROUPS);
 		if (fs_info)
 			break;
 		fprintf(stderr, "Could not open root, trying backup super\n");
#2 lore raw
Quoted 20 lines · David Sterba
Quoted 19 lines · Qu Wenruo

Corrupted extent tree (either the root node or leaf) can normally block us from open the fs.
As normally open_ctree() has the following call chain:
__open_ctree_fd()
|- btrfs_setup_all_roots()
|- btrfs_read_block_groups()
And we will search block group items in extent tree.

And considering how block group items are scattered around the whole extent tree, any error would block the fs from being mounted.

Fortunately, we already have OPEN_CTREE_NO_BLOCK_GROUPS flags to disable block group items search, which will not only allow us to open some fs, but also hugely speed up open time.

Currently dump-tree and btrfs-restore is ensured that they care nothing about block group items. So specify OPEN_CTREE_NO_BLOCK_GROUPS flag as default.

Quoted 6 lines
Quoted 4 lines

Also fix a typo where dump-tree is using OPEN_CTREE_FS_PARTIAL, which should be OPEN_CTREE_PARTIAL.
This makes dump-tree do more check and can sometimes fail to open certain filesystems.

Oh, that's too easy to get wrong, we should rename one or the other.

Of course, I'll add extra patches for this.

Quoted 9 lines
Quoted 9 lines
Fixes 8698a2b9ba89 ("btrfs-progs: Allow inspect dump-tree to show specified tree block even some tree roots are corrupted")
---
 cmds-inspect-dump-tree.c | 4 +++-
 cmds-restore.c           | 3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)
Quoted 15 lines

cmds-inspect-dump-tree.c int cmd_inspect_dump_tree(int argc, char **argv) · line 308

Quoted 12 lines
cmds-inspect-dump-tree.c +3 -1
index 7defb7164a49..8be976041543 100644
@@ -303,7 +303,9 @@ int cmd_inspect_dump_tree(int argc, char **argv)
 	int uuid_tree_only = 0;
 	int roots_only = 0;
 	int root_backups = 0;
-	unsigned open_ctree_flags = OPEN_CTREE_FS_PARTIAL;
+	/* Speed up open_ctree() and continue if extent tree is corrupted */
+	unsigned open_ctree_flags = OPEN_CTREE_PARTIAL |
+				    OPEN_CTREE_NO_BLOCK_GROUPS;

We could consider that separatelly, whether to allow dumping a partially created filesystem, ie. OPEN_CTREE_FS_PARTIAL added by a new option.

For dump-tree, there is really no limitation here. As long as chunk tree is OK, we should be OK to do "-b" dump.

For "-t" or all tree (default) dump, with chunk tree opened only, we could try our best to print any good trees.
So I don't get the point to read all other trees.

Thanks,
Qu

Quoted 27 lines

This would be really for debugging/testing purposes, but could be useful when the filesystem is prefilled with files or if we enable the runtime features and the final write fails for some reason.

Quoted 8 lines
cmds-inspect-dump-tree.c +0 -0
 	u64 block_bytenr;
 	struct btrfs_root *tree_root_scan;
 	u64 tree_id = 0;
cmds-restore.c +0 -0
index ade35f0f880f..b43bd2ac6502 100644

cmds-restore.c static struct btrfs_root *open_fs(const char *dev, u64 root_location, · line 1289

Quoted 9 lines
cmds-restore.c +2 -1
@@ -1282,7 +1282,8 @@ static struct btrfs_root *open_fs(const char *dev, u64 root_location,
 	for (i = super_mirror; i < BTRFS_SUPER_MIRROR_MAX; i++) {
 		bytenr = btrfs_sb_offset(i);
 		fs_info = open_ctree_fs_info(dev, bytenr, root_location, 0,
-					     OPEN_CTREE_PARTIAL);
+					     OPEN_CTREE_PARTIAL |
+					     OPEN_CTREE_NO_BLOCK_GROUPS);
 		if (fs_info)
 			break;
 		fprintf(stderr, "Could not open root, trying backup super\n");

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
Quoted 14 lines · Qu Wenruo
Quoted 5 lines
Quoted 5 lines
cmds-inspect-dump-tree.c +0 -0
@@ -303,7 +303,9 @@ int cmd_inspect_dump_tree(int argc, char **argv)
 	int uuid_tree_only = 0;
 	int roots_only = 0;
Quoted 8 lines

cmds-inspect-dump-tree.c int cmd_inspect_dump_tree(int argc, char **argv) · line 308

Quoted 5 lines
cmds-inspect-dump-tree.c +3 -1
 	int root_backups = 0;
-	unsigned open_ctree_flags = OPEN_CTREE_FS_PARTIAL;
+	/* Speed up open_ctree() and continue if extent tree is corrupted */
+	unsigned open_ctree_flags = OPEN_CTREE_PARTIAL |
+				    OPEN_CTREE_NO_BLOCK_GROUPS;

We could consider that separatelly, whether to allow dumping a partially created filesystem, ie. OPEN_CTREE_FS_PARTIAL added by a new option.

Quoted 6 lines

For dump-tree, there is really no limitation here. As long as chunk tree is OK, we should be OK to do "-b" dump.

For "-t" or all tree (default) dump, with chunk tree opened only, we could try our best to print any good trees.
So I don't get the point to read all other trees.

Yeah, eg. a bad filesystem magic number does not stop dump-tree, so no change is needed.

#4 lore raw
Quoted 18 lines · David Sterba
Quoted 17 lines · Qu Wenruo
Quoted 5 lines
Quoted 5 lines
cmds-inspect-dump-tree.c +0 -0
@@ -303,7 +303,9 @@ int cmd_inspect_dump_tree(int argc, char **argv)
 	int uuid_tree_only = 0;
 	int roots_only = 0;
Quoted 8 lines

cmds-inspect-dump-tree.c int cmd_inspect_dump_tree(int argc, char **argv) · line 308

Quoted 5 lines
cmds-inspect-dump-tree.c +3 -1
 	int root_backups = 0;
-	unsigned open_ctree_flags = OPEN_CTREE_FS_PARTIAL;
+	/* Speed up open_ctree() and continue if extent tree is corrupted */
+	unsigned open_ctree_flags = OPEN_CTREE_PARTIAL |
+				    OPEN_CTREE_NO_BLOCK_GROUPS;

We could consider that separatelly, whether to allow dumping a partially created filesystem, ie. OPEN_CTREE_FS_PARTIAL added by a new option.

For dump-tree, there is really no limitation here. As long as chunk tree is OK, we should be OK to do "-b" dump.

Quoted 6 lines
Quoted 3 lines

For "-t" or all tree (default) dump, with chunk tree opened only, we could try our best to print any good trees.
So I don't get the point to read all other trees.

Yeah, eg. a bad filesystem magic number does not stop dump-tree, so no change is needed.

Bad magic will still stop dump-tree, super block check is not affected by open ctree flags.
The only way to block dump-tree is corrupted chunk tree, which mostly makes the fs garbage.

Even root tree is corrupted, we can still specify bytenr manually to salvage what we need.

The point here is still the same, for read-only operations where we don't care about the cross-reference, there is no need to care about extent tree at all, and skip block group item search could save us a lot of time.

Thanks,
Qu

Quoted 1 line