diff mbox series

[1/2] btrfs-progs: tune: delete the csum change item after converting the fs

Message ID bda7d23000be5d97a782e8c56c99ddb66ca7a2bb.1684802060.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: tune: fix the leftover csum change item and add a test case for it | expand

Commit Message

Qu Wenruo May 23, 2023, 12:37 a.m. UTC
[BUG]
Doing the following csum change in a row, it would fail:

 # mkfs.btrfs -f --csum crc32c $dev
 # btrfstune --csum sha256 $dev
 # btrfstune --csum crc32c $dev
 # btrfstune --csum sha256 $dev
 WARNING: Experimental build with unstable or unfinished features
 WARNING: Switching checksums is experimental, do not use for valuable data!

 Proceed to switch checksums
 ERROR: failed to insert csum change item: File exists
 ERROR: failed to generate new data csums: File exists
 WARNING: reserved space leaked, flag=0x4 bytes_reserved=16384
 extent buffer leak: start 30572544 len 16384
 extent buffer leak: start 30441472 len 16384
 WARNING: dirty eb leak (aborted trans): start 30441472 len 16384

[CAUSE]
During every csum change operation, btrfstune would insert an temporaray
csum change item into root tree.

But unfortunately after the conversion btrfstune doesn't properly delete
the csum change item, result the following items in the root tree:

	item 10 key (CSUM_CHANGE TEMPORARY_ITEM 0) itemoff 13423 itemsize 0
		temporary item objectid CSUM_CHANGE offset 0
		target csum type crc32c (0)
	item 11 key (CSUM_CHANGE TEMPORARY_ITEM 2) itemoff 13423 itemsize 0
		temporary item objectid CSUM_CHANGE offset 2
		target csum type sha256 (2)

Thus at the last conversion try to go back to SHA256, we failed to
insert the same item, and caused the above error.

[FIX]
After finishing the metadata csum conversion, do a proper removal of the
csum item.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tune/change-csum.c | 42 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/tune/change-csum.c b/tune/change-csum.c
index c8809300a143..b5efc3a8807f 100644
--- a/tune/change-csum.c
+++ b/tune/change-csum.c
@@ -583,10 +583,13 @@  out:
 	btrfs_release_path(&path);
 
 	/*
-	 * Finish the change by clearing the csum change flag and update the superblock
-	 * csum type.
+	 * Finish the change by clearing the csum change flag, update the superblock
+	 * csum type, and delete the csum change item in the fs with new csum type.
 	 */
 	if (ret == 0) {
+		struct btrfs_root *tree_root = fs_info->tree_root;
+		struct btrfs_trans_handle *trans;
+
 		u64 super_flags = btrfs_super_flags(fs_info->super_copy);
 
 		btrfs_set_super_csum_type(fs_info->super_copy, new_csum_type);
@@ -596,11 +599,42 @@  out:
 
 		fs_info->csum_type = new_csum_type;
 		fs_info->csum_size = btrfs_csum_type_size(new_csum_type);
+		fs_info->skip_csum_check = 0;
 
-		ret = write_all_supers(fs_info);
+		trans = btrfs_start_transaction(tree_root, 1);
+		if (IS_ERR(trans)) {
+			ret = PTR_ERR(trans);
+			errno = -ret;
+			error("failed to start new transaction with new csum type: %m");
+			return ret;
+		}
+		key.objectid = BTRFS_CSUM_CHANGE_OBJECTID;
+		key.type = BTRFS_TEMPORARY_ITEM_KEY;
+		key.offset = new_csum_type;
+
+		ret = btrfs_search_slot(trans, tree_root, &key, &path, -1, 1);
+		if (ret > 0)
+			ret = -ENOENT;
 		if (ret < 0) {
 			errno = -ret;
-			error("failed to write super blocks: %m");
+			error("failed to locate the csum change item: %m");
+			btrfs_release_path(&path);
+			btrfs_abort_transaction(trans, ret);
+			return ret;
+		}
+		ret = btrfs_del_item(trans, tree_root, &path);
+		if (ret < 0) {
+			errno = -ret;
+			error("failed to delete the csum change item: %m");
+			btrfs_release_path(&path);
+			btrfs_abort_transaction(trans, ret);
+			return ret;
+		}
+		btrfs_release_path(&path);
+		ret = btrfs_commit_transaction(trans, tree_root);
+		if (ret < 0) {
+			errno = -ret;
+			error("failed to finalize the csum change: %m");
 		}
 	}
 	return ret;