[PATCH v2 0/5] btrfs-progs: image: Fix error when restoring multi-disk image to single disk

[PATCH v2 0/5] btrfs-progs: image: Fix error when restoring multi-disk image to single disk

[Qu Wenruo]

This patchset can be fetched from github:
https://github.com/adam900710/btrfs-progs/tree/image_recover

The base commit is as usual, the latest stable tag, v4.19.


Test case misc/021 will fail if using latest upstream kernel.

This is due to the enhanced code in kernel to check block group <->
chunk <-> dev extent mapping.

This means from the very beginning, btrfs-image can't really restore a
multi-disk image to single-disk one correctly.

The problem is, although we modified chunk item, we didn't modify block
group item's flags or dev extents.

This patch will reset block group flags, then rebuild the whole
dev extents by removing existing ones first, then re-add the correct
dev extents calculated from chunk map.

Now it could pass all misc tests and fsck tests.

Changelog:
v2:
- Parameter list cleanup
  * Use trans->fs_info to remove fs_info parameter
  * Remove trans parameter for function who doesn't need
- Merge dev extents removal code with rebuild code
- Refactor btrfs_alloc_dev_extent() into 2 functions
  * btrfs_insert_dev_extent() for convert and dev extent rebuild
  * btrfs_alloc_dev_extent() for old use case
  
Qu Wenruo (5):
  btrfs-progs: image: Refactor fixup_devices() to
    fixup_chunks_and_devices()
  btrfs-progs: image: Fix block group item flags when restoring
    multi-device image to single device
  btrfs-progs: volumes: Refactor btrfs_alloc_dev_extent() into two
    functions
  btrfs-progs: image: Remove all existing dev extents for later rebuild
  btrfs-progs: misc-tests/021: Do extra btrfs check before mounting

 image/main.c                                  | 200 ++++++++++++++++--
 .../021-image-multi-devices/test.sh           |   3 +
 volumes.c                                     |  48 +++--
 volumes.h                                     |   3 +
 4 files changed, 220 insertions(+), 34 deletions(-)

-- 
2.19.2

[PATCH v2 1/5] btrfs-progs: image: Refactor fixup_devices() to fixup_chunks_and_devices()

[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 <wqu@suse.com>
---
 image/main.c | 52 ++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 16 deletions(-)

diff --git a/image/main.c b/image/main.c
index c680ab19de6c..bbfcf8f19298 100644
--- a/image/main.c
+++ b/image/main.c
@@ -2084,28 +2084,19 @@ static void remap_overlapping_chunks(struct mdrestore_struct *mdres)
 	}
 }
 
-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 mdrestore_struct *mdres,
+			     off_t dev_size)
 {
-	struct btrfs_trans_handle *trans;
+	struct btrfs_fs_info *fs_info = trans->fs_info;
 	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, 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;
-- 
2.19.2

[PATCH v2 2/5] btrfs-progs: image: Fix block group item flags when restoring multi-device image to single device

[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 <wqu@suse.com>
---
 image/main.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/image/main.c b/image/main.c
index bbfcf8f19298..9187de34f34a 100644
--- a/image/main.c
+++ b/image/main.c
@@ -2164,6 +2164,51 @@ again:
 	return 0;
 }
 
+static void fixup_block_groups(struct btrfs_fs_info *fs_info)
+{
+	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 +2225,7 @@ static int fixup_chunks_and_devices(struct btrfs_fs_info *fs_info,
 		return PTR_ERR(trans);
 	}
 
+	fixup_block_groups(fs_info);
 	ret = fixup_device_size(trans, mdres, dev_size);
 	if (ret < 0)
 		goto error;
-- 
2.19.2

[PATCH v2 3/5] btrfs-progs: volumes: Refactor btrfs_alloc_dev_extent() into two functions

[Qu Wenruo]

We have btrfs_alloc_dev_extent() accepting @convert flag to toggle
special handling for convert.

However that @convert flag only determine whether we call
find_free_dev_extent(), and we may later need to insert dev extents
without searching dev tree.

So refactor btrfs_alloc_dev_extent() into 2 functions,
btrfs_alloc_dev_extent(), which will try to find free dev extent, and
btrfs_insert_dev_extent(), which will just insert a dev extent.

For implementation, btrfs_alloc_dev_extent() will call
btrfs_insert_dev_extent() anyway, so no duplicated code.

This removes the need of @convert parameter, and make
btrfs_insert_dev_extent() public for later usage.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 volumes.c | 48 ++++++++++++++++++++++++++++++------------------
 volumes.h |  3 +++
 2 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/volumes.c b/volumes.c
index 30090ce5f8e8..0dd082cd1718 100644
--- a/volumes.c
+++ b/volumes.c
@@ -530,10 +530,12 @@ 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);
 }
 
-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)
+/*
+ * Insert one device extent into the fs.
+ */
+int btrfs_insert_dev_extent(struct btrfs_trans_handle *trans,
+			    struct btrfs_device *device,
+			    u64 chunk_offset, u64 num_bytes, u64 start)
 {
 	int ret;
 	struct btrfs_path *path;
@@ -546,18 +548,8 @@ static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
 	if (!path)
 		return -ENOMEM;
 
-	/*
-	 * For convert case, just skip search free dev_extent, as caller
-	 * is responsible to make sure it's free.
-	 */
-	if (!convert) {
-		ret = find_free_dev_extent(device, num_bytes, start, NULL);
-		if (ret)
-			goto err;
-	}
-
 	key.objectid = device->devid;
-	key.offset = *start;
+	key.offset = start;
 	key.type = BTRFS_DEV_EXTENT_KEY;
 	ret = btrfs_insert_empty_item(trans, root, path, &key,
 				      sizeof(*extent));
@@ -583,6 +575,22 @@ err:
 	return ret;
 }
 
+/*
+ * Allocate one free dev extent and insert it into the fs.
+ */
+static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
+				  struct btrfs_device *device,
+				  u64 chunk_offset, u64 num_bytes, u64 *start)
+{
+	int ret;
+
+	ret = find_free_dev_extent(device, num_bytes, start, NULL);
+	if (ret)
+		return ret;
+	return btrfs_insert_dev_extent(trans, device, chunk_offset, num_bytes,
+					*start);
+}
+
 static int find_next_chunk(struct btrfs_fs_info *fs_info, u64 *offset)
 {
 	struct btrfs_root *root = fs_info->chunk_root;
@@ -1107,7 +1115,7 @@ again:
 			list_move_tail(&device->dev_list, dev_list);
 
 		ret = btrfs_alloc_dev_extent(trans, device, key.offset,
-			     calc_size, &dev_offset, 0);
+			     calc_size, &dev_offset);
 		if (ret < 0)
 			goto out_chunk_map;
 
@@ -1241,8 +1249,12 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
 	while (index < num_stripes) {
 		struct btrfs_stripe *stripe;
 
-		ret = btrfs_alloc_dev_extent(trans, device, key.offset,
-			     calc_size, &dev_offset, convert);
+		if (convert)
+			ret = btrfs_insert_dev_extent(trans, device, key.offset,
+					calc_size, dev_offset);
+		else
+			ret = btrfs_alloc_dev_extent(trans, device, key.offset,
+					calc_size, &dev_offset);
 		BUG_ON(ret);
 
 		device->bytes_used += calc_size;
diff --git a/volumes.h b/volumes.h
index b4ea93f0bec3..44284ee75adb 100644
--- a/volumes.h
+++ b/volumes.h
@@ -268,6 +268,9 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 		       int flags);
 int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
 void btrfs_close_all_devices(void);
+int btrfs_insert_dev_extent(struct btrfs_trans_handle *trans,
+			    struct btrfs_device *device,
+			    u64 chunk_offset, u64 num_bytes, u64 start);
 int btrfs_add_device(struct btrfs_trans_handle *trans,
 		     struct btrfs_fs_info *fs_info,
 		     struct btrfs_device *device);
-- 
2.19.2

[PATCH v2 4/5] btrfs-progs: image: Remove all existing dev extents for later rebuild

[Qu Wenruo]

For multi-disk btrfs image recovered to single disk, the dev tree would
look like:
        item 2 key (1 DEV_EXTENT 22020096)
                dev extent chunk_tree 3
                chunk_objectid 256 chunk_offset 22020096 length 8388608
        item 3 key (1 DEV_EXTENT 30408704)
                dev extent chunk_tree 3
                chunk_objectid 256 chunk_offset 30408704 length 1073741824
        item 4 key (1 DEV_EXTENT 1104150528)
                dev extent chunk_tree 3
                chunk_objectid 256 chunk_offset 1104150528 length 536870912
        item 5 key (2 DEV_EXTENT 1048576)
                dev extent chunk_tree 3
                chunk_objectid 256 chunk_offset 22020096 length 8388608
        item 6 key (2 DEV_EXTENT 9437184)
                dev extent chunk_tree 3
                chunk_objectid 256 chunk_offset 30408704 length 1073741824
        item 7 key (2 DEV_EXTENT 1083179008)
                dev extent chunk_tree 3
                chunk_objectid 256 chunk_offset 1104150528 length 536870912

However in chunk tree, we only use devid 2, thus devid 1 is completely
garbage:
        item 0 key (DEV_ITEMS DEV_ITEM 2)
        item 1 key (FIRST_CHUNK_TREE CHUNK_ITEM 22020096) itemoff 16105 itemsize 80
                length 8388608 owner 2 stripe_len 65536 type SYSTEM
                num_stripes 1 sub_stripes 0
                        stripe 0 devid 2 offset 1048576
        item 2 key (FIRST_CHUNK_TREE CHUNK_ITEM 30408704) itemoff 16025 itemsize 80
                length 1073741824 owner 2 stripe_len 65536 type METADATA
                num_stripes 1 sub_stripes 0
                        stripe 0 devid 2 offset 9437184
        item 3 key (FIRST_CHUNK_TREE CHUNK_ITEM 1104150528) itemoff 15945 itemsize 80
                length 1073741824 owner 2 stripe_len 65536 type DATA
                num_stripes 1 sub_stripes 0
                        stripe 0 devid 2 offset 1083179008

To fix the problem, the most straight-forward way is to remove all
existing dev extents, and then re-fill correct dev extents from chunk.

So this patch just follow the straight-forward way to fix it, causing
the final dev extents layout to match with chunk tree, and make btrfs
check happy.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 image/main.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/image/main.c b/image/main.c
index 9187de34f34a..8f756689a1fa 100644
--- a/image/main.c
+++ b/image/main.c
@@ -2209,6 +2209,104 @@ static void fixup_block_groups(struct btrfs_fs_info *fs_info)
 	}
 }
 
+static int remove_all_dev_extents(struct btrfs_trans_handle *trans)
+{
+	struct btrfs_fs_info *fs_info = trans->fs_info;
+	struct btrfs_root *root = fs_info->dev_root;
+	struct btrfs_path path;
+	struct btrfs_key key;
+	struct extent_buffer *leaf;
+	int slot;
+	int ret;
+
+	key.objectid = 1;
+	key.type = BTRFS_DEV_EXTENT_KEY;
+	key.offset = 0;
+	btrfs_init_path(&path);
+
+	ret = btrfs_search_slot(trans, root, &key, &path, -1, 1);
+	if (ret < 0) {
+		error("failed to search dev tree: %s", strerror(-ret));
+		return ret;
+	}
+
+	while (1) {
+		slot = path.slots[0];
+		leaf = path.nodes[0];
+		if (slot >= btrfs_header_nritems(leaf)) {
+			ret = btrfs_next_leaf(root, &path);
+			if (ret < 0) {
+				error("failed to search dev tree: %s",
+					strerror(-ret));
+				goto out;
+			}
+			if (ret > 0) {
+				ret = 0;
+				goto out;
+			}
+		}
+
+		btrfs_item_key_to_cpu(leaf, &key, slot);
+		if (key.type != BTRFS_DEV_EXTENT_KEY)
+			break;
+		ret = btrfs_del_item(trans, root, &path);
+		if (ret < 0) {
+			error("failed to delete dev extent %llu, %llu: %s",
+				key.objectid, key.offset, strerror(-ret));
+			goto out;
+		}
+	}
+out:
+	btrfs_release_path(&path);
+	return ret;
+}
+
+static int fixup_dev_extents(struct btrfs_trans_handle *trans)
+{
+	struct btrfs_fs_info *fs_info = trans->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);
+	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_insert_dev_extent(trans, dev, ce->start,
+					stripe_len, map->stripes[i].physical);
+			if (ret < 0) {
+				error(
+				"failed to insert dev extent %llu %llu: %s",
+					devid, map->stripes[i].physical,
+					strerror(-ret));
+				goto out;
+			}
+		}
+	}
+out:
+	return ret;
+}
+
 static int fixup_chunks_and_devices(struct btrfs_fs_info *fs_info,
 			 struct mdrestore_struct *mdres, off_t dev_size)
 {
@@ -2226,6 +2324,10 @@ static int fixup_chunks_and_devices(struct btrfs_fs_info *fs_info,
 	}
 
 	fixup_block_groups(fs_info);
+	ret = fixup_dev_extents(trans);
+	if (ret < 0)
+		goto error;
+
 	ret = fixup_device_size(trans, mdres, dev_size);
 	if (ret < 0)
 		goto error;
-- 
2.19.2

[PATCH v2 5/5] btrfs-progs: misc-tests/021: Do extra btrfs check before mounting

[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 <wqu@suse.com>
---
 tests/misc-tests/021-image-multi-devices/test.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/misc-tests/021-image-multi-devices/test.sh b/tests/misc-tests/021-image-multi-devices/test.sh
index 5430847f4e2f..26beae6e4b85 100755
--- a/tests/misc-tests/021-image-multi-devices/test.sh
+++ b/tests/misc-tests/021-image-multi-devices/test.sh
@@ -37,6 +37,9 @@ run_check $SUDO_HELPER wipefs -a "$loop2"
 
 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"
+
 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"
-- 
2.19.2

Re: [PATCH v2 3/5] btrfs-progs: volumes: Refactor btrfs_alloc_dev_extent() into two functions

[Nikolay Borisov]

On 27.11.18 г. 10:38 ч., Qu Wenruo wrote:
> We have btrfs_alloc_dev_extent() accepting @convert flag to toggle
> special handling for convert.
> 
> However that @convert flag only determine whether we call
> find_free_dev_extent(), and we may later need to insert dev extents
> without searching dev tree.
> 
> So refactor btrfs_alloc_dev_extent() into 2 functions,
> btrfs_alloc_dev_extent(), which will try to find free dev extent, and
> btrfs_insert_dev_extent(), which will just insert a dev extent.
> 
> For implementation, btrfs_alloc_dev_extent() will call
> btrfs_insert_dev_extent() anyway, so no duplicated code.
> 
> This removes the need of @convert parameter, and make
> btrfs_insert_dev_extent() public for later usage.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

This looks much better:

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  volumes.c | 48 ++++++++++++++++++++++++++++++------------------
>  volumes.h |  3 +++
>  2 files changed, 33 insertions(+), 18 deletions(-)
> 
> diff --git a/volumes.c b/volumes.c
> index 30090ce5f8e8..0dd082cd1718 100644
> --- a/volumes.c
> +++ b/volumes.c
> @@ -530,10 +530,12 @@ 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);
>  }
>  
> -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)
> +/*
> + * Insert one device extent into the fs.
> + */
> +int btrfs_insert_dev_extent(struct btrfs_trans_handle *trans,
> +			    struct btrfs_device *device,
> +			    u64 chunk_offset, u64 num_bytes, u64 start)
>  {
>  	int ret;
>  	struct btrfs_path *path;
> @@ -546,18 +548,8 @@ static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
>  	if (!path)
>  		return -ENOMEM;
>  
> -	/*
> -	 * For convert case, just skip search free dev_extent, as caller
> -	 * is responsible to make sure it's free.
> -	 */
> -	if (!convert) {
> -		ret = find_free_dev_extent(device, num_bytes, start, NULL);
> -		if (ret)
> -			goto err;
> -	}
> -
>  	key.objectid = device->devid;
> -	key.offset = *start;
> +	key.offset = start;
>  	key.type = BTRFS_DEV_EXTENT_KEY;
>  	ret = btrfs_insert_empty_item(trans, root, path, &key,
>  				      sizeof(*extent));
> @@ -583,6 +575,22 @@ err:
>  	return ret;
>  }
>  
> +/*
> + * Allocate one free dev extent and insert it into the fs.
> + */
> +static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
> +				  struct btrfs_device *device,
> +				  u64 chunk_offset, u64 num_bytes, u64 *start)
> +{
> +	int ret;
> +
> +	ret = find_free_dev_extent(device, num_bytes, start, NULL);
> +	if (ret)
> +		return ret;
> +	return btrfs_insert_dev_extent(trans, device, chunk_offset, num_bytes,
> +					*start);
> +}
> +
>  static int find_next_chunk(struct btrfs_fs_info *fs_info, u64 *offset)
>  {
>  	struct btrfs_root *root = fs_info->chunk_root;
> @@ -1107,7 +1115,7 @@ again:
>  			list_move_tail(&device->dev_list, dev_list);
>  
>  		ret = btrfs_alloc_dev_extent(trans, device, key.offset,
> -			     calc_size, &dev_offset, 0);
> +			     calc_size, &dev_offset);
>  		if (ret < 0)
>  			goto out_chunk_map;
>  
> @@ -1241,8 +1249,12 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
>  	while (index < num_stripes) {
>  		struct btrfs_stripe *stripe;
>  
> -		ret = btrfs_alloc_dev_extent(trans, device, key.offset,
> -			     calc_size, &dev_offset, convert);
> +		if (convert)
> +			ret = btrfs_insert_dev_extent(trans, device, key.offset,
> +					calc_size, dev_offset);
> +		else
> +			ret = btrfs_alloc_dev_extent(trans, device, key.offset,
> +					calc_size, &dev_offset);
>  		BUG_ON(ret);
>  
>  		device->bytes_used += calc_size;
> diff --git a/volumes.h b/volumes.h
> index b4ea93f0bec3..44284ee75adb 100644
> --- a/volumes.h
> +++ b/volumes.h
> @@ -268,6 +268,9 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
>  		       int flags);
>  int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
>  void btrfs_close_all_devices(void);
> +int btrfs_insert_dev_extent(struct btrfs_trans_handle *trans,
> +			    struct btrfs_device *device,
> +			    u64 chunk_offset, u64 num_bytes, u64 start);
>  int btrfs_add_device(struct btrfs_trans_handle *trans,
>  		     struct btrfs_fs_info *fs_info,
>  		     struct btrfs_device *device);
> 

Re: [PATCH v2 1/5] btrfs-progs: image: Refactor fixup_devices() to fixup_chunks_and_devices()

[Nikolay Borisov]

On 27.11.18 г. 10:38 ч., Qu Wenruo wrote:
> 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 <wqu@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

However, one minor nit below.

> ---
>  image/main.c | 52 ++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 36 insertions(+), 16 deletions(-)
> 
> diff --git a/image/main.c b/image/main.c
> index c680ab19de6c..bbfcf8f19298 100644
> --- a/image/main.c
> +++ b/image/main.c
> @@ -2084,28 +2084,19 @@ static void remap_overlapping_chunks(struct mdrestore_struct *mdres)
>  	}
>  }
>  
> -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 mdrestore_struct *mdres,
> +			     off_t dev_size)
>  {
> -	struct btrfs_trans_handle *trans;
> +	struct btrfs_fs_info *fs_info = trans->fs_info;
>  	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;

nit: Unnecessary change

>  	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, 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;
> 

Re: [PATCH v2 5/5] btrfs-progs: misc-tests/021: Do extra btrfs check before mounting

[Nikolay Borisov]

On 27.11.18 г. 10:38 ч., Qu Wenruo wrote:
> 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 <wqu@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  tests/misc-tests/021-image-multi-devices/test.sh | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tests/misc-tests/021-image-multi-devices/test.sh b/tests/misc-tests/021-image-multi-devices/test.sh
> index 5430847f4e2f..26beae6e4b85 100755
> --- a/tests/misc-tests/021-image-multi-devices/test.sh
> +++ b/tests/misc-tests/021-image-multi-devices/test.sh
> @@ -37,6 +37,9 @@ run_check $SUDO_HELPER wipefs -a "$loop2"
>  
>  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"
> +
>  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"
> 

Re: [PATCH v2 1/5] btrfs-progs: image: Refactor fixup_devices() to fixup_chunks_and_devices()

[Qu Wenruo]

On 2018/11/27 下午4:46, Nikolay Borisov wrote:
> 
> 
> On 27.11.18 г. 10:38 ч., Qu Wenruo wrote:
>> 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 <wqu@suse.com>
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> 
> However, one minor nit below.
> 
>> ---
>>  image/main.c | 52 ++++++++++++++++++++++++++++++++++++----------------
>>  1 file changed, 36 insertions(+), 16 deletions(-)
>>
>> diff --git a/image/main.c b/image/main.c
>> index c680ab19de6c..bbfcf8f19298 100644
>> --- a/image/main.c
>> +++ b/image/main.c
>> @@ -2084,28 +2084,19 @@ static void remap_overlapping_chunks(struct mdrestore_struct *mdres)
>>  	}
>>  }
>>  
>> -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 mdrestore_struct *mdres,
>> +			     off_t dev_size)
>>  {
>> -	struct btrfs_trans_handle *trans;
>> +	struct btrfs_fs_info *fs_info = trans->fs_info;
>>  	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;
> 
> nit: Unnecessary change

Doesn't it look better when all btrfs_ prefix get batched together? :)

Thanks,
Qu

> 
>>  	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, 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;
>>

Re: [PATCH v2 1/5] btrfs-progs: image: Refactor fixup_devices() to fixup_chunks_and_devices()

[Nikolay Borisov]

On 27.11.18 г. 10:50 ч., Qu Wenruo wrote:
> 
> 
> On 2018/11/27 下午4:46, Nikolay Borisov wrote:
>>
>>
>> On 27.11.18 г. 10:38 ч., Qu Wenruo wrote:
>>> 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 <wqu@suse.com>
>>
>> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
>>
>> However, one minor nit below.
>>
>>> ---
>>>  image/main.c | 52 ++++++++++++++++++++++++++++++++++++----------------
>>>  1 file changed, 36 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/image/main.c b/image/main.c
>>> index c680ab19de6c..bbfcf8f19298 100644
>>> --- a/image/main.c
>>> +++ b/image/main.c
>>> @@ -2084,28 +2084,19 @@ static void remap_overlapping_chunks(struct mdrestore_struct *mdres)
>>>  	}
>>>  }
>>>  
>>> -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 mdrestore_struct *mdres,
>>> +			     off_t dev_size)
>>>  {
>>> -	struct btrfs_trans_handle *trans;
>>> +	struct btrfs_fs_info *fs_info = trans->fs_info;
>>>  	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;
>>
>> nit: Unnecessary change
> 
> Doesn't it look better when all btrfs_ prefix get batched together? :)

Didn't even realize this was the intended effect. IMO doesn't make much
difference, what does, though, is probably reverse christmas tree, ie

longer variable names
come before shorter
ones

But I guess this is a matter of taste, no need to resend.

> 
> Thanks,
> Qu
> 
>>
>>>  	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, 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;
>>>
> 

Re: [PATCH v2 1/5] btrfs-progs: image: Refactor fixup_devices() to fixup_chunks_and_devices()

[David Sterba]

On Tue, Nov 27, 2018 at 04:50:57PM +0800, Qu Wenruo wrote:
> >> -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 mdrestore_struct *mdres,
> >> +			     off_t dev_size)
> >>  {
> >> -	struct btrfs_trans_handle *trans;
> >> +	struct btrfs_fs_info *fs_info = trans->fs_info;
> >>  	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;
> > 
> > nit: Unnecessary change
> 
> Doesn't it look better when all btrfs_ prefix get batched together? :)

Please don't do unrelated changes like that.

Re: [PATCH v2 1/5] btrfs-progs: image: Refactor fixup_devices() to fixup_chunks_and_devices()

[David Sterba]

On Tue, Nov 27, 2018 at 04:38:24PM +0800, Qu Wenruo wrote:
> +error:
> +	error(
> +"failed to fix chunks and devices mapping, the fs may not be mountable: %s",
> +		strerror(-ret));

Recently the sterror(error code) have been converted to %m, so I changed
this to

	errno = -ret
	error("... %m");

Re: [PATCH v2 1/5] btrfs-progs: image: Refactor fixup_devices() to fixup_chunks_and_devices()

[Qu Wenruo]

On 2018/12/4 下午6:18, David Sterba wrote:
> On Tue, Nov 27, 2018 at 04:50:57PM +0800, Qu Wenruo wrote:
>>>> -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 mdrestore_struct *mdres,
>>>> +			     off_t dev_size)
>>>>  {
>>>> -	struct btrfs_trans_handle *trans;
>>>> +	struct btrfs_fs_info *fs_info = trans->fs_info;
>>>>  	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;
>>>
>>> nit: Unnecessary change
>>
>> Doesn't it look better when all btrfs_ prefix get batched together? :)
> 
> Please don't do unrelated changes like that.

Understood, the github version has that @leaf reverted to the original
location.

Thanks,
Qu

Re: [PATCH v2 1/5] btrfs-progs: image: Refactor fixup_devices() to fixup_chunks_and_devices()

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 471 bytes --]



On 2018/12/4 下午6:20, David Sterba wrote:
> On Tue, Nov 27, 2018 at 04:38:24PM +0800, Qu Wenruo wrote:
>> +error:
>> +	error(
>> +"failed to fix chunks and devices mapping, the fs may not be mountable: %s",
>> +		strerror(-ret));
> 
> Recently the sterror(error code) have been converted to %m, so I changed
> this to
> 
> 	errno = -ret
> 	error("... %m");
> 

Thanks for mentioning this.

I'll use this format for later patches.

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 0/5] btrfs-progs: image: Fix error when restoring multi-disk image to single disk

[David Sterba]

On Tue, Nov 27, 2018 at 04:38:23PM +0800, Qu Wenruo wrote:
> This patchset can be fetched from github:
> https://github.com/adam900710/btrfs-progs/tree/image_recover
> 
> The base commit is as usual, the latest stable tag, v4.19.
> 
> 
> Test case misc/021 will fail if using latest upstream kernel.
> 
> This is due to the enhanced code in kernel to check block group <->
> chunk <-> dev extent mapping.
> 
> This means from the very beginning, btrfs-image can't really restore a
> multi-disk image to single-disk one correctly.
> 
> The problem is, although we modified chunk item, we didn't modify block
> group item's flags or dev extents.
> 
> This patch will reset block group flags, then rebuild the whole
> dev extents by removing existing ones first, then re-add the correct
> dev extents calculated from chunk map.
> 
> Now it could pass all misc tests and fsck tests.
> 
> Changelog:
> v2:
> - Parameter list cleanup
>   * Use trans->fs_info to remove fs_info parameter
>   * Remove trans parameter for function who doesn't need
> - Merge dev extents removal code with rebuild code
> - Refactor btrfs_alloc_dev_extent() into 2 functions
>   * btrfs_insert_dev_extent() for convert and dev extent rebuild
>   * btrfs_alloc_dev_extent() for old use case
>   
> Qu Wenruo (5):
>   btrfs-progs: image: Refactor fixup_devices() to
>     fixup_chunks_and_devices()
>   btrfs-progs: image: Fix block group item flags when restoring
>     multi-device image to single device
>   btrfs-progs: volumes: Refactor btrfs_alloc_dev_extent() into two
>     functions
>   btrfs-progs: image: Remove all existing dev extents for later rebuild
>   btrfs-progs: misc-tests/021: Do extra btrfs check before mounting

Merged to devel, thanks. I've switched strerror(-ret) to the errno
pattern.