btrfs-progs: Rename OPEN_CTREE_FS_PARTIAL to OPEN_CTREE_TEMPORARY_SUPER
PATCH v2 1/2 btrfs-progs: Rename OPEN_CTREE_FS_PARTIAL to OPEN_CTREE_TEMPORARY_SUPER
The old flag OPEN_CTREE_FS_PARTIAL is in fact quite easy to be confused with OPEN_CTREE_PARTIAL, which allow btrfs-progs to open damaged filesystem (like corrupted extent/csum tree).
However OPEN_CTREE_FS_PARTIAL, unlike its name, is just allowing btrfs-progs to open fs with temporary superblocks (which only has 6 basic trees on SINGLE meta/sys chunks).
The usage of FS_PARTIAL is really confusing here.
So rename OPEN_CTREE_FS_PARTIAL to OPEN_CTREE_TEMPORARY_SUPER, and add extra comment for its behavior.
Also rename BTRFS_MAGIC_PARTIAL to BTRFS_MAGIC_TEMPORARY to keep the naming consistent.
And with above comment, the usage of FS_PARTIAL in dump-tree is obviously incorrect, fix it.
---
changelog:
v2:
New patch
--- cmds-inspect-dump-tree.c | 2 +- convert/main.c | 4 ++-- ctree.h | 8 +++++--- disk-io.c | 12 ++++++------ disk-io.h | 10 +++++++--- mkfs/main.c | 2 +- 6 files changed, 22 insertions(+), 16 deletions(-)
cmds-inspect-dump-tree.c +1 -1
@@ -220,7 +220,7 @@ 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 ; +unsigned open_ctree_flags =OPEN_CTREE_PARTIAL ;u64 block_only =0 ;struct btrfs_root *tree_root_scan;u64 tree_id =0 ;
convert/main.c +2 -2
@@ -1140,7 +1140,7 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize, } root =open_ctree_fd (fd, devname, mkfs_cfg.super_bytenr, -OPEN_CTREE_WRITES |OPEN_CTREE_FS_PARTIAL ); +OPEN_CTREE_WRITES |OPEN_CTREE_TEMPORARY_SUPER );if (!root) {error ("unable to open ctree" ); goto fail;
@@ -1230,7 +1230,7 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize, } root =open_ctree_fd (fd, devname,0 , -OPEN_CTREE_WRITES |OPEN_CTREE_FS_PARTIAL ); +OPEN_CTREE_WRITES |OPEN_CTREE_TEMPORARY_SUPER );if (!root) {error ("unable to open ctree for finalization" ); goto fail;
ctree.h +5 -3
@@ -45,10 +45,12 @@ struct btrfs_free_space_ctl;#define BTRFS_MAGIC 0x4D5F53665248425FULL/* ascii _BHRfS_M, no null */ /* -* Fake signature for an unfinalized filesystem, structures might be partially -* created or missing. +* Fake signature for an unfinalized filesystem, which only has barebone tree +* structures (normally 6 near empty trees, on SINGLE meta/sys temporary chunks) +* +* ascii !BHRfS_M, no null */ -#define BTRFS_MAGIC_PARTIAL 0x4D5F536652484221ULL/* ascii !BHRfS_M, no null */ +#define BTRFS_MAGIC_TEMPORARY 0x4D5F536652484221ULL#define BTRFS_MAX_MIRRORS 3
disk-io.c +6 -6
@@ -1117,14 +1117,14 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path, fs_info->ignore_chunk_tree_error =1 ;if ((flags &OPEN_CTREE_RECOVER_SUPER ) - && (flags &OPEN_CTREE_FS_PARTIAL )) { + && (flags &OPEN_CTREE_TEMPORARY_SUPER )) {fprintf (stderr, -"cannot open a partially created filesystem for recovery" ); +"cannot open a filesystem with temporary super block for recovery" ); goto out; } -if (flags &OPEN_CTREE_FS_PARTIAL ) - sbflags =SBREAD_PARTIAL ; +if (flags &OPEN_CTREE_TEMPORARY_SUPER ) + sbflags =SBREAD_TEMPORARY ; ret =btrfs_scan_fs_devices (fp, path, &fs_devices, sb_bytenr, sbflags, (flags &OPEN_CTREE_NO_DEVICES ));
@@ -1285,8 +1285,8 @@ static int check_super(struct btrfs_super_block *sb, unsigned sbflags)int csum_size;if (btrfs_super_magic (sb) !=BTRFS_MAGIC ) { -if (btrfs_super_magic (sb) ==BTRFS_MAGIC_PARTIAL ) { -if (!(sbflags &SBREAD_PARTIAL )) { +if (btrfs_super_magic (sb) ==BTRFS_MAGIC_TEMPORARY ) { +if (!(sbflags &SBREAD_TEMPORARY )) {error ("superblock magic doesn't match" );return -EIO ; }
disk-io.h +7 -3
@@ -73,8 +73,12 @@ enum btrfs_open_ctree_flags {*/ OPEN_CTREE_IGNORE_CHUNK_TREE_ERROR = (1U <<11 ), -/* Allow to open a partially created filesystem */ - OPEN_CTREE_FS_PARTIAL = (1U << 12), +/* +* Allow to open fs with temporary superblock (BTRFS_MAGIC_PARTIAL), +* such fs contains very basic tree layout, just able to be opened. +* Such temporary super is used for mkfs or convert. +*/ + OPEN_CTREE_TEMPORARY_SUPER = (1U << 12),/* * Invalidate the free space tree (i.e., clear the FREE_SPACE_TREE_VALID
@@ -95,7 +99,7 @@ enum btrfs_read_sb_flags {* Read superblock with the fake signature, cannot be used with * SBREAD_RECOVER */ - SBREAD_PARTIAL = (1 << 1), + SBREAD_TEMPORARY = (1 << 1), };/*
mkfs/main.c +1 -1
@@ -1099,7 +1099,7 @@ int main(int argc, char **argv) } fs_info =open_ctree_fs_info (file,0 ,0 ,0 , -OPEN_CTREE_WRITES |OPEN_CTREE_FS_PARTIAL ); +OPEN_CTREE_WRITES |OPEN_CTREE_TEMPORARY_SUPER );if (!fs_info) {error ("open ctree failed" ); goto error;
Signature · 1 line
2.17.0
PATCH v2 2/2 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.
---
changelog:
v2:
New comment to explain why OPEN_CTREE_NO_BLOCK_GROUPS is OK here.
--- cmds-inspect-dump-tree.c | 10 +++++++++- cmds-restore.c | 9 ++++++++- 2 files changed, 17 insertions(+), 2 deletions(-)
cmds-inspect-dump-tree.c +9 -1
@@ -220,12 +220,20 @@ 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_PARTIAL ; +unsigned open_ctree_flags;u64 block_only =0 ;struct btrfs_root *tree_root_scan;u64 tree_id =0 ;bool follow = false; +/* +* For debug-tree, we care nothing about extent tree (it's just backref +* and usage accounting, only makes sense for RW operations). +* Use NO_BLOCK_GROUPS here could also speedup open_ctree() and allow us +* to inspect fs with corrupted extent tree blocks, and show as many good +* tree blocks as possible. +*/ + open_ctree_flags =OPEN_CTREE_PARTIAL |OPEN_CTREE_NO_BLOCK_GROUPS ;while (1 ) {int c;enum {GETOPT_VAL_FOLLOW =256 };
cmds-restore.c +8 -1
@@ -1281,8 +1281,15 @@ 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); + +/* +* Restore won't allocate extent and doesn't care anything +* in extent tree. Skip block group item search will allow +* restore to be executed on heavily damaged fs. +*/ 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
PATCH 2/2 btrfs-progs: Use more loose open ctree flags for dump-tree and restore
In reply to Qu WenruoCorrupted 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.
---
changelog:
v2:
New comment to explain why OPEN_CTREE_NO_BLOCK_GROUPS is OK here.
v2.1:
Update commit message, as the typo fix goes to previous patch, not this one.
--- cmds-inspect-dump-tree.c | 10 +++++++++- cmds-restore.c | 9 ++++++++- 2 files changed, 17 insertions(+), 2 deletions(-)
cmds-inspect-dump-tree.c +9 -1
@@ -220,12 +220,20 @@ 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_PARTIAL ; +unsigned open_ctree_flags;u64 block_only =0 ;struct btrfs_root *tree_root_scan;u64 tree_id =0 ;bool follow = false; +/* +* For debug-tree, we care nothing about extent tree (it's just backref +* and usage accounting, only makes sense for RW operations). +* Use NO_BLOCK_GROUPS here could also speedup open_ctree() and allow us +* to inspect fs with corrupted extent tree blocks, and show as many good +* tree blocks as possible. +*/ + open_ctree_flags =OPEN_CTREE_PARTIAL |OPEN_CTREE_NO_BLOCK_GROUPS ;while (1 ) {int c;enum {GETOPT_VAL_FOLLOW =256 };
cmds-restore.c +8 -1
@@ -1281,8 +1281,15 @@ 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); + +/* +* Restore won't allocate extent and doesn't care anything +* in extent tree. Skip block group item search will allow +* restore to be executed on heavily damaged fs. +*/ 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
David Sterba
AppliedQuoted 15 lines · Qu Wenruo
The old flag
OPEN_CTREE_FS_PARTIALis in fact quite easy to be confused withOPEN_CTREE_PARTIAL, which allow btrfs-progs to open damaged filesystem (like corrupted extent/csum tree).However
OPEN_CTREE_FS_PARTIAL, unlike its name, is just allowing btrfs-progs to open fs with temporary superblocks (which only has 6 basic trees on SINGLE meta/sys chunks).The usage of
FS_PARTIALis really confusing here.So rename
OPEN_CTREE_FS_PARTIALtoOPEN_CTREE_TEMPORARY_SUPER, and add extra comment for its behavior.
Also renameBTRFS_MAGIC_PARTIALtoBTRFS_MAGIC_TEMPORARYto keep the naming consistent.
Quoted 5 lines
And with above comment, the usage of
FS_PARTIALin dump-tree is obviously incorrect, fix it.Fixes 8698a2b9ba89 ("btrfs-progs: Allow inspect dump-tree to show specified tree block even some tree roots are corrupted")Signed-off-by Qu Wenruo <[email protected]>
Applied, thanks. I got a few compilation errors where BTRFS_MAGIC_PARTIAL was not updated, but that's trivial to fix.
Hey.
Better late than never ;-)
Just to confirm: At least since 4.16.1, I could btrfs-restore from the broken fs image again (that I've described in "spurious full btrfs corruption" from around mid March).
So the regression in btrfsprogs has in fact been fixed by these patches, it seems.
Thanks,
Chris.
Quoted 188 lines · Qu Wenruo
The old flag
OPEN_CTREE_FS_PARTIALis in fact quite easy to be confused
withOPEN_CTREE_PARTIAL, which allow btrfs-progs to open damaged filesystem (like corrupted extent/csum tree).However
OPEN_CTREE_FS_PARTIAL, unlike its name, is just allowing btrfs-progs to open fs with temporary superblocks (which only has 6 basic trees on SINGLE meta/sys chunks).The usage of
FS_PARTIALis really confusing here.So rename
OPEN_CTREE_FS_PARTIALtoOPEN_CTREE_TEMPORARY_SUPER, and add
extra comment for its behavior.
Also renameBTRFS_MAGIC_PARTIALtoBTRFS_MAGIC_TEMPORARYto keep the naming consistent.And with above comment, the usage of
FS_PARTIALin dump-tree is obviously incorrect, fix it.Fixes 8698a2b9ba89 ("btrfs-progs: Allow inspect dump-tree to showspecified tree block even some tree roots are corrupted")
Signed-off-by Qu Wenruo <[email protected]>---
changelog:
v2:
New patch--- cmds-inspect-dump-tree.c | 2 +- convert/main.c | 4 ++-- ctree.h | 8 +++++--- disk-io.c | 12 ++++++------ disk-io.h | 10 +++++++--- mkfs/main.c | 2 +- 6 files changed, 22 insertions(+), 16 deletions(-)cmds-inspect-dump-tree.c +1
-1@@ -220,7 +220,7 @@ 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 ; +unsigned open_ctree_flags =OPEN_CTREE_PARTIAL ;u64 block_only =0 ;struct btrfs_root *tree_root_scan;u64 tree_id =0 ;convert/main.c +1
-1@@ -1140,7 +1140,7 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize, } root =open_ctree_fd (fd, devname, mkfs_cfg.super_bytenr, -OPEN_CTREE_WRITES |OPEN_CTREE_FS_PARTIAL ); +OPEN_CTREE_WRITES | OPEN_CTREE_TEMPORARY_SUPER);convert/main.c +1
-1if (!root) {error ("unable to open ctree" ); goto fail;@@ -1230,7 +1230,7 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize, } root =open_ctree_fd (fd, devname,0 , -OPEN_CTREE_WRITES |OPEN_CTREE_FS_PARTIAL ); +OPEN_CTREE_WRITES |OPEN_CTREE_TEMPORARY_SUPER );if (!root) {convert/main.c +0
-0error ("unable to open ctree for finalization" ); goto fail;ctree.h +0
-1@@ -45,10 +45,12 @@ struct btrfs_free_space_ctl;#define BTRFS_MAGIC 0x4D5F53665248425FULL/* ascii _BHRfS_M, no null */ /* -* Fake signature for an unfinalized filesystem, structures might be partially
- * created or missing.
+ * Fake signature for an unfinalized filesystem, which only has barebone tree
+ * structures (normally 6 near empty trees, on SINGLE meta/sys temporary chunks)
+ *
+ * ascii !BHRfS_M, no null
*/
-#defineBTRFS_MAGIC_PARTIAL0x4D5F536652484221ULL/* ascii !BHRfS_M, no null */ctree.h +1
-0+#define BTRFS_MAGIC_TEMPORARY 0x4D5F536652484221ULL#define BTRFS_MAX_MIRRORS 3disk-io.c +4
-4@@ -1117,14 +1117,14 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path, fs_info->ignore_chunk_tree_error = 1;if ((flags &OPEN_CTREE_RECOVER_SUPER ) - && (flags &OPEN_CTREE_FS_PARTIAL )) { + && (flags &OPEN_CTREE_TEMPORARY_SUPER )) {fprintf (stderr, -"cannot open a partially created filesystem for recovery"); +" cannot open a filesystem with temporary super block for recovery"); goto out; } -if (flags &OPEN_CTREE_FS_PARTIAL ) - sbflags =SBREAD_PARTIAL ; +if (flags &OPEN_CTREE_TEMPORARY_SUPER ) + sbflags =SBREAD_TEMPORARY ;ret = btrfs_scan_fs_devices(fp, path,
&fs_devices,sb_bytenr, sbflags,disk-io.c +2
-2(flags & OPEN_CTREE_NO_DEVICES));@@ -1285,8 +1285,8 @@ static int check_super(struct btrfs_super_block *sb, unsigned sbflags)int csum_size;if (btrfs_super_magic (sb) !=BTRFS_MAGIC ) { -if (btrfs_super_magic (sb) ==BTRFS_MAGIC_PARTIAL ) { -if (!(sbflags &SBREAD_PARTIAL )) { +if (btrfs_super_magic (sb) ==BTRFS_MAGIC_TEMPORARY ) { +if (!(sbflags &SBREAD_TEMPORARY )) { error("superblock magic doesn'tmatch");
disk-io.c +0
-0return -EIO ; }disk-io.h +3
-2@@ -73,8 +73,12 @@ enum btrfs_open_ctree_flags {*/ OPEN_CTREE_IGNORE_CHUNK_TREE_ERROR = (1U << 11), -/* Allow to open a partially created filesystem */ -OPEN_CTREE_FS_PARTIAL = (1U <<12 ), +/* +* Allow to open fs with temporary superblock (BTRFS_MAGIC_PARTIAL), +* such fs contains very basic tree layout, just able to be opened.
disk-io.h +3
-0+* Such temporary super is used for mkfs or convert. +*/ + OPEN_CTREE_TEMPORARY_SUPER = (1U << 12),/* * Invalidate the free space tree (i.e., clear the
FREE_SPACE_TREE_VALIDdisk-io.h +0
-0@@ -95,7 +99,7 @@ enum btrfs_read_sb_flags {* Read superblock with the fake signature, cannot be used with
disk-io.h +1
-1* SBREAD_RECOVER */ - SBREAD_PARTIAL = (1 << 1), + SBREAD_TEMPORARY = (1 << 1), };/* mkfs/main.c +1
-1@@ -1099,7 +1099,7 @@ int main(int argc, char **argv) } fs_info =open_ctree_fs_info (file,0 ,0 ,0 , -OPEN_CTREE_WRITES |OPEN_CTREE_FS_PARTIAL ); +OPEN_CTREE_WRITES |OPEN_CTREE_TEMPORARY_SUPER );if (!fs_info) {error ("open ctree failed" );goto error;