[PATCH 1/2] btrfs-progs: convert: Fix a bug in rollback check which overwrite return value

[PATCH 1/2] btrfs-progs: convert: Fix a bug in rollback check which overwrite return value

[Qu Wenruo]

Commit 1170ac307900 ("btrfs-progs: convert: Introduce function to check if
convert image is able to be rolled back") reworked rollback check
condition, by checking 1:1 mapping of each file extent.

The idea itself has nothing wrong, but error handler is not implemented
correctly, which over writes the return value and always try to rollback
the fs even it fails to pass the check.

Fix it by correctly return the error before rollback the fs.

Fixes: 1170ac307900 ("btrfs-progs: convert: Introduce function to check if convert image is able to be rolled back")
Reported-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/convert/main.c b/convert/main.c
index af2855316fee..89f9261172ca 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -1443,6 +1443,8 @@ next:
 		}
 	}
 	btrfs_release_path(&path);
+	if (ret)
+		return ret;
 	/*
 	 * For HOLES mode (without NO_HOLES), we must ensure file extents
 	 * cover the whole range of the image
-- 
2.15.0

[PATCH 2/2] btrfs-progs: tests/convert: Introduce test case to ensure btrfs-convert won't rollback the filesystem after balance

[Qu Wenruo]

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 .../015-not-to-rollback-after-balance/test.sh      | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100755 tests/convert-tests/015-not-to-rollback-after-balance/test.sh

diff --git a/tests/convert-tests/015-not-to-rollback-after-balance/test.sh b/tests/convert-tests/015-not-to-rollback-after-balance/test.sh
new file mode 100755
index 000000000000..0d523d2250db
--- /dev/null
+++ b/tests/convert-tests/015-not-to-rollback-after-balance/test.sh
@@ -0,0 +1,33 @@
+#!/bin/bash
+# Check if btrfs-convert refuse to rollback the filesystem, and leave
+# the btrfs and the convert image untouched
+
+source "$TOP/tests/common"
+source "$TOP/tests/common.convert"
+
+setup_root_helper
+prepare_test_dev
+check_prereq btrfs-convert
+check_global_prereq mke2fs
+
+# convert_test_prep_fs() will create large enough file inside the
+# test device, that's good enough for us to test rollback failure.
+convert_test_prep_fs ext4 mke2fs -t ext4 -b 4096
+run_check_umount_test_dev
+convert_test_do_convert "" 4096
+
+run_check_mount_test_dev
+
+# Now the fs is converted, balance the fs so later rollback should fail
+run_check $SUDO_HELPER "$TOP/btrfs" balance start --full-balance $TEST_MNT
+run_check_umount_test_dev
+
+# rollback should fail
+run_mustfail "rollback fs after balance" "$TOP/btrfs-convert" -r $TEST_DEV
+
+# Ensure the fs and convert image can pass the check
+run_check "$TOP/btrfs" check $TEST_DEV
+
+run_check_mount_test_dev
+run_check $SUDO_HELPER e2fsck -fn $TEST_MNT/ext2_saved/image
+run_check_umount_test_dev
-- 
2.15.0

Re: [PATCH 1/2] btrfs-progs: convert: Fix a bug in rollback check which overwrite return value

[David Sterba]

On Wed, Nov 29, 2017 at 09:48:05PM +0800, Qu Wenruo wrote:
> Commit 1170ac307900 ("btrfs-progs: convert: Introduce function to check if
> convert image is able to be rolled back") reworked rollback check
> condition, by checking 1:1 mapping of each file extent.
> 
> The idea itself has nothing wrong, but error handler is not implemented
> correctly, which over writes the return value and always try to rollback
> the fs even it fails to pass the check.
> 
> Fix it by correctly return the error before rollback the fs.
> 
> Fixes: 1170ac307900 ("btrfs-progs: convert: Introduce function to check if convert image is able to be rolled back")
> Reported-by: Jeff Mahoney <jeffm@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

1-2 applied, thanks.