[PATCH 0/5 v3] btrfs-progs: recover from failed metadata_uuid port kernel

[PATCH 0/5 v3] btrfs-progs: recover from failed metadata_uuid port kernel

[Anand Jain]

The earlier revision, v2, of this patchset consisted of 16 patches, out of
which 12 have already been merged into the devel branch.

 v2: https://patchwork.kernel.org/project/linux-btrfs/list/?series=776027

This current patchset contains the remaining unmerged patches and
addresses the reported bug:

 bug report: https://github.com/kdave/btrfs-progs/actions/runs/5956097489/job/16156138260

In v3 of this patchset, btrfs_fs_devices::inconsistent_super variable
added, which helps determine whether all the devices in the fs_devices
share the same fsid and metadata_uuid.

Anand Jain (5):
  btrfs-progs: cleanup duplicate check metadata_uuid flag
  btrfs-progs: tune use the latest bdev in fs_devices for super_copy
  btrfs-progs: add support to fix superblock with CHANGING_FSID_V2 flag
  btrfs-progs: recover from the failed btrfstune -m|M
  btrfs-progs: test btrfstune -m|M ability to fix previous failures

 kernel-shared/volumes.c                    | 193 +++++++++++++++++++--
 kernel-shared/volumes.h                    |   1 +
 tests/misc-tests/034-metadata-uuid/test.sh |  70 ++++++--
 tune/change-metadata-uuid.c                |  48 ++++-
 tune/change-uuid.c                         |   4 +-
 tune/main.c                                |   6 +-
 tune/tune.h                                |   2 -
 7 files changed, 281 insertions(+), 43 deletions(-)

-- 
2.31.1

[PATCH 1/5] btrfs-progs: cleanup duplicate check metadata_uuid flag

[Anand Jain]

The active_metadata_uuid already holds the value of the metadata_uuid
flag. Remove the check for the same flag from the super_copy, which
below patch forgot.

     btrfs-progs: Track active metadata_uuid per fs_devices

This patch should be rolled into it.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tune/main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tune/main.c b/tune/main.c
index e3b199c10dad..d344cdad06e1 100644
--- a/tune/main.c
+++ b/tune/main.c
@@ -426,8 +426,7 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 	}
 
 	if (random_fsid || (new_fsid_str && !change_metadata_uuid)) {
-		if (btrfs_fs_incompat(fs_info, METADATA_UUID) ||
-		    fs_info->fs_devices->active_metadata_uuid) {
+		if (fs_info->fs_devices->active_metadata_uuid) {
 			error(
 		"Cannot rewrite fsid while METADATA_UUID flag is active. \n"
 		"Ensure fsid and metadata_uuid match before retrying.");
-- 
2.31.1

Re: [PATCH 1/5] btrfs-progs: cleanup duplicate check metadata_uuid flag

[David Sterba]

On Fri, Aug 25, 2023 at 10:47:47PM +0800, Anand Jain wrote:
> The active_metadata_uuid already holds the value of the metadata_uuid
> flag. Remove the check for the same flag from the super_copy, which
> below patch forgot.
> 
>      btrfs-progs: Track active metadata_uuid per fs_devices
> 
> This patch should be rolled into it.

Folded to the patch, thanks.

> Signed-off-by: Anand Jain <anand.jain@oracle.com>

[PATCH 2/5] btrfs-progs: tune use the latest bdev in fs_devices for super_copy

[Anand Jain]

btrfstune relies on the superblock of the device specified in the
btrfstune argument for fs_info::super_copy. Instead, should use
fs_devices::latest_bdev.

To support for reuniting devices following past failures of
btrfstune -m|M|u|U as in the following patch, use
fs_devices::latest_bdev.

 btrfs-progs: add support to fix superblock with CHANGING_FSID_V2 flag
 btrfs-progs: recover from the failed btrfstune -m|M

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tune/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tune/main.c b/tune/main.c
index d344cdad06e1..7d2e2fb3b97d 100644
--- a/tune/main.c
+++ b/tune/main.c
@@ -290,6 +290,9 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 		goto free_out;
 	}
 
+	if (change_metadata_uuid || random_fsid || new_fsid_str)
+		ctree_flags |= OPEN_CTREE_USE_LATEST_BDEV;
+
 	root = open_ctree_fd(fd, device, 0, ctree_flags);
 
 	if (!root) {
-- 
2.31.1

Re: [PATCH 2/5] btrfs-progs: tune use the latest bdev in fs_devices for super_copy

[David Sterba]

On Fri, Aug 25, 2023 at 10:47:48PM +0800, Anand Jain wrote:
> btrfstune relies on the superblock of the device specified in the
> btrfstune argument for fs_info::super_copy. Instead, should use
> fs_devices::latest_bdev.

This lacks explanatni why it should use it, it's not obvious.

> To support for reuniting devices following past failures of
> btrfstune -m|M|u|U as in the following patch, use
> fs_devices::latest_bdev.
> 
>  btrfs-progs: add support to fix superblock with CHANGING_FSID_V2 flag
>  btrfs-progs: recover from the failed btrfstune -m|M
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Re: [PATCH 2/5] btrfs-progs: tune use the latest bdev in fs_devices for super_copy

[Anand Jain]

Currently, btrfstune relies on the superblock of the device specified
in the btrfstune argument for fs_info::super_copy. However, it should
use fs_devices::latest_bdev, as it points to the device with the highest
fs_devices::generation number. This will contain the superblock updates
that other devices may have missed and we can now support reuniting
devices following failures of btrfstune -m|M|u|U as in the patches:

    btrfs-progs: add support to fix superblock with CHANGING_FSID_V2 flag
    btrfs-progs: recover from the failed btrfstune -m|M


> This lacks explanatni why it should use it, it's not obvious.

Could you please use the commit log above, or let me know
if a reroll is preferred?

Thanks, Anand

[PATCH 3/5] btrfs-progs: add support to fix superblock with CHANGING_FSID_V2 flag

[Anand Jain]

Most of the code and functions in this patch is copied from the kernel.
Now, with this patch applied, there is no need to mount the device to
complete the incomplete 'btrfstune -m|M' command (CHANING_FSID_V2 flag).
Instead, the same command could be run, which will successfully complete
the operation.

Currently, the 'tests/misc-tests/034-metadata-uuid' tests the kernel using
four sets of disk images with CHANING_FSID_V2. Now, this test case has been
updated (as in the next patch) to test the the progs part.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 kernel-shared/volumes.c | 193 +++++++++++++++++++++++++++++++++++++---
 kernel-shared/volumes.h |   1 +
 2 files changed, 182 insertions(+), 12 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index ad006b9de315..e1426a526f6b 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -332,6 +332,159 @@ static struct btrfs_fs_devices *find_fsid(u8 *fsid, u8 *metadata_uuid)
 	return NULL;
 }
 
+static u8 *btrfs_sb_fsid_ptr(struct btrfs_super_block *sb)
+{
+	bool has_metadata_uuid = (btrfs_super_incompat_flags(sb) &
+				  BTRFS_FEATURE_INCOMPAT_METADATA_UUID);
+
+	return has_metadata_uuid ? sb->metadata_uuid : sb->fsid;
+}
+
+static bool match_fsid_fs_devices(const struct btrfs_fs_devices *fs_devices,
+				  const u8 *fsid, const u8 *metadata_fsid)
+{
+	if (memcmp(fsid, fs_devices->fsid, BTRFS_FSID_SIZE) != 0)
+		return false;
+
+	if (!metadata_fsid)
+		return true;
+
+	if (memcmp(metadata_fsid, fs_devices->metadata_uuid, BTRFS_FSID_SIZE) != 0)
+		return false;
+
+	return true;
+}
+
+/*
+ * First check if the metadata_uuid is different from the fsid in the given
+ * fs_devices. Then check if the given fsid is the same as the metadata_uuid
+ * in the fs_devices. If it is, return true; otherwise, return false.
+ */
+static inline bool check_fsid_changed(const struct btrfs_fs_devices *fs_devices,
+				      const u8 *fsid)
+{
+	return memcmp(fs_devices->fsid, fs_devices->metadata_uuid,
+		      BTRFS_FSID_SIZE) != 0 &&
+	       memcmp(fs_devices->metadata_uuid, fsid, BTRFS_FSID_SIZE) == 0;
+}
+
+static struct btrfs_fs_devices *find_fsid_with_metadata_uuid(
+				struct btrfs_super_block *disk_super)
+{
+
+	struct btrfs_fs_devices *fs_devices;
+
+	/*
+	 * Handle scanned device having completed its fsid change but
+	 * belonging to a fs_devices that was created by first scanning
+	 * a device which didn't have its fsid/metadata_uuid changed
+	 * at all and the CHANGING_FSID_V2 flag set.
+	 */
+	list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
+		if (!fs_devices->changing_fsid)
+			continue;
+
+		if (match_fsid_fs_devices(fs_devices, disk_super->metadata_uuid,
+					  fs_devices->fsid))
+			return fs_devices;
+	}
+
+	/*
+	 * Handle scanned device having completed its fsid change but
+	 * belonging to a fs_devices that was created by a device that
+	 * has an outdated pair of fsid/metadata_uuid and
+	 * CHANGING_FSID_V2 flag set.
+	 */
+	list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
+		if (!fs_devices->changing_fsid)
+			continue;
+
+		if (check_fsid_changed(fs_devices, disk_super->metadata_uuid))
+			return fs_devices;
+	}
+
+	return find_fsid(disk_super->fsid, disk_super->metadata_uuid);
+}
+
+/*
+ * Handle scanned device having its CHANGING_FSID_V2 flag set and the fs_devices
+ * being created with a disk that has already completed its fsid change. Such
+ * disk can belong to an fs which has its FSID changed or to one which doesn't.
+ * Handle both cases here.
+ */
+static struct btrfs_fs_devices *find_fsid_inprogress(
+					struct btrfs_super_block *disk_super)
+{
+	struct btrfs_fs_devices *fs_devices;
+
+	list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
+		if (fs_devices->changing_fsid)
+			continue;
+
+		if (check_fsid_changed(fs_devices,  disk_super->fsid))
+			return fs_devices;
+	}
+
+	return find_fsid(disk_super->fsid, NULL);
+}
+
+static struct btrfs_fs_devices *find_fsid_changed(
+					struct btrfs_super_block *disk_super)
+{
+	struct btrfs_fs_devices *fs_devices;
+
+	/*
+	 * Handles the case where scanned device is part of an fs that had
+	 * multiple successful changes of FSID but currently device didn't
+	 * observe it. Meaning our fsid will be different than theirs. We need
+	 * to handle two subcases :
+	 *  1 - The fs still continues to have different METADATA/FSID uuids.
+	 *  2 - The fs is switched back to its original FSID (METADATA/FSID
+	 *  are equal).
+	 */
+	list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
+		/* Changed UUIDs */
+		if (check_fsid_changed(fs_devices, disk_super->metadata_uuid) &&
+		    memcmp(fs_devices->fsid, disk_super->fsid,
+			   BTRFS_FSID_SIZE) != 0)
+			return fs_devices;
+
+		/* Unchanged UUIDs */
+		if (memcmp(fs_devices->metadata_uuid, fs_devices->fsid,
+			   BTRFS_FSID_SIZE) == 0 &&
+		    memcmp(fs_devices->fsid, disk_super->metadata_uuid,
+			   BTRFS_FSID_SIZE) == 0)
+			return fs_devices;
+	}
+
+	return NULL;
+}
+
+static struct btrfs_fs_devices *find_fsid_reverted_metadata(
+				struct btrfs_super_block *disk_super)
+{
+	struct btrfs_fs_devices *fs_devices;
+
+	/*
+	 * Handle the case where the scanned device is part of an fs whose last
+	 * metadata UUID change reverted it to the original FSID. At the same
+	 * time fs_devices was first created by another constituent device
+	 * which didn't fully observe the operation. This results in an
+	 * btrfs_fs_devices created with metadata/fsid different AND
+	 * btrfs_fs_devices::fsid_change set AND the metadata_uuid of the
+	 * fs_devices equal to the FSID of the disk.
+	 */
+	list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
+		if (!fs_devices->changing_fsid)
+			continue;
+
+		if (check_fsid_changed(fs_devices, disk_super->fsid))
+			return fs_devices;
+	}
+
+	return NULL;
+}
+
 static int device_list_add(const char *path,
 			   struct btrfs_super_block *disk_super,
 			   struct btrfs_fs_devices **fs_devices_ret)
@@ -346,11 +499,18 @@ static int device_list_add(const char *path,
 			      (BTRFS_SUPER_FLAG_CHANGING_FSID |
 			       BTRFS_SUPER_FLAG_CHANGING_FSID_V2));
 
-	if (metadata_uuid)
-		fs_devices = find_fsid(disk_super->fsid,
-				       disk_super->metadata_uuid);
-	else
-		fs_devices = find_fsid(disk_super->fsid, NULL);
+	if (changing_fsid) {
+		if (!metadata_uuid)
+			fs_devices = find_fsid_inprogress(disk_super);
+		else
+			fs_devices = find_fsid_changed(disk_super);
+	} else if (metadata_uuid) {
+		fs_devices = find_fsid_with_metadata_uuid(disk_super);
+	} else {
+		fs_devices = find_fsid_reverted_metadata(disk_super);
+		if (!fs_devices)
+			fs_devices = find_fsid(disk_super->fsid, NULL);
+	}
 
 	if (!fs_devices) {
 		fs_devices = kzalloc(sizeof(*fs_devices), GFP_NOFS);
@@ -375,7 +535,20 @@ static int device_list_add(const char *path,
 	} else {
 		device = find_device(fs_devices, devid,
 				       disk_super->dev_item.uuid);
+		/*
+		 * If this disk has been pulled into an fs devices created by
+		 * a device which had the CHANGING_FSID_V2 flag then replace the
+		 * metadata_uuid/fsid values of the fs_devices.
+		 */
+		if (fs_devices->changing_fsid &&
+		    found_transid > fs_devices->latest_generation) {
+			memcpy(fs_devices->fsid, disk_super->fsid,
+			       BTRFS_FSID_SIZE);
+			memcpy(fs_devices->metadata_uuid,
+			       btrfs_sb_fsid_ptr(disk_super), BTRFS_FSID_SIZE);
+		}
 	}
+
 	if (!device) {
 		device = kzalloc(sizeof(*device), GFP_NOFS);
 		if (!device) {
@@ -429,19 +602,15 @@ static int device_list_add(const char *path,
                 device->name = name;
         }
 
-	/*
-	 * If changing_fsid the fs_devices will still hold the status from
-	 * the other devices.
-	 */
 	if (changing_fsid)
-		fs_devices->changing_fsid = true;
-	if (metadata_uuid)
-		fs_devices->active_metadata_uuid = true;
+		fs_devices->inconsistent_super = changing_fsid;
 
 	if (found_transid > fs_devices->latest_generation) {
 		fs_devices->latest_devid = devid;
 		fs_devices->latest_generation = found_transid;
 		fs_devices->total_devices = device->total_devs;
+		fs_devices->active_metadata_uuid = metadata_uuid;
+		fs_devices->changing_fsid = changing_fsid;
 	}
 	if (fs_devices->lowest_devid > devid) {
 		fs_devices->lowest_devid = devid;
diff --git a/kernel-shared/volumes.h b/kernel-shared/volumes.h
index 7f571bddee87..593fe75ed087 100644
--- a/kernel-shared/volumes.h
+++ b/kernel-shared/volumes.h
@@ -104,6 +104,7 @@ struct btrfs_fs_devices {
 
 	bool changing_fsid;
 	bool active_metadata_uuid;
+	bool inconsistent_super;
 };
 
 struct btrfs_bio_stripe {
-- 
2.31.1

[PATCH 4/5] btrfs-progs: recover from the failed btrfstune -m|M

[Anand Jain]

Currently, to fix device following the write failure of one or more devices
during btrfstune -m|M, we rely on the kernel's ability to reassemble devices,
even when they possess distinct fsids.

Kernel hinges combinations of metadata_uuid and generation number, with
additional cues taken from the fsid and the BTRFS_SUPER_FLAG_CHANGING_FSID_V2
flag. This patch adds this logic to btrfs-progs.

In complex scenarios (such as multiple fsids with the same metadata_uuid and
matching generation), user intervention becomes necessary to resolve the
situations which btrfs-prog can do better.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tune/change-metadata-uuid.c | 48 ++++++++++++++++++++++++++++++-------
 tune/change-uuid.c          |  4 ++--
 tune/tune.h                 |  2 --
 3 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/tune/change-metadata-uuid.c b/tune/change-metadata-uuid.c
index ada3149ad549..642977c3747b 100644
--- a/tune/change-metadata-uuid.c
+++ b/tune/change-metadata-uuid.c
@@ -21,9 +21,31 @@
 #include "kernel-shared/uapi/btrfs.h"
 #include "kernel-shared/ctree.h"
 #include "kernel-shared/transaction.h"
+#include "kernel-shared/volumes.h"
 #include "common/messages.h"
 #include "tune/tune.h"
 
+/*
+ * Return 0 for no unfinished metadata_uuid change.
+ * Return >0 for unfinished metadata_uuid change, and restore unfinished
+ * fsid/metadata_uuid into fsid_ret/metadata_uuid_ret.
+ */
+static int check_unfinished_metadata_uuid(struct btrfs_fs_info *fs_info,
+					  uuid_t fsid_ret,
+					  uuid_t metadata_uuid_ret)
+{
+	struct btrfs_root *tree_root = fs_info->tree_root;
+
+	if (fs_info->fs_devices->inconsistent_super) {
+		memcpy(fsid_ret, fs_info->super_copy->fsid, BTRFS_FSID_SIZE);
+		read_extent_buffer(tree_root->node, metadata_uuid_ret,
+				btrfs_header_chunk_tree_uuid(tree_root->node),
+				BTRFS_UUID_SIZE);
+		return 1;
+	}
+	return 0;
+}
+
 int set_metadata_uuid(struct btrfs_root *root, const char *new_fsid_string)
 {
 	struct btrfs_super_block *disk_super;
@@ -45,15 +67,25 @@ int set_metadata_uuid(struct btrfs_root *root, const char *new_fsid_string)
 		return 1;
 	}
 
-	if (check_unfinished_fsid_change(root->fs_info, fsid, metadata_uuid)) {
-		error("UUID rewrite in progress, cannot change metadata_uuid");
-		return 1;
-	}
+	if (check_unfinished_metadata_uuid(root->fs_info, fsid,
+					   metadata_uuid)) {
+		if (new_fsid_string) {
+			uuid_t tmp;
 
-	if (new_fsid_string)
-		uuid_parse(new_fsid_string, fsid);
-	else
-		uuid_generate(fsid);
+			uuid_parse(new_fsid_string, tmp);
+			if (memcmp(tmp, fsid, BTRFS_FSID_SIZE)) {
+				error(
+		"new fsid %s is not the same with unfinished fsid change",
+				      new_fsid_string);
+				return -EINVAL;
+			}
+		}
+	} else {
+		if (new_fsid_string)
+			uuid_parse(new_fsid_string, fsid);
+		else
+			uuid_generate(fsid);
+	}
 
 	new_fsid = (memcmp(fsid, disk_super->fsid, BTRFS_FSID_SIZE) != 0);
 
diff --git a/tune/change-uuid.c b/tune/change-uuid.c
index 30cfb145459f..e81b7980bb69 100644
--- a/tune/change-uuid.c
+++ b/tune/change-uuid.c
@@ -210,8 +210,8 @@ static int change_fsid_done(struct btrfs_fs_info *fs_info)
  * Return >0 for unfinished fsid change, and restore unfinished fsid/
  * chunk_tree_id into fsid_ret/chunk_id_ret.
  */
-int check_unfinished_fsid_change(struct btrfs_fs_info *fs_info,
-				 uuid_t fsid_ret, uuid_t chunk_id_ret)
+static int check_unfinished_fsid_change(struct btrfs_fs_info *fs_info,
+					uuid_t fsid_ret, uuid_t chunk_id_ret)
 {
 	struct btrfs_root *tree_root = fs_info->tree_root;
 
diff --git a/tune/tune.h b/tune/tune.h
index 0ef249d89eee..e84cc336846c 100644
--- a/tune/tune.h
+++ b/tune/tune.h
@@ -24,8 +24,6 @@ struct btrfs_fs_info;
 
 int update_seeding_flag(struct btrfs_root *root, const char *device, int set_flag, int force);
 
-int check_unfinished_fsid_change(struct btrfs_fs_info *fs_info,
-				 uuid_t fsid_ret, uuid_t chunk_id_ret);
 int change_uuid(struct btrfs_fs_info *fs_info, const char *new_fsid_str);
 int set_metadata_uuid(struct btrfs_root *root, const char *uuid_string);
 
-- 
2.31.1

[PATCH 5/5] btrfs-progs: test btrfstune -m|M ability to fix previous failures

[Anand Jain]

The misc-test/034-metadata_uuid test case, has four sets of disk images to
simulate failed writes during btrfstune -m|M operations. As of now, this
tests kernel only. Update the test case to verify btrfstune -m|M's
capacity to recover from the same scenarios.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/misc-tests/034-metadata-uuid/test.sh | 70 ++++++++++++++++------
 1 file changed, 53 insertions(+), 17 deletions(-)

diff --git a/tests/misc-tests/034-metadata-uuid/test.sh b/tests/misc-tests/034-metadata-uuid/test.sh
index 479c7da7a5b2..0b06f1266f57 100755
--- a/tests/misc-tests/034-metadata-uuid/test.sh
+++ b/tests/misc-tests/034-metadata-uuid/test.sh
@@ -195,13 +195,42 @@ check_multi_fsid_unchanged() {
 	check_flag_cleared "$1" "$2"
 }
 
-failure_recovery() {
+failure_recovery_progs() {
+	local image1
+	local image2
+	local loop1
+	local loop2
+	local devcount
+
+	image1=$(extract_image "$1")
+	image2=$(extract_image "$2")
+	loop1=$(run_check_stdout $SUDO_HELPER losetup --find --show "$image1")
+	loop2=$(run_check_stdout $SUDO_HELPER losetup --find --show "$image2")
+
+	run_check $SUDO_HELPER udevadm settle
+
+	# Scan to make sure btrfs detects both devices before trying to mount
+	#run_check "$TOP/btrfstune" -m --noscan --device="$loop1" "$loop2"
+	run_check "$TOP/btrfstune" -m "$loop2"
+
+	# perform any specific check
+	"$3" "$loop1" "$loop2"
+
+	# cleanup
+	run_check $SUDO_HELPER losetup -d "$loop1"
+	run_check $SUDO_HELPER losetup -d "$loop2"
+	rm -f -- "$image1" "$image2"
+}
+
+failure_recovery_kernel() {
 	local image1
 	local image2
 	local loop1
 	local loop2
 	local devcount
 
+	reload_btrfs
+
 	image1=$(extract_image "$1")
 	image2=$(extract_image "$2")
 	loop1=$(run_check_stdout $SUDO_HELPER losetup --find --show "$image1")
@@ -226,47 +255,55 @@ failure_recovery() {
 	rm -f -- "$image1" "$image2"
 }
 
+failure_recovery() {
+	failure_recovery_progs $@
+	failure_recovery_kernel $@
+}
+
 reload_btrfs() {
 	run_check $SUDO_HELPER rmmod btrfs
 	run_check $SUDO_HELPER modprobe btrfs
 }
 
-# for full coverage we need btrfs to actually be a module
-modinfo btrfs > /dev/null 2>&1 || _not_run "btrfs must be a module"
-run_mayfail $SUDO_HELPER modprobe -r btrfs || _not_run "btrfs must be unloadable"
-run_mayfail $SUDO_HELPER modprobe btrfs || _not_run "loading btrfs module failed"
+test_progs() {
+	run_check_mkfs_test_dev
+	check_btrfstune
+
+	run_check_mkfs_test_dev
+	check_dump_super_output
 
-run_check_mkfs_test_dev
-check_btrfstune
+	run_check_mkfs_test_dev
+	check_image_restore
+}
+
+check_kernel_reloadable() {
+	# for full coverage we need btrfs to actually be a module
+	modinfo btrfs > /dev/null 2>&1 || _not_run "btrfs must be a module"
+	run_mayfail $SUDO_HELPER modprobe -r btrfs || _not_run "btrfs must be unloadable"
+	run_mayfail $SUDO_HELPER modprobe btrfs || _not_run "loading btrfs module failed"
+}
 
-run_check_mkfs_test_dev
-check_dump_super_output
+check_kernel_reloadable
 
-run_check_mkfs_test_dev
-check_image_restore
+test_progs
 
 # disk1 is an image which has no metadata uuid flags set and disk2 is part of
 # the same fs but has the in-progress flag set. Test that whicever is scanned
 # first will result in consistent filesystem.
 failure_recovery "./disk1.raw.xz" "./disk2.raw.xz" check_inprogress_flag
-reload_btrfs
 failure_recovery "./disk2.raw.xz" "./disk1.raw.xz" check_inprogress_flag
 
 # disk4 contains an image in with the in-progress flag set and disk 3 is part
 # of the same filesystem but has both METADATA_UUID incompat and a new
 # metadata uuid set. So disk 3 must always take precedence
-reload_btrfs
 failure_recovery "./disk3.raw.xz" "./disk4.raw.xz" check_completed
-reload_btrfs
 failure_recovery "./disk4.raw.xz" "./disk3.raw.xz" check_completed
 
 # disk5 contains an image which has undergone a successful fsid change more
 # than once, disk6 on the other hand is member of the same filesystem but
 # hasn't completed its last change. Thus it has both the FSID_CHANGING flag set
 # and METADATA_UUID flag set.
-reload_btrfs
 failure_recovery "./disk5.raw.xz" "./disk6.raw.xz" check_multi_fsid_change
-reload_btrfs
 failure_recovery "./disk6.raw.xz" "./disk5.raw.xz" check_multi_fsid_change
 
 # disk7 contains an image which has undergone a successful fsid change once to
@@ -275,5 +312,4 @@ failure_recovery "./disk6.raw.xz" "./disk5.raw.xz" check_multi_fsid_change
 # during the process change. So disk 7 looks as if it never underwent fsid change
 # and disk 8 has FSID_CHANGING_FLAG and METADATA_UUID but is stale.
 failure_recovery "./disk7.raw.xz" "./disk8.raw.xz" check_multi_fsid_unchanged
-reload_btrfs
 failure_recovery "./disk8.raw.xz" "./disk7.raw.xz" check_multi_fsid_unchanged
-- 
2.31.1

Re: [PATCH 0/5 v3] btrfs-progs: recover from failed metadata_uuid port kernel

[David Sterba]

On Fri, Aug 25, 2023 at 10:47:46PM +0800, Anand Jain wrote:
> The earlier revision, v2, of this patchset consisted of 16 patches, out of
> which 12 have already been merged into the devel branch.
> 
>  v2: https://patchwork.kernel.org/project/linux-btrfs/list/?series=776027
> 
> This current patchset contains the remaining unmerged patches and
> addresses the reported bug:
> 
>  bug report: https://github.com/kdave/btrfs-progs/actions/runs/5956097489/job/16156138260
> 
> In v3 of this patchset, btrfs_fs_devices::inconsistent_super variable
> added, which helps determine whether all the devices in the fs_devices
> share the same fsid and metadata_uuid.
> 
> Anand Jain (5):
>   btrfs-progs: cleanup duplicate check metadata_uuid flag
>   btrfs-progs: tune use the latest bdev in fs_devices for super_copy
>   btrfs-progs: add support to fix superblock with CHANGING_FSID_V2 flag
>   btrfs-progs: recover from the failed btrfstune -m|M
>   btrfs-progs: test btrfstune -m|M ability to fix previous failures

The first patch has been folded in.  I'll postpone the rest of the
series after the next release, ETA 6.5.1, I need to refresh the
metadata_uuid error states.