[PATCH 0/2] Support patches for btrfs backup root rework

[PATCH 0/2] Support patches for btrfs backup root rework

[Nikolay Borisov]

Here are two patches which I cooked up while working on the kernel side of backup
root retention code. The first one fixes '-m <bytenr> -f generation' options to 
btrfs-corrupt-block, allowing me to simulate backup root corruption. 

The second patch is a test case which sanity checks the implementation. It suceeds
both before and after my rework of the kernel code. 

Nikolay Borisov (2):
  btrfs-progs: corrupt-block: Refactor tree block corruption code
  btrfs-progs: tests: Test backup root retention logic

 btrfs-corrupt-block.c                              | 108 +++++++++++----------
 .../misc-tests/038-backup-root-corruption/test.sh  |  50 ++++++++++
 2 files changed, 107 insertions(+), 51 deletions(-)
 create mode 100755 tests/misc-tests/038-backup-root-corruption/test.sh

-- 
2.7.4

[PATCH 1/2] btrfs-progs: corrupt-block: Refactor tree block corruption code

[Nikolay Borisov]

As progs' transaction/CoW logic evolved over the years the metadata block
corruption code failed to do so. It's currently impossible to corrupt
the generation because the CoW logic will not only set it to the value
of the currently running transaction (__btrfs_cow_block) but the
current code will ASSERT due to the following check in __btrfs_cow_block:

   WARN_ON(!(buf->flags & EXTENT_BAD_TRANSID) &&
                   btrfs_header_generation(buf) > trans->transid);

Fix this by making the generation corruption code directly write
the modified block, outside of the transaction mechanism. At the same
time move the old code into BTRFS_METADATA_BLOCK_SHIFT_ITEMS handling
case, essentially leaving it unchanged.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 btrfs-corrupt-block.c | 108 ++++++++++++++++++++++++++------------------------
 1 file changed, 57 insertions(+), 51 deletions(-)

diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
index 716439a5ea7c..dc5b81ffca3a 100644
--- a/btrfs-corrupt-block.c
+++ b/btrfs-corrupt-block.c
@@ -746,15 +746,8 @@ static void shift_items(struct btrfs_root *root, struct extent_buffer *eb)
 static int corrupt_metadata_block(struct btrfs_fs_info *fs_info, u64 block,
 				  char *field)
 {
-	struct btrfs_trans_handle *trans;
-	struct btrfs_root *root;
-	struct btrfs_path *path;
 	struct extent_buffer *eb;
-	struct btrfs_key key, root_key;
 	enum btrfs_metadata_block_field corrupt_field;
-	u64 root_objectid;
-	u64 orig, bogus;
-	u8 level;
 	int ret;
 
 	corrupt_field = convert_metadata_block_field(field);
@@ -768,63 +761,76 @@ static int corrupt_metadata_block(struct btrfs_fs_info *fs_info, u64 block,
 		fprintf(stderr, "Couldn't read in tree block %s\n", field);
 		return -EINVAL;
 	}
-	root_objectid = btrfs_header_owner(eb);
-	level = btrfs_header_level(eb);
-	if (level)
-		btrfs_node_key_to_cpu(eb, &key, 0);
-	else
-		btrfs_item_key_to_cpu(eb, &key, 0);
-	free_extent_buffer(eb);
-
-	root_key.objectid = root_objectid;
-	root_key.type = BTRFS_ROOT_ITEM_KEY;
-	root_key.offset = (u64)-1;
-
-	root = btrfs_read_fs_root(fs_info, &root_key);
-	if (IS_ERR(root)) {
-		fprintf(stderr, "Couldn't find owner root %llu\n",
-			key.objectid);
-		return PTR_ERR(root);
-	}
-
-	path = btrfs_alloc_path();
-	if (!path)
-		return -ENOMEM;
-
-	trans = btrfs_start_transaction(root, 1);
-	if (IS_ERR(trans)) {
-		btrfs_free_path(path);
-		fprintf(stderr, "Couldn't start transaction %ld\n",
-			PTR_ERR(trans));
-		return PTR_ERR(trans);
-	}
-
-	path->lowest_level = level;
-	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
-	if (ret < 0) {
-		fprintf(stderr, "Error searching to node %d\n", ret);
-		goto out;
-	}
-	eb = path->nodes[level];
 
 	ret = 0;
 	switch (corrupt_field) {
 	case BTRFS_METADATA_BLOCK_GENERATION:
-		orig = btrfs_header_generation(eb);
-		bogus = generate_u64(orig);
+		{
+		u64 orig = btrfs_header_generation(eb);
+		u64 bogus = generate_u64(orig);
 		btrfs_set_header_generation(eb, bogus);
+		write_and_map_eb(fs_info, eb);
+		free_extent_buffer(eb);
 		break;
+		}
 	case BTRFS_METADATA_BLOCK_SHIFT_ITEMS:
+		{
+		struct btrfs_trans_handle *trans;
+		struct btrfs_root *root;
+		struct btrfs_path *path;
+		struct btrfs_key key, root_key;
+		u64 root_objectid;
+		u8 level;
+		root_objectid = btrfs_header_owner(eb);
+		level = btrfs_header_level(eb);
+		if (level)
+			btrfs_node_key_to_cpu(eb, &key, 0);
+		else
+			btrfs_item_key_to_cpu(eb, &key, 0);
+		free_extent_buffer(eb);
+
+		root_key.objectid = root_objectid;
+		root_key.type = BTRFS_ROOT_ITEM_KEY;
+		root_key.offset = (u64)-1;
+
+		root = btrfs_read_fs_root(fs_info, &root_key);
+		if (IS_ERR(root)) {
+			fprintf(stderr, "Couldn't find owner root %llu\n",
+				key.objectid);
+			return PTR_ERR(root);
+		}
+
+		path = btrfs_alloc_path();
+		if (!path)
+			return -ENOMEM;
+
+		trans = btrfs_start_transaction(root, 1);
+		if (IS_ERR(trans)) {
+			btrfs_free_path(path);
+			fprintf(stderr, "Couldn't start transaction %ld\n",
+				PTR_ERR(trans));
+			return PTR_ERR(trans);
+		}
+
+		path->lowest_level = level;
+		ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
+		if (ret < 0) {
+			fprintf(stderr, "Error searching to node %d\n", ret);
+			btrfs_free_path(path);
+			btrfs_abort_transaction(trans, ret);
+			return ret;
+		}
+		eb = path->nodes[level];
 		shift_items(root, path->nodes[level]);
+		btrfs_mark_buffer_dirty(path->nodes[level]);
+		btrfs_commit_transaction(trans, root);
 		break;
+		}
 	default:
 		ret = -EINVAL;
 		break;
 	}
-	btrfs_mark_buffer_dirty(path->nodes[level]);
-out:
-	btrfs_commit_transaction(trans, root);
-	btrfs_free_path(path);
+
 	return ret;
 }
 
-- 
2.7.4

[PATCH 2/2] btrfs-progs: tests: Test backup root retention logic

[Nikolay Borisov]

This tests ensures that the kernel correctly persists backup roots in
case the filesystem has been mounted from a backup root.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 .../misc-tests/038-backup-root-corruption/test.sh  | 50 ++++++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100755 tests/misc-tests/038-backup-root-corruption/test.sh

diff --git a/tests/misc-tests/038-backup-root-corruption/test.sh b/tests/misc-tests/038-backup-root-corruption/test.sh
new file mode 100755
index 000000000000..2fb117b3a928
--- /dev/null
+++ b/tests/misc-tests/038-backup-root-corruption/test.sh
@@ -0,0 +1,50 @@
+#!/bin/bash
+# Test that a corrupted filesystem will correctly handle writing of 
+# backup root
+
+source "$TEST_TOP/common"
+
+check_prereq mkfs.btrfs
+check_prereq btrfs
+check_prereq btrfs-corrupt-block
+
+setup_loopdevs 1
+prepare_loopdevs
+dev=${loopdevs[1]}
+
+run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$dev"
+
+# Create a file and unmount to commit some backup roots
+run_check $SUDO_HELPER mount "$dev" "$TEST_MNT"
+run_check touch "$TEST_MNT/file" && sync
+run_check $SUDO_HELPER umount "$TEST_MNT"
+
+# Ensure currently active backup slot is the expected one (slot 3)
+backup2_root_ptr=$($SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super -f "$dev" \
+	| grep -A1 "backup 2" | grep backup_tree_root | awk '{print $2}')
+
+main_root_ptr=$($SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super -f "$dev" \
+	| grep root | head -n1 | awk '{print $2}')
+
+[[ "$backup2_root_ptr" -eq "$main_root_ptr" ]] || _fail "Backup slot 2 is not in use"
+
+run_check "$TOP/btrfs-corrupt-block" -m $main_root_ptr -f generation "$dev"
+
+## should fail because the root is corrupted
+run_mustfail "Unexpected successful mount" $SUDO_HELPER mount "$dev" "$TEST_MNT"
+
+# Cycle mount with the backup to force rewrite of slot 3 
+run_check $SUDO_HELPER mount -ousebackuproot "$dev" "$TEST_MNT"
+run_check $SUDO_HELPER umount "$TEST_MNT"
+
+
+# Since we've used backup 1 as the usable root, then backup 2 should have been 
+# overwritten 
+main_root_ptr=$($SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super -f "$dev" \
+	| grep root | head -n1 | awk '{print $2}')
+backup2_new_root_ptr=$($SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super -f "$dev" \
+	| grep -A1 "backup 2" | grep backup_tree_root | awk '{print $2}')
+
+[[ "$backup2_root_ptr" -ne "$backup2_new_root_ptr" ]] || _fail "Backup 2 not overwritten"
+
+cleanup_loopdevs
-- 
2.7.4

Re: [PATCH 2/2] btrfs-progs: tests: Test backup root retention logic

[David Sterba]

On Tue, Oct 15, 2019 at 06:42:49PM +0300, Nikolay Borisov wrote:
> This tests ensures that the kernel correctly persists backup roots in
> case the filesystem has been mounted from a backup root.

The test does not work very well under a non-root user.

> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  .../misc-tests/038-backup-root-corruption/test.sh  | 50 ++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100755 tests/misc-tests/038-backup-root-corruption/test.sh
> 
> diff --git a/tests/misc-tests/038-backup-root-corruption/test.sh b/tests/misc-tests/038-backup-root-corruption/test.sh
> new file mode 100755
> index 000000000000..2fb117b3a928
> --- /dev/null
> +++ b/tests/misc-tests/038-backup-root-corruption/test.sh
> @@ -0,0 +1,50 @@
> +#!/bin/bash
> +# Test that a corrupted filesystem will correctly handle writing of 
> +# backup root
> +
> +source "$TEST_TOP/common"
> +
> +check_prereq mkfs.btrfs
> +check_prereq btrfs
> +check_prereq btrfs-corrupt-block
> +

setup_root_helper

> +setup_loopdevs 1
> +prepare_loopdevs
> +dev=${loopdevs[1]}

You can use TEST_DEV and then all the common mkfs/mount/umount helpers
will work

> +run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$dev"
> +
> +# Create a file and unmount to commit some backup roots
> +run_check $SUDO_HELPER mount "$dev" "$TEST_MNT"
> +run_check touch "$TEST_MNT/file" && sync

'touch' is on the mounted fs, so it needs $SUDO_HELPER too

> +run_check $SUDO_HELPER umount "$TEST_MNT"
> +
> +# Ensure currently active backup slot is the expected one (slot 3)
> +backup2_root_ptr=$($SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super -f "$dev" \
> +	| grep -A1 "backup 2" | grep backup_tree_root | awk '{print $2}')
> +
> +main_root_ptr=$($SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super -f "$dev" \
> +	| grep root | head -n1 | awk '{print $2}')
> +
> +[[ "$backup2_root_ptr" -eq "$main_root_ptr" ]] || _fail "Backup slot 2 is not in use"
> +
> +run_check "$TOP/btrfs-corrupt-block" -m $main_root_ptr -f generation "$dev"
> +
> +## should fail because the root is corrupted

double ##

> +run_mustfail "Unexpected successful mount" $SUDO_HELPER mount "$dev" "$TEST_MNT"
> +
> +# Cycle mount with the backup to force rewrite of slot 3 
> +run_check $SUDO_HELPER mount -ousebackuproot "$dev" "$TEST_MNT"

run_check_mount_test_dev -o usebackuproot

> +run_check $SUDO_HELPER umount "$TEST_MNT"

run_check_umount_test_dev

> +
> +

two empty lines

> +# Since we've used backup 1 as the usable root, then backup 2 should have been 
> +# overwritten 

trailing whitespace (here and in several above as well)

> +main_root_ptr=$($SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super -f "$dev" \
> +	| grep root | head -n1 | awk '{print $2}')
> +backup2_new_root_ptr=$($SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super -f "$dev" \
> +	| grep -A1 "backup 2" | grep backup_tree_root | awk '{print $2}')
> +
> +[[ "$backup2_root_ptr" -ne "$backup2_new_root_ptr" ]] || _fail "Backup 2 not overwritten"
> +
> +cleanup_loopdevs
> -- 
> 2.7.4

Re: [PATCH 2/2] btrfs-progs: tests: Test backup root retention logic

[David Sterba]

On Tue, Oct 15, 2019 at 06:42:49PM +0300, Nikolay Borisov wrote:
> This tests ensures that the kernel correctly persists backup roots in
> case the filesystem has been mounted from a backup root.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  .../misc-tests/038-backup-root-corruption/test.sh  | 50 ++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100755 tests/misc-tests/038-backup-root-corruption/test.sh
> 
> diff --git a/tests/misc-tests/038-backup-root-corruption/test.sh b/tests/misc-tests/038-backup-root-corruption/test.sh
> new file mode 100755
> index 000000000000..2fb117b3a928
> --- /dev/null
> +++ b/tests/misc-tests/038-backup-root-corruption/test.sh
> @@ -0,0 +1,50 @@
> +#!/bin/bash
> +# Test that a corrupted filesystem will correctly handle writing of 
> +# backup root
> +
> +source "$TEST_TOP/common"
> +
> +check_prereq mkfs.btrfs
> +check_prereq btrfs
> +check_prereq btrfs-corrupt-block
> +
> +setup_loopdevs 1
> +prepare_loopdevs
> +dev=${loopdevs[1]}

And the loop devices are not necessary at all

prepare_device

and be done.
> +
> +run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$dev"
> +
> +# Create a file and unmount to commit some backup roots
> +run_check $SUDO_HELPER mount "$dev" "$TEST_MNT"
> +run_check touch "$TEST_MNT/file" && sync

sync is not necessary when it's followed by umount, besides that it
syncs all fileystems so it's an unnecessary slowdown

> +run_check $SUDO_HELPER umount "$TEST_MNT"
> +
> +# Ensure currently active backup slot is the expected one (slot 3)
> +backup2_root_ptr=$($SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super -f "$dev" \

this should use run_check_stdout so we have the full output logged, as
the inspect-part is called several times I added a helper for that.

> +	| grep -A1 "backup 2" | grep backup_tree_root | awk '{print $2}')
> +
> +main_root_ptr=$($SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super -f "$dev" \
> +	| grep root | head -n1 | awk '{print $2}')
> +
> +[[ "$backup2_root_ptr" -eq "$main_root_ptr" ]] || _fail "Backup slot 2 is not in use"

[[ ]] is not necessary when it's a simple check that [ ] can do

All fixed and pushed.