btrfs-progs: image: Fix error when restoring multi-disk image to single disk

Current fixup_devices() will only remove DEV_ITEMs and reset DEV_ITEM size.
Later we need to do more fixup works, so change the name to fixup_chunks_and_devices() and refactor the original device size fixup to fixup_device_size().

Signed-off-by Qu Wenruo <[email protected]>

---
 image/main.c | 52 ++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 16 deletions(-)

image/main.c +0 -0

index c680ab19de6c..36b5c95ea146 100644

image/main.c +34 -14

+			     struct mdrestore_struct *mdres,
+			     off_t dev_size)
 {
-	struct btrfs_trans_handle *trans;
 	struct btrfs_dev_item *dev_item;
 	struct btrfs_path path;
-	struct extent_buffer *leaf;
 	struct btrfs_root *root = fs_info->chunk_root;
 	struct btrfs_key key;
+	struct extent_buffer *leaf;
 	u64 devid, cur_devid;
 	int ret;
 
-	if (btrfs_super_log_root(fs_info->super_copy)) {
-		warning(
-		"log tree detected, its generation will not match superblock");
-	}
-	trans = btrfs_start_transaction(fs_info->tree_root, 1);
-	if (IS_ERR(trans)) {
-		error("cannot starting transaction %ld", PTR_ERR(trans));
-		return PTR_ERR(trans);
-	}
-
 	dev_item = &fs_info->super_copy->dev_item;
 
 	devid = btrfs_stack_device_id(dev_item);

@@ -2123,7 +2114,7 @@ again:
 	ret = btrfs_search_slot(trans, root, &key, &path, -1, 1);
 	if (ret < 0) {
 		error("search failed: %d", ret);
-		exit(1);
+		return ret;
 	}
 
 	while (1) {

@@ -2170,12 +2161,41 @@ again:
 	}
 
 	btrfs_release_path(&path);
+	return 0;
+}
+
+static int fixup_chunks_and_devices(struct btrfs_fs_info *fs_info,
+			 struct mdrestore_struct *mdres, off_t dev_size)
+{
+	struct btrfs_trans_handle *trans;
+	int ret;
+
+	if (btrfs_super_log_root(fs_info->super_copy)) {
+		warning(
+		"log tree detected, its generation will not match superblock");
+	}
+	trans = btrfs_start_transaction(fs_info->tree_root, 1);
+	if (IS_ERR(trans)) {
+		error("cannot starting transaction %ld", PTR_ERR(trans));
+		return PTR_ERR(trans);
+	}
+
+	ret = fixup_device_size(trans, fs_info, mdres, dev_size);
+	if (ret < 0)
+		goto error;
+
 	ret = btrfs_commit_transaction(trans, fs_info->tree_root);
 	if (ret) {
 		error("unable to commit transaction: %d", ret);
 		return ret;
 	}
 	return 0;
+error:
+	error(
+"failed to fix chunks and devices mapping, the fs may not be mountable: %s",
+		strerror(-ret));
+	btrfs_abort_transaction(trans, ret);
+	return ret;
 }
 
 static int restore_metadump(const char *input, FILE *out, int old_restore,

@@ -2282,7 +2302,7 @@ static int restore_metadump(const char *input, FILE *out, int old_restore,
 			return 1;
 		}
 
-		ret = fixup_devices(info, &mdrestore, st.st_size);
+		ret = fixup_chunks_and_devices(info, &mdrestore, st.st_size);
 		close_ctree(info->chunk_root);
 		if (ret)
 			goto out;

Quoted 19 lines · Qu Wenruo

Current fixup_devices() will only remove DEV_ITEMs and reset DEV_ITEM size.
Later we need to do more fixup works, so change the name to fixup_chunks_and_devices() and refactor the original device size fixup to fixup_device_size().

Signed-off-by Qu Wenruo <[email protected]>

---
 image/main.c | 52 ++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 16 deletions(-)

image/main.c +0 -0

index c680ab19de6c..36b5c95ea146 100644
@@ -2084,28 +2084,19 @@ static void remap_overlapping_chunks(struct mdrestore_struct *mdres)
 	}
 }
 

image/main.c static void remap_overlapping_chunks(struct mdrestore_struct *mdres) · line 2088

Quoted 4 lines

image/main.c +2 -2

-static int fixup_devices(struct btrfs_fs_info *fs_info,
-			 struct mdrestore_struct *mdres, off_t dev_size)
+static int fixup_device_size(struct btrfs_trans_handle *trans,
+			     struct btrfs_fs_info *fs_info,

trans already has a handle to the fs_info so you can drop it from the param list.

Quoted 88 lines

image/main.c +34 -14

+			     struct mdrestore_struct *mdres,
+			     off_t dev_size)
 {
-	struct btrfs_trans_handle *trans;
 	struct btrfs_dev_item *dev_item;
 	struct btrfs_path path;
-	struct extent_buffer *leaf;
 	struct btrfs_root *root = fs_info->chunk_root;
 	struct btrfs_key key;
+	struct extent_buffer *leaf;
 	u64 devid, cur_devid;
 	int ret;
 
-	if (btrfs_super_log_root(fs_info->super_copy)) {
-		warning(
-		"log tree detected, its generation will not match superblock");
-	}
-	trans = btrfs_start_transaction(fs_info->tree_root, 1);
-	if (IS_ERR(trans)) {
-		error("cannot starting transaction %ld", PTR_ERR(trans));
-		return PTR_ERR(trans);
-	}
-
 	dev_item = &fs_info->super_copy->dev_item;
 
 	devid = btrfs_stack_device_id(dev_item);

@@ -2123,7 +2114,7 @@ again:
 	ret = btrfs_search_slot(trans, root, &key, &path, -1, 1);
 	if (ret < 0) {
 		error("search failed: %d", ret);
-		exit(1);
+		return ret;
 	}
 
 	while (1) {

@@ -2170,12 +2161,41 @@ again:
 	}
 
 	btrfs_release_path(&path);
+	return 0;
+}
+
+static int fixup_chunks_and_devices(struct btrfs_fs_info *fs_info,
+			 struct mdrestore_struct *mdres, off_t dev_size)
+{
+	struct btrfs_trans_handle *trans;
+	int ret;
+
+	if (btrfs_super_log_root(fs_info->super_copy)) {
+		warning(
+		"log tree detected, its generation will not match superblock");
+	}
+	trans = btrfs_start_transaction(fs_info->tree_root, 1);
+	if (IS_ERR(trans)) {
+		error("cannot starting transaction %ld", PTR_ERR(trans));
+		return PTR_ERR(trans);
+	}
+
+	ret = fixup_device_size(trans, fs_info, mdres, dev_size);
+	if (ret < 0)
+		goto error;
+
 	ret = btrfs_commit_transaction(trans, fs_info->tree_root);
 	if (ret) {
 		error("unable to commit transaction: %d", ret);
 		return ret;
 	}
 	return 0;
+error:
+	error(
+"failed to fix chunks and devices mapping, the fs may not be mountable: %s",
+		strerror(-ret));
+	btrfs_abort_transaction(trans, ret);
+	return ret;
 }
 
 static int restore_metadump(const char *input, FILE *out, int old_restore,

@@ -2282,7 +2302,7 @@ static int restore_metadump(const char *input, FILE *out, int old_restore,
 			return 1;
 		}
 
-		ret = fixup_devices(info, &mdrestore, st.st_size);
+		ret = fixup_chunks_and_devices(info, &mdrestore, st.st_size);
 		close_ctree(info->chunk_root);
 		if (ret)
 			goto out;

Since btrfs-image is just restoring tree blocks without really check if that tree block contents makes sense, for multi-device image, block group items will keep that incorrect block group flags.

For example, for a metadata RAID1 data RAID0 btrfs recovered to a single disk, its chunk tree will look like:

	item 1 key (FIRST_CHUNK_TREE CHUNK_ITEM 22020096)
		length 8388608 owner 2 stripe_len 65536 type SYSTEM
	item 2 key (FIRST_CHUNK_TREE CHUNK_ITEM 30408704)
		length 1073741824 owner 2 stripe_len 65536 type METADATA
	item 3 key (FIRST_CHUNK_TREE CHUNK_ITEM 1104150528)
		length 1073741824 owner 2 stripe_len 65536 type DATA

All chunks have correct type (SINGLE).

While its block group items will look like:

	item 1 key (22020096 BLOCK_GROUP_ITEM 8388608)
		block group used 16384 chunk_objectid 256 flags SYSTEM|RAID1
	item 3 key (30408704 BLOCK_GROUP_ITEM 1073741824)
		block group used 114688 chunk_objectid 256 flags METADATA|RAID1
	item 11 key (1104150528 BLOCK_GROUP_ITEM 1073741824)
		block group used 1572864 chunk_objectid 256 flags DATA|RAID0

All block group items still have the wrong profiles.

And btrfs check (lowmem mode for better output) will report error for such image:

ERROR: chunk[22020096 30408704) related block group item flags mismatch, wanted: 2, have: 18
ERROR: chunk[30408704 1104150528) related block group item flags mismatch, wanted: 4, have: 20
ERROR: chunk[1104150528 2177892352) related block group item flags mismatch, wanted: 1, have: 9

This patch will do an extra repair for block group items to fix the profile of them.

Signed-off-by Qu Wenruo <[email protected]>

---
 image/main.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

image/main.c +0 -0

index 36b5c95ea146..9060f6b1b665 100644

image/main.c +45 -0

+{
+	struct btrfs_block_group_cache *bg;
+	struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree;
+	struct cache_extent *ce;
+	struct map_lookup *map;
+	u64 extra_flags;
+
+	for (ce = search_cache_extent(&map_tree->cache_tree, 0); ce;
+	     ce = next_cache_extent(ce)) {
+		map = container_of(ce, struct map_lookup, ce);
+
+		bg = btrfs_lookup_block_group(fs_info, ce->start);
+		if (!bg) {
+			warning(
+		"can't find block group %llu, result fs may not be mountable",
+				ce->start);
+			continue;
+		}
+		extra_flags = map->type & BTRFS_BLOCK_GROUP_PROFILE_MASK;
+
+		if (bg->flags == map->type)
+			continue;
+
+		/* Update the block group item and mark the bg dirty */
+		bg->flags = map->type;
+		btrfs_set_block_group_flags(&bg->item, bg->flags);
+		set_extent_bits(&fs_info->block_group_cache, ce->start,
+				ce->start + ce->size - 1, BLOCK_GROUP_DIRTY);
+
+		/*
+		 * Chunk and bg flags can be different, changing bg flags
+		 * without update avail_data/meta_alloc_bits will lead to
+		 * ENOSPC.
+		 * So here we set avail_*_alloc_bits to match chunk types.
+		 */
+		if (map->type & BTRFS_BLOCK_GROUP_DATA)
+			fs_info->avail_data_alloc_bits = extra_flags;
+		if (map->type & BTRFS_BLOCK_GROUP_METADATA)
+			fs_info->avail_metadata_alloc_bits = extra_flags;
+		if (map->type & BTRFS_BLOCK_GROUP_SYSTEM)
+			fs_info->avail_system_alloc_bits = extra_flags;
+	}
+}
+
 static int fixup_chunks_and_devices(struct btrfs_fs_info *fs_info,
 			 struct mdrestore_struct *mdres, off_t dev_size)
 {

@@ -2180,6 +2226,7 @@ static int fixup_chunks_and_devices(struct btrfs_fs_info *fs_info,
 		return PTR_ERR(trans);
 	}
 
+	fixup_block_groups(trans, fs_info);
 	ret = fixup_device_size(trans, fs_info, mdres, dev_size);
 	if (ret < 0)
 		goto error;

Quoted 49 lines · Qu Wenruo

Since btrfs-image is just restoring tree blocks without really check if that tree block contents makes sense, for multi-device image, block group items will keep that incorrect block group flags.

For example, for a metadata RAID1 data RAID0 btrfs recovered to a single disk, its chunk tree will look like:

	item 1 key (FIRST_CHUNK_TREE CHUNK_ITEM 22020096)
		length 8388608 owner 2 stripe_len 65536 type SYSTEM
	item 2 key (FIRST_CHUNK_TREE CHUNK_ITEM 30408704)
		length 1073741824 owner 2 stripe_len 65536 type METADATA
	item 3 key (FIRST_CHUNK_TREE CHUNK_ITEM 1104150528)
		length 1073741824 owner 2 stripe_len 65536 type DATA

All chunks have correct type (SINGLE).

While its block group items will look like:

	item 1 key (22020096 BLOCK_GROUP_ITEM 8388608)
		block group used 16384 chunk_objectid 256 flags SYSTEM|RAID1
	item 3 key (30408704 BLOCK_GROUP_ITEM 1073741824)
		block group used 114688 chunk_objectid 256 flags METADATA|RAID1
	item 11 key (1104150528 BLOCK_GROUP_ITEM 1073741824)
		block group used 1572864 chunk_objectid 256 flags DATA|RAID0

All block group items still have the wrong profiles.

And btrfs check (lowmem mode for better output) will report error for such image:

ERROR: chunk[22020096 30408704) related block group item flags mismatch, wanted: 2, have: 18
ERROR: chunk[30408704 1104150528) related block group item flags mismatch, wanted: 4, have: 20
ERROR: chunk[1104150528 2177892352) related block group item flags mismatch, wanted: 1, have: 9

This patch will do an extra repair for block group items to fix the profile of them.

Signed-off-by Qu Wenruo <[email protected]>

---
 image/main.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

image/main.c +0 -0

index 36b5c95ea146..9060f6b1b665 100644
@@ -2164,6 +2164,52 @@ again:
 	return 0;
 }
 

image/main.c again: · line 2168

Quoted 2 lines

image/main.c +2 -0

+static void fixup_block_groups(struct btrfs_trans_handle *trans,
+			      struct btrfs_fs_info *fs_info)

You are not even using the trans handle in this function, why pass it?

Quoted 57 lines

image/main.c +45 -0

+{
+	struct btrfs_block_group_cache *bg;
+	struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree;
+	struct cache_extent *ce;
+	struct map_lookup *map;
+	u64 extra_flags;
+
+	for (ce = search_cache_extent(&map_tree->cache_tree, 0); ce;
+	     ce = next_cache_extent(ce)) {
+		map = container_of(ce, struct map_lookup, ce);
+
+		bg = btrfs_lookup_block_group(fs_info, ce->start);
+		if (!bg) {
+			warning(
+		"can't find block group %llu, result fs may not be mountable",
+				ce->start);
+			continue;
+		}
+		extra_flags = map->type & BTRFS_BLOCK_GROUP_PROFILE_MASK;
+
+		if (bg->flags == map->type)
+			continue;
+
+		/* Update the block group item and mark the bg dirty */
+		bg->flags = map->type;
+		btrfs_set_block_group_flags(&bg->item, bg->flags);
+		set_extent_bits(&fs_info->block_group_cache, ce->start,
+				ce->start + ce->size - 1, BLOCK_GROUP_DIRTY);
+
+		/*
+		 * Chunk and bg flags can be different, changing bg flags
+		 * without update avail_data/meta_alloc_bits will lead to
+		 * ENOSPC.
+		 * So here we set avail_*_alloc_bits to match chunk types.
+		 */
+		if (map->type & BTRFS_BLOCK_GROUP_DATA)
+			fs_info->avail_data_alloc_bits = extra_flags;
+		if (map->type & BTRFS_BLOCK_GROUP_METADATA)
+			fs_info->avail_metadata_alloc_bits = extra_flags;
+		if (map->type & BTRFS_BLOCK_GROUP_SYSTEM)
+			fs_info->avail_system_alloc_bits = extra_flags;
+	}
+}
+
 static int fixup_chunks_and_devices(struct btrfs_fs_info *fs_info,
 			 struct mdrestore_struct *mdres, off_t dev_size)
 {

@@ -2180,6 +2226,7 @@ static int fixup_chunks_and_devices(struct btrfs_fs_info *fs_info,
 		return PTR_ERR(trans);
 	}
 
+	fixup_block_groups(trans, fs_info);
 	ret = fixup_device_size(trans, fs_info, mdres, dev_size);
 	if (ret < 0)
 		goto error;

This patch will remove all existing dev extents for later rebuild.

Signed-off-by Qu Wenruo <[email protected]>

---
 image/main.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

image/main.c +0 -0

index 9060f6b1b665..707568f22e01 100644

image/main.c +14 -0

+{
+	int ret;
+
+	ret = remove_all_dev_extents(trans, fs_info);
+	if (ret < 0)
+		error("failed to remove all existing dev extents: %s",
+			strerror(-ret));
+	return ret;
+}
+
 static int fixup_chunks_and_devices(struct btrfs_fs_info *fs_info,
 			 struct mdrestore_struct *mdres, off_t dev_size)
 {

@@ -2227,6 +2291,10 @@ static int fixup_chunks_and_devices(struct btrfs_fs_info *fs_info,
 	}
 
 	fixup_block_groups(trans, fs_info);
+	ret = fixup_dev_extents(trans, fs_info);
+	if (ret < 0)
+		goto error;
+
 	ret = fixup_device_size(trans, fs_info, mdres, dev_size);
 	if (ret < 0)
 		goto error;

With existing dev extents cleaned up, now we can rebuild dev extents using the correct chunk tree.

Since new dev extents are all rebuild from scratch, even we're restoring image from multi-device fs to single disk, we won't have any problem reported by btrfs check.

Signed-off-by Qu Wenruo <[email protected]>

---
 image/main.c | 34 ++++++++++++++++++++++++++++++++++
 volumes.c    | 10 +++++-----
 volumes.h    |  4 ++++
 3 files changed, 43 insertions(+), 5 deletions(-)

image/main.c +34 -0

index 707568f22e01..626eb933d5cc 100644
@@ -2265,12 +2265,46 @@ out:
 static int fixup_dev_extents(struct btrfs_trans_handle *trans,
 			     struct btrfs_fs_info *fs_info)
 {
+	struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree;
+	struct btrfs_device *dev;
+	struct cache_extent *ce;
+	struct map_lookup *map;
+	u64 devid = btrfs_stack_device_id(&fs_info->super_copy->dev_item);
+	int i;
 	int ret;
 
 	ret = remove_all_dev_extents(trans, fs_info);
 	if (ret < 0)
 		error("failed to remove all existing dev extents: %s",
 			strerror(-ret));
+
+	dev = btrfs_find_device(fs_info, devid, NULL, NULL);
+	if (!dev) {
+		error("faild to find devid %llu", devid);
+		return -ENODEV;
+	}
+
+	/* Rebuild all dev extents using chunk maps */
+	for (ce = search_cache_extent(&map_tree->cache_tree, 0); ce;
+	     ce = next_cache_extent(ce)) {
+		u64 stripe_len;
+
+		map = container_of(ce, struct map_lookup, ce);
+		stripe_len = calc_stripe_length(map->type, ce->size,
+						map->num_stripes);
+		for (i = 0; i < map->num_stripes; i++) {
+			ret = btrfs_alloc_dev_extent(trans, dev, ce->start,
+				stripe_len, &map->stripes[i].physical, 1);
+			if (ret < 0) {
+				error(
+				"failed to insert dev extent %llu %llu: %s",
+					devid, map->stripes[i].physical,
+					strerror(-ret));
+				goto out;
+			}
+		}
+	}
+out:
 	return ret;
 }
 

volumes.c +0 -0

index 30090ce5f8e8..73c9204fa7d1 100644
@@ -530,10 +530,10 @@ static int find_free_dev_extent(struct btrfs_device *device, u64 num_bytes,
 	return find_free_dev_extent_start(device, num_bytes, 0, start, len);

volumes.c +1 -1

 {
 	int ret;
 	struct btrfs_path *path;

@@ -550,7 +550,7 @@ static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
 	 * For convert case, just skip search free dev_extent, as caller
 	 * is responsible to make sure it's free.
 	 */
-	if (!convert) {
+	if (!insert_asis) {
 		ret = find_free_dev_extent(device, num_bytes, start, NULL);
 		if (ret)
 			goto err;

volumes.h +4 -0

index b4ea93f0bec3..5ca2779ebd45 100644
@@ -271,6 +271,10 @@ void btrfs_close_all_devices(void);
 int btrfs_add_device(struct btrfs_trans_handle *trans,
 		     struct btrfs_fs_info *fs_info,
 		     struct btrfs_device *device);
+int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
+			   struct btrfs_device *device,
+			   u64 chunk_offset, u64 num_bytes, u64 *start,
+			   int insert_asis);
 int btrfs_update_device(struct btrfs_trans_handle *trans,
 			struct btrfs_device *device);
 int btrfs_scan_one_device(int fd, const char *path,

Quoted 73 lines · Qu Wenruo

With existing dev extents cleaned up, now we can rebuild dev extents using the correct chunk tree.

Since new dev extents are all rebuild from scratch, even we're restoring image from multi-device fs to single disk, we won't have any problem reported by btrfs check.

Signed-off-by Qu Wenruo <[email protected]>

---
 image/main.c | 34 ++++++++++++++++++++++++++++++++++
 volumes.c    | 10 +++++-----
 volumes.h    |  4 ++++
 3 files changed, 43 insertions(+), 5 deletions(-)

image/main.c +34 -0

index 707568f22e01..626eb933d5cc 100644
@@ -2265,12 +2265,46 @@ out:
 static int fixup_dev_extents(struct btrfs_trans_handle *trans,
 			     struct btrfs_fs_info *fs_info)
 {
+	struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree;
+	struct btrfs_device *dev;
+	struct cache_extent *ce;
+	struct map_lookup *map;
+	u64 devid = btrfs_stack_device_id(&fs_info->super_copy->dev_item);
+	int i;
 	int ret;
 
 	ret = remove_all_dev_extents(trans, fs_info);
 	if (ret < 0)
 		error("failed to remove all existing dev extents: %s",
 			strerror(-ret));
+
+	dev = btrfs_find_device(fs_info, devid, NULL, NULL);
+	if (!dev) {
+		error("faild to find devid %llu", devid);
+		return -ENODEV;
+	}
+
+	/* Rebuild all dev extents using chunk maps */
+	for (ce = search_cache_extent(&map_tree->cache_tree, 0); ce;
+	     ce = next_cache_extent(ce)) {
+		u64 stripe_len;
+
+		map = container_of(ce, struct map_lookup, ce);
+		stripe_len = calc_stripe_length(map->type, ce->size,
+						map->num_stripes);
+		for (i = 0; i < map->num_stripes; i++) {
+			ret = btrfs_alloc_dev_extent(trans, dev, ce->start,
+				stripe_len, &map->stripes[i].physical, 1);
+			if (ret < 0) {
+				error(
+				"failed to insert dev extent %llu %llu: %s",
+					devid, map->stripes[i].physical,
+					strerror(-ret));
+				goto out;
+			}
+		}
+	}
+out:
 	return ret;
 }
 

volumes.c +0 -0

index 30090ce5f8e8..73c9204fa7d1 100644
@@ -530,10 +530,10 @@ static int find_free_dev_extent(struct btrfs_device *device, u64 num_bytes,
 	return find_free_dev_extent_start(device, num_bytes, 0, start, len);
 }
 

volumes.c static int find_free_dev_extent(struct btrfs_device *device, u64 num_bytes, · line 536

Quoted 8 lines

volumes.c +4 -4

-static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
-				  struct btrfs_device *device,
-				  u64 chunk_offset, u64 num_bytes, u64 *start,
-				  int convert)
+int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
+			   struct btrfs_device *device,
+			   u64 chunk_offset, u64 num_bytes, u64 *start,
+			   int insert_asis)

Make that parameter a bool. Also why do you rename it ?

Quoted 29 lines

volumes.c +1 -1

 {
 	int ret;
 	struct btrfs_path *path;

@@ -550,7 +550,7 @@ static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
 	 * For convert case, just skip search free dev_extent, as caller
 	 * is responsible to make sure it's free.
 	 */
-	if (!convert) {
+	if (!insert_asis) {
 		ret = find_free_dev_extent(device, num_bytes, start, NULL);
 		if (ret)
 			goto err;

volumes.h +4 -0

index b4ea93f0bec3..5ca2779ebd45 100644
@@ -271,6 +271,10 @@ void btrfs_close_all_devices(void);
 int btrfs_add_device(struct btrfs_trans_handle *trans,
 		     struct btrfs_fs_info *fs_info,
 		     struct btrfs_device *device);
+int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
+			   struct btrfs_device *device,
+			   u64 chunk_offset, u64 num_bytes, u64 *start,
+			   int insert_asis);
 int btrfs_update_device(struct btrfs_trans_handle *trans,
 			struct btrfs_device *device);
 int btrfs_scan_one_device(int fd, const char *path,

Quoted 76 lines · Nikolay Borisov

Quoted 73 lines · Qu Wenruo

With existing dev extents cleaned up, now we can rebuild dev extents using the correct chunk tree.

Since new dev extents are all rebuild from scratch, even we're restoring image from multi-device fs to single disk, we won't have any problem reported by btrfs check.

Signed-off-by Qu Wenruo <[email protected]>

---
 image/main.c | 34 ++++++++++++++++++++++++++++++++++
 volumes.c    | 10 +++++-----
 volumes.h    |  4 ++++
 3 files changed, 43 insertions(+), 5 deletions(-)

image/main.c +34 -0

index 707568f22e01..626eb933d5cc 100644
@@ -2265,12 +2265,46 @@ out:
 static int fixup_dev_extents(struct btrfs_trans_handle *trans,
 			     struct btrfs_fs_info *fs_info)
 {
+	struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree;
+	struct btrfs_device *dev;
+	struct cache_extent *ce;
+	struct map_lookup *map;
+	u64 devid = btrfs_stack_device_id(&fs_info->super_copy->dev_item);
+	int i;
 	int ret;
 
 	ret = remove_all_dev_extents(trans, fs_info);
 	if (ret < 0)
 		error("failed to remove all existing dev extents: %s",
 			strerror(-ret));
+
+	dev = btrfs_find_device(fs_info, devid, NULL, NULL);
+	if (!dev) {
+		error("faild to find devid %llu", devid);
+		return -ENODEV;
+	}
+
+	/* Rebuild all dev extents using chunk maps */
+	for (ce = search_cache_extent(&map_tree->cache_tree, 0); ce;
+	     ce = next_cache_extent(ce)) {
+		u64 stripe_len;
+
+		map = container_of(ce, struct map_lookup, ce);
+		stripe_len = calc_stripe_length(map->type, ce->size,
+						map->num_stripes);
+		for (i = 0; i < map->num_stripes; i++) {
+			ret = btrfs_alloc_dev_extent(trans, dev, ce->start,
+				stripe_len, &map->stripes[i].physical, 1);
+			if (ret < 0) {
+				error(
+				"failed to insert dev extent %llu %llu: %s",
+					devid, map->stripes[i].physical,
+					strerror(-ret));
+				goto out;
+			}
+		}
+	}
+out:
 	return ret;
 }
 

volumes.c +0 -0

index 30090ce5f8e8..73c9204fa7d1 100644
@@ -530,10 +530,10 @@ static int find_free_dev_extent(struct btrfs_device *device, u64 num_bytes,
 	return find_free_dev_extent_start(device, num_bytes, 0, start, len);
 }
 

Quoted 10 lines

volumes.c static int find_free_dev_extent(struct btrfs_device *device, u64 num_bytes, · line 536

Quoted 8 lines

volumes.c +4 -4

-static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
-				  struct btrfs_device *device,
-				  u64 chunk_offset, u64 num_bytes, u64 *start,
-				  int convert)
+int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
+			   struct btrfs_device *device,
+			   u64 chunk_offset, u64 num_bytes, u64 *start,
+			   int insert_asis)

Make that parameter a bool. Also why do you rename it ?

Since it's no longer only used by convert.

The best naming may be two function, one called btrfs_insert_device_extent(), and then btrfs_alloc_device_extent().

As for convert and this use case, we are not allocating, but just inserting one.

What about above naming change?

Thanks,
Qu

Quoted 29 lines

Quoted 29 lines

volumes.c +1 -1

 {
 	int ret;
 	struct btrfs_path *path;

@@ -550,7 +550,7 @@ static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
 	 * For convert case, just skip search free dev_extent, as caller
 	 * is responsible to make sure it's free.
 	 */
-	if (!convert) {
+	if (!insert_asis) {
 		ret = find_free_dev_extent(device, num_bytes, start, NULL);
 		if (ret)
 			goto err;

volumes.h +4 -0

index b4ea93f0bec3..5ca2779ebd45 100644
@@ -271,6 +271,10 @@ void btrfs_close_all_devices(void);
 int btrfs_add_device(struct btrfs_trans_handle *trans,
 		     struct btrfs_fs_info *fs_info,
 		     struct btrfs_device *device);
+int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
+			   struct btrfs_device *device,
+			   u64 chunk_offset, u64 num_bytes, u64 *start,
+			   int insert_asis);
 int btrfs_update_device(struct btrfs_trans_handle *trans,
 			struct btrfs_device *device);
 int btrfs_scan_one_device(int fd, const char *path,

Test case misc/021 is testing if we could mount a single disk btrfs image recovered from multi disk fs.

The problem is, current kernel has extra check for block group, chunk and dev extent.
This means any image can't pass btrfs check for chunk tree will not mount.

So do extra btrfs check before mount, this will also help us to locate the problem in btrfs-image easier.

Signed-off-by Qu Wenruo <[email protected]>

---
 tests/misc-tests/021-image-multi-devices/test.sh | 3 +++
 1 file changed, 3 insertions(+)

tests/misc-tests/021-image-multi-devices/test.sh +0 -0

index 5430847f4e2f..26beae6e4b85 100755

tests/misc-tests/021-image-multi-devices/test.sh +1 -0

+
 run_check $SUDO_HELPER mount "$loop1" "$TEST_MNT"
 new_md5=$(run_check_stdout md5sum "$TEST_MNT/foobar" | cut -d ' ' -f 1)
 run_check $SUDO_HELPER umount "$TEST_MNT"

Quoted 22 lines · Qu Wenruo

Test case misc/021 is testing if we could mount a single disk btrfs image recovered from multi disk fs.

The problem is, current kernel has extra check for block group, chunk and dev extent.
This means any image can't pass btrfs check for chunk tree will not mount.

So do extra btrfs check before mount, this will also help us to locate the problem in btrfs-image easier.

Signed-off-by Qu Wenruo <[email protected]>

---
 tests/misc-tests/021-image-multi-devices/test.sh | 3 +++
 1 file changed, 3 insertions(+)

tests/misc-tests/021-image-multi-devices/test.sh +0 -0

index 5430847f4e2f..26beae6e4b85 100755
@@ -37,6 +37,9 @@ run_check $SUDO_HELPER wipefs -a "$loop2"
 

tests/misc-tests/021-image-multi-devices/test.sh run_check $SUDO_HELPER wipefs -a "$loop2" · line 41

Quoted 4 lines

tests/misc-tests/021-image-multi-devices/test.sh +2 -0

 run_check $SUDO_HELPER "$TOP/btrfs-image" -r "$IMAGE" "$loop1"
 
+# Run check to make sure there is nothing wrong for the recovered image
+run_check "$TOP/btrfs" check "$loop1"

I think this needs to be run_check $SUDO_HELPER "$TOP/btrfs" check "$loop1"

Quoted 5 lines

tests/misc-tests/021-image-multi-devices/test.sh +1 -0

+
 run_check $SUDO_HELPER mount "$loop1" "$TEST_MNT"
 new_md5=$(run_check_stdout md5sum "$TEST_MNT/foobar" | cut -d ' ' -f 1)
 run_check $SUDO_HELPER umount "$TEST_MNT"