[PATCH 0/4] Fix for btrfs-convert chunk type and fsck support

[PATCH 0/4] Fix for btrfs-convert chunk type and fsck support

[Qu Wenruo]

Again the buggy btrfs-convert, even David tried to ban mixed-bg features
for btrfs-convert, it will still put data and metadata extents into the
same chunk, without marking the chunk mixed.

So in the patchset, first add fsck support for such problem, and then
force btrfs-convert to use mixed block group.

Lastly change the test case as they won't convert with non pagesize
nodesize.

Qu Wenruo (5):
  btrfs-progs: fsck: Add check for extent and parent chunk type
  btrfs: utils: Check nodesize against features
  btrfs: convert: force convert to used mixed block group
  btrfs: util: add parameter for btrfs_list_all_fs_features
  btrfs-progs: convert-test: Disable different nodesize test

 btrfs-convert.c        | 59 ++++++++++++++++++++++++++-----------------
 cmds-check.c           | 68 +++++++++++++++++++++++++++++++++++++++++++++++++-
 ctree.h                |  2 +-
 extent-tree.c          | 15 ++++++++++-
 mkfs.c                 | 19 ++++++--------
 tests/convert-tests.sh | 36 +++++++++++++++-----------
 utils.c                | 12 ++++++---
 utils.h                | 20 ++++++++++++---
 8 files changed, 171 insertions(+), 60 deletions(-)

-- 
2.5.1

[PATCH 1/5] btrfs-progs: fsck: Add check for extent and parent chunk type

[Qu Wenruo]

There is a bug in btrfs-convert in 4.1.2, even we don't allow mixed
block group for converted image, btrfs-convert will still create image
with data and metadata inside one chunk.

And further more, the chunk type is still DATA or METADATA, not
DATA|METADATA (not mixed).

So add btrfsck check for it right now.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 cmds-check.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 67 insertions(+), 1 deletion(-)

diff --git a/cmds-check.c b/cmds-check.c
index 0694a3b..c818bec 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -55,6 +55,7 @@ static int repair = 0;
 static int no_holes = 0;
 static int init_extent_tree = 0;
 static int check_data_csum = 0;
+static struct btrfs_fs_info *global_info;
 
 struct extent_backref {
 	struct list_head list;
@@ -127,6 +128,7 @@ struct extent_record {
 	unsigned int metadata:1;
 	unsigned int bad_full_backref:1;
 	unsigned int crossing_stripes:1;
+	unsigned int wrong_chunk_type:1;
 };
 
 struct inode_backref {
@@ -3749,7 +3751,8 @@ static int maybe_free_extent_rec(struct cache_tree *extent_cache,
 	if (rec->content_checked && rec->owner_ref_checked &&
 	    rec->extent_item_refs == rec->refs && rec->refs > 0 &&
 	    rec->num_duplicates == 0 && !all_backpointers_checked(rec, 0) &&
-	    !rec->bad_full_backref && !rec->crossing_stripes) {
+	    !rec->bad_full_backref && !rec->crossing_stripes &&
+	    !rec->wrong_chunk_type) {
 		remove_cache_extent(extent_cache, &rec->cache);
 		free_all_extent_backrefs(rec);
 		list_del_init(&rec->list);
@@ -4313,6 +4316,56 @@ static struct data_backref *alloc_data_backref(struct extent_record *rec,
 	return ref;
 }
 
+/* Check if the type of extent matches with its chunk */
+static void check_extent_type(struct extent_record *rec)
+{
+	struct btrfs_block_group_cache *bg_cache;
+
+	bg_cache = btrfs_lookup_first_block_group(global_info, rec->start);
+	if (!bg_cache)
+		return;
+
+	/* data extent, check chunk directly*/
+	if (!rec->metadata) {
+		if (!(bg_cache->flags & BTRFS_BLOCK_GROUP_DATA))
+			rec->wrong_chunk_type = 1;
+		return;
+	}
+
+	/* metadata extent, check the obvious case first */
+	if (!(bg_cache->flags & (BTRFS_BLOCK_GROUP_SYSTEM |
+				 BTRFS_BLOCK_GROUP_METADATA))) {
+		rec->wrong_chunk_type = 1;
+		return;
+	}
+
+	/*
+	 * Check SYSTEM extent, as it's also marked as metadata, we can only
+	 * make sure it's a SYSTEM extent by its backref
+	 */
+	if (!list_empty(&rec->backrefs)) {
+		struct extent_backref *node;
+		struct tree_backref *tback;
+		u64 bg_type;
+
+		node = list_entry(rec->backrefs.next, struct extent_backref,
+				  list);
+		if (node->is_data) {
+			/* tree block shouldn't have data backref */
+			rec->wrong_chunk_type = 1;
+			return;
+		}
+		tback = container_of(node, struct tree_backref, node);
+
+		if (tback->root == BTRFS_CHUNK_TREE_OBJECTID)
+			bg_type = BTRFS_BLOCK_GROUP_SYSTEM;
+		else
+			bg_type = BTRFS_BLOCK_GROUP_METADATA;
+		if (!(bg_cache->flags & bg_type))
+			rec->wrong_chunk_type = 1;
+	}
+}
+
 static int add_extent_rec(struct cache_tree *extent_cache,
 			  struct btrfs_key *parent_key, u64 parent_gen,
 			  u64 start, u64 nr, u64 extent_item_refs,
@@ -4405,6 +4458,7 @@ static int add_extent_rec(struct cache_tree *extent_cache,
 		if (metadata && check_crossing_stripes(rec->start,
 						       rec->max_size))
 				rec->crossing_stripes = 1;
+		check_extent_type(rec);
 		maybe_free_extent_rec(extent_cache, rec);
 		return ret;
 	}
@@ -4420,6 +4474,7 @@ static int add_extent_rec(struct cache_tree *extent_cache,
 	rec->flag_block_full_backref = -1;
 	rec->bad_full_backref = 0;
 	rec->crossing_stripes = 0;
+	rec->wrong_chunk_type = 0;
 	INIT_LIST_HEAD(&rec->backrefs);
 	INIT_LIST_HEAD(&rec->dups);
 	INIT_LIST_HEAD(&rec->list);
@@ -4462,6 +4517,7 @@ static int add_extent_rec(struct cache_tree *extent_cache,
 	if (metadata)
 		if (check_crossing_stripes(rec->start, rec->max_size))
 			rec->crossing_stripes = 1;
+	check_extent_type(rec);
 	return ret;
 }
 
@@ -4509,6 +4565,7 @@ static int add_tree_backref(struct cache_tree *extent_cache, u64 bytenr,
 		}
 		back->node.found_extent_tree = 1;
 	}
+	check_extent_type(rec);
 	maybe_free_extent_rec(extent_cache, rec);
 	return 0;
 }
@@ -7520,6 +7577,14 @@ static int check_extent_refs(struct btrfs_root *root,
 			cur_err = 1;
 		}
 
+		if (rec->wrong_chunk_type) {
+			fprintf(stderr,
+				"bad extent [%llu, %llu), type dismatch with chunk\n",
+				rec->start, rec->start + rec->max_size);
+			err = 1;
+			cur_err = 1;
+		}
+
 		remove_cache_extent(extent_cache, cache);
 		free_all_extent_backrefs(rec);
 		if (!init_extent_tree && repair && (!cur_err || fixed))
@@ -9378,6 +9443,7 @@ int cmd_check(int argc, char **argv)
 		goto err_out;
 	}
 
+	global_info = info;
 	root = info->fs_root;
 
 	/*
-- 
2.5.1

[PATCH 2/5] btrfs: utils: Check nodesize against features

[Qu Wenruo]

Check nodesize against features, not only sectorsize.
In fact, one of the btrfs-convert and mkfs differs in the nodesize
check.

This patch also provides the basis for later btrfs-convert fix.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 btrfs-convert.c |  2 +-
 mkfs.c          | 16 +++++-----------
 utils.c         |  8 +++++++-
 utils.h         |  2 +-
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/btrfs-convert.c b/btrfs-convert.c
index f4fc650..459b89a 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -2314,7 +2314,7 @@ static int do_convert(const char *devname, int datacsum, int packing, int noxatt
 		fprintf(stderr, "filetype feature is missing\n");
 		goto fail;
 	}
-	if (btrfs_check_nodesize(nodesize, blocksize))
+	if (btrfs_check_nodesize(nodesize, blocksize, features))
 		goto fail;
 	blocks_per_node = nodesize / blocksize;
 	ret = -blocks_per_node;
diff --git a/mkfs.c b/mkfs.c
index 14e7eb4..b8879fc 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1466,9 +1466,8 @@ int main(int ac, char **av)
 				print_usage(c != GETOPT_VAL_HELP);
 		}
 	}
+
 	sectorsize = max(sectorsize, (u32)sysconf(_SC_PAGESIZE));
-	if (btrfs_check_nodesize(nodesize, sectorsize))
-		exit(1);
 	saved_optind = optind;
 	dev_cnt = ac - optind;
 	if (dev_cnt == 0)
@@ -1542,17 +1541,12 @@ int main(int ac, char **av)
 			}
 		}
 
-		if (!nodesize_forced) {
+		if (!nodesize_forced)
 			nodesize = best_nodesize;
-			if (btrfs_check_nodesize(nodesize, sectorsize))
-				exit(1);
-		}
-		if (nodesize != sectorsize) {
-			fprintf(stderr, "Error: mixed metadata/data block groups "
-				"require metadata blocksizes equal to the sectorsize\n");
-			exit(1);
-		}
 	}
+	if (btrfs_check_nodesize(nodesize, sectorsize,
+				 features))
+		exit(1);
 
 	/* Check device/block_count after the nodesize is determined */
 	if (block_count && block_count < btrfs_min_dev_size(nodesize)) {
diff --git a/utils.c b/utils.c
index 52791b5..c0d1afa 100644
--- a/utils.c
+++ b/utils.c
@@ -2928,7 +2928,7 @@ int btrfs_tree_search2_ioctl_supported(int fd)
 	return v2_supported;
 }
 
-int btrfs_check_nodesize(u32 nodesize, u32 sectorsize)
+int btrfs_check_nodesize(u32 nodesize, u32 sectorsize, u64 features)
 {
 	if (nodesize < sectorsize) {
 		fprintf(stderr,
@@ -2945,6 +2945,12 @@ int btrfs_check_nodesize(u32 nodesize, u32 sectorsize)
 			"ERROR: Illegal nodesize %u (not aligned to %u)\n",
 			nodesize, sectorsize);
 		return -1;
+	} else if (features & BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS &&
+		   nodesize != sectorsize) {
+		fprintf(stderr,
+			"ERROR: Illegal nodesize %u (not equal to %u for mixed block group)\n",
+			nodesize, sectorsize);
+		return -1;
 	}
 	return 0;
 }
diff --git a/utils.h b/utils.h
index dce0a47..0eadaf1 100644
--- a/utils.h
+++ b/utils.h
@@ -241,7 +241,7 @@ static inline u64 div_factor(u64 num, int factor)
 }
 
 int btrfs_tree_search2_ioctl_supported(int fd);
-int btrfs_check_nodesize(u32 nodesize, u32 sectorsize);
+int btrfs_check_nodesize(u32 nodesize, u32 sectorsize, u64 features);
 
 const char *get_argv0_buf(void);
 
-- 
2.5.1

[PATCH 3/5] btrfs: convert: force convert to used mixed block group

[Qu Wenruo]

Convert has a bug that even we disable it from using mixed block, it
will still put data and metadata extent into the same chunk.

The bug can be reported by previous fsck patch.

Even without the bug, it's still high recommeneded to use mixed block
group, as unlike btrfs, ext* is not extent based from designed, so even
if we find a method to fix above bug, chunks will be quite scattered.

So force convert to use mixed block group right now.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 btrfs-convert.c | 55 +++++++++++++++++++++++++++++++++----------------------
 ctree.h         |  2 +-
 extent-tree.c   | 15 ++++++++++++++-
 utils.h         | 16 ++++++++++++++--
 4 files changed, 62 insertions(+), 26 deletions(-)

diff --git a/btrfs-convert.c b/btrfs-convert.c
index 459b89a..a60f380 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -1683,7 +1683,7 @@ static int create_subvol(struct btrfs_trans_handle *trans,
 	return 0;
 }
 
-static int init_btrfs(struct btrfs_root *root)
+static int init_btrfs(struct btrfs_root *root, u64 features)
 {
 	int ret;
 	struct btrfs_key location;
@@ -1693,7 +1693,7 @@ static int init_btrfs(struct btrfs_root *root)
 
 	trans = btrfs_start_transaction(root, 1);
 	BUG_ON(!trans);
-	ret = btrfs_make_block_groups(trans, root);
+	ret = btrfs_make_block_groups(trans, root, features);
 	if (ret)
 		goto err;
 	ret = btrfs_fix_block_accounting(trans, root);
@@ -2387,7 +2387,7 @@ static int do_convert(const char *devname, int datacsum, int packing, int noxatt
 			ext2_free_block_range(ext2_fs, blocks[i],
 					blocks_per_node);
 	}
-	ret = init_btrfs(root);
+	ret = init_btrfs(root, features);
 	if (ret) {
 		fprintf(stderr, "unable to setup the root tree\n");
 		goto fail;
@@ -2861,15 +2861,15 @@ int main(int argc, char *argv[])
 	int packing = 1;
 	int noxattr = 0;
 	int datacsum = 1;
-	u32 nodesize = max_t(u32, sysconf(_SC_PAGESIZE),
-			BTRFS_MKFS_DEFAULT_NODE_SIZE);
+	u32 nodesize = sysconf(_SC_PAGESIZE);
 	int rollback = 0;
 	int copylabel = 0;
 	int usage_error = 0;
 	int progress = 1;
 	char *file;
 	char fslabel[BTRFS_LABEL_SIZE];
-	u64 features = BTRFS_MKFS_DEFAULT_FEATURES;
+	u64 features = BTRFS_MKFS_DEFAULT_FEATURES |
+		       BTRFS_CONVERT_FORCE_FEATURES;
 
 	while(1) {
 		enum { GETOPT_VAL_NO_PROGRESS = 256 };
@@ -2937,22 +2937,6 @@ int main(int argc, char *argv[])
 					exit(1);
 				}
 				free(orig);
-				if (features & BTRFS_FEATURE_LIST_ALL) {
-					btrfs_list_all_fs_features(
-						~BTRFS_CONVERT_ALLOWED_FEATURES);
-					exit(0);
-				}
-				if (features & ~BTRFS_CONVERT_ALLOWED_FEATURES) {
-					char buf[64];
-
-					btrfs_parse_features_to_string(buf,
-						features & ~BTRFS_CONVERT_ALLOWED_FEATURES);
-					fprintf(stderr,
-						"ERROR: features not allowed for convert: %s\n",
-						buf);
-					exit(1);
-				}
-
 				break;
 				}
 			case GETOPT_VAL_NO_PROGRESS:
@@ -2964,6 +2948,12 @@ int main(int argc, char *argv[])
 				return c != GETOPT_VAL_HELP;
 		}
 	}
+	if (features & BTRFS_FEATURE_LIST_ALL) {
+		btrfs_list_all_fs_features(
+			~BTRFS_CONVERT_ALLOWED_FEATURES);
+		exit(0);
+	}
+
 	argc = argc - optind;
 	set_argv0(argv);
 	if (check_argc_exact(argc, 1)) {
@@ -2971,6 +2961,27 @@ int main(int argc, char *argv[])
 		return 1;
 	}
 
+	if (features & ~BTRFS_CONVERT_ALLOWED_FEATURES) {
+		char buf[64];
+
+		btrfs_parse_features_to_string(buf,
+			features & ~BTRFS_CONVERT_ALLOWED_FEATURES);
+		fprintf(stderr,
+			"ERROR: features not allowed for convert: %s\n",
+			buf);
+		exit(1);
+	}
+	if (!(features & BTRFS_CONVERT_FORCE_FEATURES)) {
+		char buf[64];
+
+		btrfs_parse_features_to_string(buf,
+				BTRFS_CONVERT_FORCE_FEATURES);
+		fprintf(stderr,
+			"ERROR: features %s must be enabled for convert\n",
+			buf);
+		exit(1);
+	}
+
 	if (rollback && (!datacsum || noxattr || !packing)) {
 		fprintf(stderr,
 			"Usage error: -d, -i, -n options do not apply to rollback\n");
diff --git a/ctree.h b/ctree.h
index bcad2b9..db39da7 100644
--- a/ctree.h
+++ b/ctree.h
@@ -2269,7 +2269,7 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans,
 			   u64 type, u64 chunk_objectid, u64 chunk_offset,
 			   u64 size);
 int btrfs_make_block_groups(struct btrfs_trans_handle *trans,
-			    struct btrfs_root *root);
+			    struct btrfs_root *root, u64 features);
 int btrfs_update_block_group(struct btrfs_trans_handle *trans,
 			     struct btrfs_root *root, u64 bytenr, u64 num,
 			     int alloc, int mark_free);
diff --git a/extent-tree.c b/extent-tree.c
index 0c8152a..6c0960d 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -3373,7 +3373,7 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans,
  * before doing any block allocation.
  */
 int btrfs_make_block_groups(struct btrfs_trans_handle *trans,
-			    struct btrfs_root *root)
+			    struct btrfs_root *root, u64 features)
 {
 	u64 total_bytes;
 	u64 cur_start;
@@ -3406,7 +3406,20 @@ int btrfs_make_block_groups(struct btrfs_trans_handle *trans,
 			group_size &= ~(group_align - 1);
 			group_size = max_t(u64, group_size, 8 * 1024 * 1024);
 			group_size = min_t(u64, group_size, 32 * 1024 * 1024);
+		} else if (features & BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS) {
+			/* mixed block group case */
+
+			if ((total_bytes - cur_start) * 4 < group_size * 5)
+				group_size = total_bytes - cur_start;
+			group_size = round_down(group_size, group_align);
+			group_type = BTRFS_BLOCK_GROUP_METADATA |
+				     BTRFS_BLOCK_GROUP_DATA;
+			group_size = min_t(u64, group_size,
+					   1ULL * 1024 * 1024 * 1024);
+			total_metadata += group_size;
+			total_data += group_size;
 		} else {
+			/* no mixed block group case, not used yet */
 			group_size &= ~(group_align - 1);
 			if (total_data >= total_metadata * 2) {
 				group_type = BTRFS_BLOCK_GROUP_METADATA;
diff --git a/utils.h b/utils.h
index 0eadaf1..7f1a128 100644
--- a/utils.h
+++ b/utils.h
@@ -31,7 +31,7 @@
 		| BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA)
 
 /*
- * Avoid multi-device features (RAID56) and mixed block groups
+ * Avoid multi-device features (RAID56)
  */
 #define BTRFS_CONVERT_ALLOWED_FEATURES				\
 	(BTRFS_FEATURE_INCOMPAT_MIXED_BACKREF			\
@@ -41,7 +41,19 @@
 	| BTRFS_FEATURE_INCOMPAT_BIG_METADATA			\
 	| BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF			\
 	| BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA		\
-	| BTRFS_FEATURE_INCOMPAT_NO_HOLES)
+	| BTRFS_FEATURE_INCOMPAT_NO_HOLES			\
+	| BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS)
+
+/*
+ * Due to bugs in btrfs-convert, it will put data and metadata
+ * extents into the same chunk, even we disallow mixed block group.
+ *
+ * Also, ext* is not as extent based as btrfs, even if we find a method
+ * to fix above bug, it will cause chunks to be very scattered.
+ * So force mixed block group for now.
+ */
+#define BTRFS_CONVERT_FORCE_FEATURES				\
+	(BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS)
 
 #define BTRFS_FEATURE_LIST_ALL		(1ULL << 63)
 
-- 
2.5.1

[PATCH 4/5] btrfs: util: add parameter for btrfs_list_all_fs_features

[Qu Wenruo]

Even we tried to make mkfs and btrfs-convert use the same features, but
previous patch forced btrfs-convert to use mix-bg feature.

So the default fs features is different for mkfs and btrfs-convert, add
mask_default parameter for btrfs_list_all_fs_features() to handle the
difference.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 btrfs-convert.c | 4 +++-
 mkfs.c          | 3 ++-
 utils.c         | 4 ++--
 utils.h         | 2 +-
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/btrfs-convert.c b/btrfs-convert.c
index a60f380..e730e4b 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -2950,7 +2950,9 @@ int main(int argc, char *argv[])
 	}
 	if (features & BTRFS_FEATURE_LIST_ALL) {
 		btrfs_list_all_fs_features(
-			~BTRFS_CONVERT_ALLOWED_FEATURES);
+			~BTRFS_CONVERT_ALLOWED_FEATURES,
+			BTRFS_MKFS_DEFAULT_FEATURES |
+			BTRFS_CONVERT_FORCE_FEATURES);
 		exit(0);
 	}
 
diff --git a/mkfs.c b/mkfs.c
index b8879fc..56ce381 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1430,7 +1430,8 @@ int main(int ac, char **av)
 				}
 				free(orig);
 				if (features & BTRFS_FEATURE_LIST_ALL) {
-					btrfs_list_all_fs_features(0);
+					btrfs_list_all_fs_features(0,
+						BTRFS_MKFS_DEFAULT_FEATURES);
 					exit(0);
 				}
 				break;
diff --git a/utils.c b/utils.c
index c0d1afa..78ba67b 100644
--- a/utils.c
+++ b/utils.c
@@ -633,7 +633,7 @@ void btrfs_process_fs_features(u64 flags)
 	}
 }
 
-void btrfs_list_all_fs_features(u64 mask_disallowed)
+void btrfs_list_all_fs_features(u64 mask_disallowed, u64 mask_default)
 {
 	int i;
 
@@ -643,7 +643,7 @@ void btrfs_list_all_fs_features(u64 mask_disallowed)
 
 		if (mkfs_features[i].flag & mask_disallowed)
 			continue;
-		if (mkfs_features[i].flag & BTRFS_MKFS_DEFAULT_FEATURES)
+		if (mkfs_features[i].flag & mask_default)
 			is_default = ", default";
 		fprintf(stderr, "%-20s- %s (0x%llx%s)\n",
 				mkfs_features[i].name,
diff --git a/utils.h b/utils.h
index 7f1a128..f8fd745 100644
--- a/utils.h
+++ b/utils.h
@@ -112,7 +112,7 @@ void set_argv0(char **argv);
 void units_set_mode(unsigned *units, unsigned mode);
 void units_set_base(unsigned *units, unsigned base);
 
-void btrfs_list_all_fs_features(u64 mask_disallowed);
+void btrfs_list_all_fs_features(u64 mask_disallowed, u64 mask_default);
 char* btrfs_parse_fs_features(char *namelist, u64 *flags);
 void btrfs_process_fs_features(u64 flags);
 void btrfs_parse_features_to_string(char *buf, u64 flags);
-- 
2.5.1