btrfs-progs: Enhance btrfs-image to handle missing device

Can be fetched from github:
https://github.com/adam900710/btrfs-progs/tree/btrfs_image_fix

Bug report:
https://github.com/kdave/btrfs-progs/issues/118

In short, the problem is caused by some old code (read_extent_data() from ancient btrfs check code) and offset-by-one from btrfs-image. Which makes btrfs-image can only read from the first stripe of RAID1.

And if device of the first stripe is missing, btrfs-image will fail.

Fix the problem and add test case for it.

Qu Wenruo (3):
btrfs-progs: disk-io: Fix read_extent_data() error handler for missing
device
btrfs-progs: convert: Fix offset-by-one error in read_data_extent()
btrfs-progs: tests/misc: Test if btrfs-image can handle RAID1 missing
device

 disk-io.c                                         |  6 ++-
 image/main.c                                      |  2 +-
 tests/misc-tests/030-missing-device-image/test.sh | 57 +++++++++++++++++++++++
 3 files changed, 62 insertions(+), 3 deletions(-)
 create mode 100755 tests/misc-tests/030-missing-device-image/test.sh

2.16.3

When device is missing, read_extent_data() (function exported from old btrfs check code) has the following problems:

1) Modify @len parameter if device is missing
If device returned in @multi is missing, @len can be larger than @max_len (originl length).

This could confusing caller and underflow the read loop.

2) Still return 0 for missing device
It only handles read error, missing device is not handled and 0 is returned.

3) Wrong check for device->fd
In fact, 0 is also a valid fd.
Although not possible under most case, it's still need fix.

Fix them all.

---
 disk-io.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

index 610963357675..310ab19cf099 100644
@@ -395,10 +395,12 @@ int read_extent_data(struct btrfs_fs_info *fs_info, char *data, u64 logical,
 	}
 	device = multi->stripes[0].dev;
 
-	if (device->fd <= 0)
-		goto err;
 	if (*len > max_len)
 		*len = max_len;
+	if (device->fd < 0) {
+		ret = -EIO;
+		goto err;
+	}
 
 	ret = pread64(device->fd, data, *len, multi->stripes[0].physical);
 	if (ret != *len)

2.16.3

For read_data_extent() in convert/main.c it's using mirror number in a incorrect way, which will not get correct copy for RAID1:
------

	for (cur_mirror = 0; cur_mirror < num_copies; cur_mirror++) {

------

In such case, for RAID1 @cur_mirror will only be 0 and 1.

However for 0 and 1 case, btrfs_map_block() will only return the first copy.
To reach the 2nd copy, it correct @cur_mirror range should be 1 and 2.

So with this offset-by-one error, btrfs-image will never be able to read out data extent if the first stripe of the chunk is the missing one.

Fix it by starting @cur_mirror from 1 and to @num_copies (including).

---
 image/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 5f155c23dce6..351c5a256938 100644
@@ -571,7 +571,7 @@ static int read_data_extent(struct metadump_struct *md,
 	num_copies = btrfs_num_copies(root->fs_info, logical, bytes_left);
 
 	/* Try our best to read data, just like read_tree_block() */
-	for (cur_mirror = 0; cur_mirror < num_copies; cur_mirror++) {
+	for (cur_mirror = 1; cur_mirror <= num_copies; cur_mirror++) {
 		while (bytes_left) {
 			read_len = bytes_left;
 			ret = read_extent_data(fs_info,

2.16.3

---
 tests/misc-tests/030-missing-device-image/test.sh | 57 +++++++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100755 tests/misc-tests/030-missing-device-image/test.sh

new file mode 100755
index 000000000000..b8ae3a950cc9
@@ -0,0 +1,57 @@
+#!/bin/bash
+# Test that btrfs-image can dump image correctly for missing device (RAID1)
+#
+# At least for RAID1, btrfs-image should be able to handle one missing device
+# without any problem
+
+source "$TEST_TOP/common"
+
+check_prereq btrfs-image
+check_prereq mkfs.btrfs
+check_prereq btrfs
+
+setup_root_helper
+setup_loopdevs 2
+prepare_loopdevs
+dev1=${loopdevs[1]}
+dev2=${loopdevs[2]}
+
+# $1:	device number to remove (either 1 or 2)
+tmp=$(mktemp --tmpdir -d btrfs-progs-misc-test-XXXXXXX)
+test_missing()
+{
+	bad_num=$1
+	bad_dev=${loopdevs[$bad_num]}
+	good_num=$((3 - $bad_num))
+	good_dev=${loopdevs[$good_num]}
+
+	run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f -d raid1 -m raid1 \
+		"$dev1" "$dev2"
+	
+	# fill the fs with some data, we could create space cache
+	run_check $SUDO_HELPER mount "$dev1" "$TEST_MNT"
+	run_check $SUDO_HELPER dd if=/dev/zero of="$TEST_MNT/a" bs=1M count=10
+	run_check $SUDO_HELPER dd if=/dev/zero of="$TEST_MNT/b" bs=4k count=1000 conv=sync
+	run_check $SUDO_HELPER umount "$TEST_MNT"
+	
+	# make sure we have space cache
+	run_check_stdout "$TOP/btrfs" inspect dump-tree -t root "$dev1" \
+		> "$tmp/output"
+	if ! grep -q "EXTENT_DATA" "$tmp/output" ; then
+		# normally above operation should create space cache.
+		# if not, it may means we have migrated to v2 cache by default
+		_not_run "unable to create v1 space cache"
+	fi
+
+	# now wipe the device
+	run_check wipefs -fa "$bad_dev"
+
+	# we don't care about the image but btrfs-image must not fail
+	run_check "$TOP/btrfs-image" "$good_dev" /dev/null
+}
+
+# Test with either device missing, so we're ensured to hit missing device
+test_missing 1
+test_missing 2
+cleanup_loopdevs
+rm $tmp -rf

2.16.3

Applied with the fixes below, thanks.

1-3 applied, thanks. I'll reorder the uuid tree patch after those patches so the test does not fail.