[PATCH 0/2 v2] btrfs: reject device with CHANGING_FSID_V2 flag

[PATCH 0/2 v2] btrfs: reject device with CHANGING_FSID_V2 flag

[Anand Jain]

v2: Splits the patch into two parts: one for the new code to reject
devices with CHANGING_FSID_V2 and the second to remove the unused code.

The btrfs-progs code to reunite devices with the CHANGING_FSID_V2 flag,
which is ported from the kernel, can be found in the following btrfs-progs
patchset:

 [PATCH 0/4 v4] btrfs-progs: recover from failed metadata_uuid port kernel
    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

Furthermore, when the kernel stops supporting the CHANGING_FSID_V2 flag,
we will need to update the btrfs-progs test case accordingly:

    btrfs-progs: misc-tests/034-metadata-uuid remove kernel support

v1:
https://lore.kernel.org/linux-btrfs/02d59bdd7a8b778deb17e300354558498db59376.1692178060.git.anand.jain@oracle.com


Anand Jain (2):
  btrfs: reject device with CHANGING_FSID_V2
  btrfs: remove unused code related to the CHANGING_FSID_V2 flag

 fs/btrfs/disk-io.c |  10 ---
 fs/btrfs/volumes.c | 166 ++++-----------------------------------------
 fs/btrfs/volumes.h |   1 -
 3 files changed, 13 insertions(+), 164 deletions(-)

-- 
2.38.1

[PATCH 1/2] btrfs: reject device with CHANGING_FSID_V2

[Anand Jain]

The BTRFS_SUPER_FLAG_CHANGING_FSID_V2 flag indicates a transient state
where the device in the userspace btrfstune -m|-M operation failed to
complete changing the fsid.

This flag makes the kernel to automatically determine the other
partner devices to which a given device can be associated, based on the
fsid, metadata_uuid and generation values.

btrfstune -m|M feature is especially useful in virtual cloud setups, where
compute instances (disk images) are quickly copied, fsid changed, and
launched. Given numerous disk images with the same metadata_uuid but
different fsid, there's no clear way a device can be correctly assembled
with the proper partners when the CHANGING_FSID_V2 flag is set. So, the
disk could be assembled incorrectly, as in the example below:

Before this patch:

Consider the following two filesystems:
   /dev/loop[2-3] are raw copies of /dev/loop[0-1] and the btrsftune -m
operation fails.

In this scenario, as the /dev/loop0's fsid change is interrupted, and the
CHANGING_FSID_V2 flag is set as shown below.

  $ p="device|devid|^metadata_uuid|^fsid|^incom|^generation|^flags"

  $ btrfs inspect dump-super /dev/loop0 | egrep '$p'
  superblock: bytenr=65536, device=/dev/loop0
  flags			0x1000000001
  fsid			7d4b4b93-2b27-4432-b4e4-4be1fbccbd45
  metadata_uuid		bb040a9f-233a-4de2-ad84-49aa5a28059b
  generation		9
  num_devices		2
  incompat_flags	0x741
  dev_item.devid	1

  $ btrfs inspect dump-super /dev/loop1 | egrep '$p'
  superblock: bytenr=65536, device=/dev/loop1
  flags			0x1
  fsid			11d2af4d-1b71-45a9-83f6-f2100766939d
  metadata_uuid		bb040a9f-233a-4de2-ad84-49aa5a28059b
  generation		10
  num_devices		2
  incompat_flags	0x741
  dev_item.devid	2

  $ btrfs inspect dump-super /dev/loop2 | egrep '$p'
  superblock: bytenr=65536, device=/dev/loop2
  flags			0x1
  fsid			7d4b4b93-2b27-4432-b4e4-4be1fbccbd45
  metadata_uuid		bb040a9f-233a-4de2-ad84-49aa5a28059b
  generation		8
  num_devices		2
  incompat_flags	0x741
  dev_item.devid	1

  $ btrfs inspect dump-super /dev/loop3 | egrep '$p'
  superblock: bytenr=65536, device=/dev/loop3
  flags			0x1
  fsid			7d4b4b93-2b27-4432-b4e4-4be1fbccbd45
  metadata_uuid		bb040a9f-233a-4de2-ad84-49aa5a28059b
  generation		8
  num_devices		2
  incompat_flags	0x741
  dev_item.devid	2

It is normal that some devices aren't instantly discovered during
system boot or iSCSI discovery. The controlled scan below demonstrates
this.

  $ btrfs device scan --forget
  $ btrfs device scan /dev/loop0
  Scanning for btrfs filesystems on '/dev/loop0'
  $ mount /dev/loop3 /btrfs
  $ btrfs filesystem show -m
  Label: none  uuid: 7d4b4b93-2b27-4432-b4e4-4be1fbccbd45
	Total devices 2 FS bytes used 144.00KiB
	devid    1 size 300.00MiB used 48.00MiB path /dev/loop0
	devid    2 size 300.00MiB used 40.00MiB path /dev/loop3

/dev/loop0 and /dev/loop3 are incorrectly partnered.

This kernel patch removes functions and code connected to the
CHANGING_FSID_V2 flag.

With this patch, now devices with the CHANGING_FSID_V2 flag are rejected.
And its partner will fail to mount with the extra -o degraded option.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
Moreover, a btrfs-progs patch (below) has eliminated the use of the
CHANGING_FSID_V2 flag entirely:

   [PATCH] btrfs-progs: btrfstune -m|M remove 2-stage commit

And we solve the compatability concerns as below:

  New-kernel new-progs - has no CHANGING_FSID_V2 flag.
  Old-kernel new-progs - has no CHANGING_FSID_V2 flag, kernel code unused.
  Old-kernel old-progs - bug may occur.
  New-kernel old-progs - Should use host with the newer btrfs-progs to fix.

For legacy systems to help fix such a condition in the userspace instead
we have the below patchset which ports of kernel's CHANGING_FSID_V2 code.

   [PATCH 0/4 v4] btrfs-progs: recover from failed metadata_uuid port kernel

And if it couldn't fix in some cases, users can use manually reunite,
with the patchset:

   [PATCH 00/10] btrfs-progs: check and tune: add device and noscan options

 fs/btrfs/disk-io.c | 10 ----------
 fs/btrfs/volumes.c |  7 +++++++
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index dc577b3c53f6..95746ddf7dc3 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3173,7 +3173,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
 	u32 nodesize;
 	u32 stripesize;
 	u64 generation;
-	u64 features;
 	u16 csum_type;
 	struct btrfs_super_block *disk_super;
 	struct btrfs_fs_info *fs_info = btrfs_sb(sb);
@@ -3255,15 +3254,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
 
 	disk_super = fs_info->super_copy;
 
-
-	features = btrfs_super_flags(disk_super);
-	if (features & BTRFS_SUPER_FLAG_CHANGING_FSID_V2) {
-		features &= ~BTRFS_SUPER_FLAG_CHANGING_FSID_V2;
-		btrfs_set_super_flags(disk_super, features);
-		btrfs_info(fs_info,
-			"found metadata UUID change in progress flag, clearing");
-	}
-
 	memcpy(fs_info->super_for_commit, fs_info->super_copy,
 	       sizeof(*fs_info->super_for_commit));
 
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index bc8d46cbc7c5..c845c60ec207 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -791,6 +791,13 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 	bool fsid_change_in_progress = (btrfs_super_flags(disk_super) &
 					BTRFS_SUPER_FLAG_CHANGING_FSID_V2);
 
+	if (fsid_change_in_progress) {
+		btrfs_err(NULL,
+"device %s has incomplete FSID changes please use btrfstune to complete",
+			  path);
+		return ERR_PTR(-EINVAL);
+	}
+
 	error = lookup_bdev(path, &path_devt);
 	if (error) {
 		btrfs_err(NULL, "failed to lookup block device for path %s: %d",
-- 
2.38.1

[PATCH 2/2] btrfs: remove unused code related to the CHANGING_FSID_V2 flag

[Anand Jain]

The commit ("btrfs: reject devices with CHANGING_FSID_V2") has stopped
the assembly of devices with the CHANGING_FSID_V2 flag in the kernel.
Remove the related unused code.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c | 161 ++-------------------------------------------
 fs/btrfs/volumes.h |   1 -
 2 files changed, 7 insertions(+), 155 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index c845c60ec207..8ebd1427a6f3 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -455,58 +455,6 @@ static noinline struct btrfs_fs_devices *find_fsid(
 	return NULL;
 }
 
-/*
- * 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->fsid_change)
-			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->fsid_change)
-			continue;
-
-		if (check_fsid_changed(fs_devices, disk_super->metadata_uuid))
-			return fs_devices;
-	}
-
-	return find_fsid(disk_super->fsid, disk_super->metadata_uuid);
-}
-
-
 static int
 btrfs_get_bdev_and_sb(const char *device_path, blk_mode_t flags, void *holder,
 		      int flush, struct block_device **bdev,
@@ -690,84 +638,6 @@ u8 *btrfs_sb_fsid_ptr(struct btrfs_super_block *sb)
 	return has_metadata_uuid ? sb->metadata_uuid : sb->fsid;
 }
 
-/*
- * 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->fsid_change)
-			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->fsid_change)
-			continue;
-
-		if (check_fsid_changed(fs_devices, disk_super->fsid))
-			return fs_devices;
-	}
-
-	return NULL;
-}
 /*
  * Add new device to list of registered devices
  *
@@ -788,10 +658,9 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 	int error;
 	bool has_metadata_uuid = (btrfs_super_incompat_flags(disk_super) &
 		BTRFS_FEATURE_INCOMPAT_METADATA_UUID);
-	bool fsid_change_in_progress = (btrfs_super_flags(disk_super) &
-					BTRFS_SUPER_FLAG_CHANGING_FSID_V2);
 
-	if (fsid_change_in_progress) {
+	if (btrfs_super_flags(disk_super) &
+	    BTRFS_SUPER_FLAG_CHANGING_FSID_V2) {
 		btrfs_err(NULL,
 "device %s has incomplete FSID changes please use btrfstune to complete",
 			  path);
@@ -805,20 +674,13 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 		return ERR_PTR(error);
 	}
 
-	if (fsid_change_in_progress) {
-		if (!has_metadata_uuid)
-			fs_devices = find_fsid_inprogress(disk_super);
-		else
-			fs_devices = find_fsid_changed(disk_super);
-	} else if (has_metadata_uuid) {
-		fs_devices = find_fsid_with_metadata_uuid(disk_super);
+	if (has_metadata_uuid) {
+		fs_devices = find_fsid(disk_super->fsid,
+				       disk_super->metadata_uuid);
 	} else {
-		fs_devices = find_fsid_reverted_metadata(disk_super);
-		if (!fs_devices)
-			fs_devices = find_fsid(disk_super->fsid, NULL);
+		fs_devices = find_fsid(disk_super->fsid, NULL);
 	}
 
-
 	if (!fs_devices) {
 		fs_devices = alloc_fs_devices(disk_super->fsid);
 		if (has_metadata_uuid)
@@ -828,8 +690,6 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 		if (IS_ERR(fs_devices))
 			return ERR_CAST(fs_devices);
 
-		fs_devices->fsid_change = fsid_change_in_progress;
-
 		mutex_lock(&fs_devices->device_list_mutex);
 		list_add(&fs_devices->fs_list, &fs_uuids);
 
@@ -843,18 +703,11 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 		mutex_lock(&fs_devices->device_list_mutex);
 		device = btrfs_find_device(fs_devices, &args);
 
-		/*
-		 * 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->fsid_change &&
-		    found_transid > fs_devices->latest_generation) {
+		if (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);
-			fs_devices->fsid_change = false;
 		}
 	}
 
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index a0e76bb20140..e485e6a3e52c 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -362,7 +362,6 @@ struct btrfs_fs_devices {
 	bool rotating;
 	/* Devices support TRIM/discard commands. */
 	bool discardable;
-	bool fsid_change;
 	/* The filesystem is a seed filesystem. */
 	bool seeding;
 
-- 
2.38.1

Re: [PATCH 1/2] btrfs: reject device with CHANGING_FSID_V2

[David Sterba]

On Thu, Sep 21, 2023 at 05:51:13AM +0800, Anand Jain wrote:
> The BTRFS_SUPER_FLAG_CHANGING_FSID_V2 flag indicates a transient state
> where the device in the userspace btrfstune -m|-M operation failed to
> complete changing the fsid.
> 
> This flag makes the kernel to automatically determine the other
> partner devices to which a given device can be associated, based on the
> fsid, metadata_uuid and generation values.
> 
> btrfstune -m|M feature is especially useful in virtual cloud setups, where
> compute instances (disk images) are quickly copied, fsid changed, and
> launched. Given numerous disk images with the same metadata_uuid but
> different fsid, there's no clear way a device can be correctly assembled
> with the proper partners when the CHANGING_FSID_V2 flag is set. So, the
> disk could be assembled incorrectly, as in the example below:
> 
> Before this patch:
> 
> Consider the following two filesystems:
>    /dev/loop[2-3] are raw copies of /dev/loop[0-1] and the btrsftune -m
> operation fails.
> 
> In this scenario, as the /dev/loop0's fsid change is interrupted, and the
> CHANGING_FSID_V2 flag is set as shown below.
> 
>   $ p="device|devid|^metadata_uuid|^fsid|^incom|^generation|^flags"
> 
>   $ btrfs inspect dump-super /dev/loop0 | egrep '$p'
>   superblock: bytenr=65536, device=/dev/loop0
>   flags			0x1000000001
>   fsid			7d4b4b93-2b27-4432-b4e4-4be1fbccbd45
>   metadata_uuid		bb040a9f-233a-4de2-ad84-49aa5a28059b
>   generation		9
>   num_devices		2
>   incompat_flags	0x741
>   dev_item.devid	1
> 
>   $ btrfs inspect dump-super /dev/loop1 | egrep '$p'
>   superblock: bytenr=65536, device=/dev/loop1
>   flags			0x1
>   fsid			11d2af4d-1b71-45a9-83f6-f2100766939d
>   metadata_uuid		bb040a9f-233a-4de2-ad84-49aa5a28059b
>   generation		10
>   num_devices		2
>   incompat_flags	0x741
>   dev_item.devid	2
> 
>   $ btrfs inspect dump-super /dev/loop2 | egrep '$p'
>   superblock: bytenr=65536, device=/dev/loop2
>   flags			0x1
>   fsid			7d4b4b93-2b27-4432-b4e4-4be1fbccbd45
>   metadata_uuid		bb040a9f-233a-4de2-ad84-49aa5a28059b
>   generation		8
>   num_devices		2
>   incompat_flags	0x741
>   dev_item.devid	1
> 
>   $ btrfs inspect dump-super /dev/loop3 | egrep '$p'
>   superblock: bytenr=65536, device=/dev/loop3
>   flags			0x1
>   fsid			7d4b4b93-2b27-4432-b4e4-4be1fbccbd45
>   metadata_uuid		bb040a9f-233a-4de2-ad84-49aa5a28059b
>   generation		8
>   num_devices		2
>   incompat_flags	0x741
>   dev_item.devid	2
> 
> It is normal that some devices aren't instantly discovered during
> system boot or iSCSI discovery. The controlled scan below demonstrates
> this.
> 
>   $ btrfs device scan --forget
>   $ btrfs device scan /dev/loop0
>   Scanning for btrfs filesystems on '/dev/loop0'
>   $ mount /dev/loop3 /btrfs
>   $ btrfs filesystem show -m
>   Label: none  uuid: 7d4b4b93-2b27-4432-b4e4-4be1fbccbd45
> 	Total devices 2 FS bytes used 144.00KiB
> 	devid    1 size 300.00MiB used 48.00MiB path /dev/loop0
> 	devid    2 size 300.00MiB used 40.00MiB path /dev/loop3
> 
> /dev/loop0 and /dev/loop3 are incorrectly partnered.
> 
> This kernel patch removes functions and code connected to the
> CHANGING_FSID_V2 flag.
> 
> With this patch, now devices with the CHANGING_FSID_V2 flag are rejected.
> And its partner will fail to mount with the extra -o degraded option.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> Moreover, a btrfs-progs patch (below) has eliminated the use of the
> CHANGING_FSID_V2 flag entirely:
> 
>    [PATCH] btrfs-progs: btrfstune -m|M remove 2-stage commit
> 
> And we solve the compatability concerns as below:
> 
>   New-kernel new-progs - has no CHANGING_FSID_V2 flag.
>   Old-kernel new-progs - has no CHANGING_FSID_V2 flag, kernel code unused.
>   Old-kernel old-progs - bug may occur.
>   New-kernel old-progs - Should use host with the newer btrfs-progs to fix.
> 
> For legacy systems to help fix such a condition in the userspace instead
> we have the below patchset which ports of kernel's CHANGING_FSID_V2 code.
> 
>    [PATCH 0/4 v4] btrfs-progs: recover from failed metadata_uuid port kernel
> 
> And if it couldn't fix in some cases, users can use manually reunite,
> with the patchset:
> 
>    [PATCH 00/10] btrfs-progs: check and tune: add device and noscan options
> 
>  fs/btrfs/disk-io.c | 10 ----------
>  fs/btrfs/volumes.c |  7 +++++++
>  2 files changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index dc577b3c53f6..95746ddf7dc3 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3173,7 +3173,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>  	u32 nodesize;
>  	u32 stripesize;
>  	u64 generation;
> -	u64 features;
>  	u16 csum_type;
>  	struct btrfs_super_block *disk_super;
>  	struct btrfs_fs_info *fs_info = btrfs_sb(sb);
> @@ -3255,15 +3254,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>  
>  	disk_super = fs_info->super_copy;
>  
> -
> -	features = btrfs_super_flags(disk_super);
> -	if (features & BTRFS_SUPER_FLAG_CHANGING_FSID_V2) {
> -		features &= ~BTRFS_SUPER_FLAG_CHANGING_FSID_V2;
> -		btrfs_set_super_flags(disk_super, features);
> -		btrfs_info(fs_info,
> -			"found metadata UUID change in progress flag, clearing");
> -	}

This is removed from the mount path but it's still rejected because at
some point the device scanning will be called and will return -EINVAL.

> -
>  	memcpy(fs_info->super_for_commit, fs_info->super_copy,
>  	       sizeof(*fs_info->super_for_commit));
>  
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index bc8d46cbc7c5..c845c60ec207 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -791,6 +791,13 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>  	bool fsid_change_in_progress = (btrfs_super_flags(disk_super) &
>  					BTRFS_SUPER_FLAG_CHANGING_FSID_V2);
>  
> +	if (fsid_change_in_progress) {
> +		btrfs_err(NULL,
> +"device %s has incomplete FSID changes please use btrfstune to complete",

This could say it's specifically metadata_uuid.

> +			  path);
> +		return ERR_PTR(-EINVAL);

We could probably return -EAGAIN as it's not a hard error.

Please let me know if you agree with the changes, I'll fix it in the
commit.

Re: [PATCH 1/2] btrfs: reject device with CHANGING_FSID_V2

[Anand Jain]

>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index dc577b3c53f6..95746ddf7dc3 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -3173,7 +3173,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>>   	u32 nodesize;
>>   	u32 stripesize;
>>   	u64 generation;
>> -	u64 features;
>>   	u16 csum_type;
>>   	struct btrfs_super_block *disk_super;
>>   	struct btrfs_fs_info *fs_info = btrfs_sb(sb);
>> @@ -3255,15 +3254,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>>   
>>   	disk_super = fs_info->super_copy;
>>   
>> -
>> -	features = btrfs_super_flags(disk_super);
>> -	if (features & BTRFS_SUPER_FLAG_CHANGING_FSID_V2) {
>> -		features &= ~BTRFS_SUPER_FLAG_CHANGING_FSID_V2;
>> -		btrfs_set_super_flags(disk_super, features);
>> -		btrfs_info(fs_info,
>> -			"found metadata UUID change in progress flag, clearing");
>> -	}
> 
> This is removed from the mount path but it's still rejected because at
> some point the device scanning will be called and will return -EINVAL.
> 

Correct. This mount thread calls btrfs_scan_one_device() with the 
mounting device as the path and verifies its superblock until it reaches 
device_list_add(), where we return -EINVAL.


>> -
>>   	memcpy(fs_info->super_for_commit, fs_info->super_copy,
>>   	       sizeof(*fs_info->super_for_commit));
>>   
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index bc8d46cbc7c5..c845c60ec207 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -791,6 +791,13 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>>   	bool fsid_change_in_progress = (btrfs_super_flags(disk_super) &
>>   					BTRFS_SUPER_FLAG_CHANGING_FSID_V2);
>>   
>> +	if (fsid_change_in_progress) {
>> +		btrfs_err(NULL,
>> +"device %s has incomplete FSID changes please use btrfstune to complete",
> 
> This could say it's specifically metadata_uuid.
> 
>> +			  path);
>> +		return ERR_PTR(-EINVAL);

Here.

> 
> We could probably return -EAGAIN as it's not a hard error.
> 

-EAGAIN is fine.

> Please let me know if you agree with the changes, I'll fix it in the
> commit.

Thanks!
Anand

Re: [PATCH 0/2 v2] btrfs: reject device with CHANGING_FSID_V2 flag

[David Sterba]

On Thu, Sep 21, 2023 at 05:51:12AM +0800, Anand Jain wrote:
> v2: Splits the patch into two parts: one for the new code to reject
> devices with CHANGING_FSID_V2 and the second to remove the unused code.
> 
> The btrfs-progs code to reunite devices with the CHANGING_FSID_V2 flag,
> which is ported from the kernel, can be found in the following btrfs-progs
> patchset:
> 
>  [PATCH 0/4 v4] btrfs-progs: recover from failed metadata_uuid port kernel
>     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
> 
> Furthermore, when the kernel stops supporting the CHANGING_FSID_V2 flag,
> we will need to update the btrfs-progs test case accordingly:
> 
>     btrfs-progs: misc-tests/034-metadata-uuid remove kernel support
> 
> v1:
> https://lore.kernel.org/linux-btrfs/02d59bdd7a8b778deb17e300354558498db59376.1692178060.git.anand.jain@oracle.com
> 
> 
> Anand Jain (2):
>   btrfs: reject device with CHANGING_FSID_V2
>   btrfs: remove unused code related to the CHANGING_FSID_V2 flag

Added to misc-next, thanks. I've updated the 2nd patch with some
historical background. The temp-fsid patch now does not apply cleanly,
I'll do a refresh on top of this series. Once it's in for-next, please
have a look. Thanks.

Re: [PATCH 0/2 v2] btrfs: reject device with CHANGING_FSID_V2 flag

[Anand Jain]

On 26/09/2023 00:58, David Sterba wrote:
> On Thu, Sep 21, 2023 at 05:51:12AM +0800, Anand Jain wrote:
>> v2: Splits the patch into two parts: one for the new code to reject
>> devices with CHANGING_FSID_V2 and the second to remove the unused code.
>>
>> The btrfs-progs code to reunite devices with the CHANGING_FSID_V2 flag,
>> which is ported from the kernel, can be found in the following btrfs-progs
>> patchset:
>>
>>   [PATCH 0/4 v4] btrfs-progs: recover from failed metadata_uuid port kernel
>>      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
>>
>> Furthermore, when the kernel stops supporting the CHANGING_FSID_V2 flag,
>> we will need to update the btrfs-progs test case accordingly:
>>
>>      btrfs-progs: misc-tests/034-metadata-uuid remove kernel support
>>
>> v1:
>> https://lore.kernel.org/linux-btrfs/02d59bdd7a8b778deb17e300354558498db59376.1692178060.git.anand.jain@oracle.com
>>
>>
>> Anand Jain (2):
>>    btrfs: reject device with CHANGING_FSID_V2
>>    btrfs: remove unused code related to the CHANGING_FSID_V2 flag
> 


> Added to misc-next, thanks.

> I've updated the 2nd patch with some
> historical background.

Now much more complete! Thanks.

> The temp-fsid patch now does not apply cleanly,
> I'll do a refresh on top of this series. Once it's in for-next, please
> have a look. Thanks.

Sure. But, temp-fsid v4 still has a subvol mount bug, as reported
before. I have a fix that is in-memory only. I am trying to get a
reasonable list of fstests passed before sending it out. Guilherme
can add the super-block flag on top of this, although it is not
mandatory from the btrfs internals pov. I will send out the
in-memory based temp-fsid soon I get the fstests (with some fix)
working today.

Thanks, Anand

Re: [PATCH 0/2 v2] btrfs: reject device with CHANGING_FSID_V2 flag

[David Sterba]

On Tue, Sep 26, 2023 at 07:56:57AM +0800, Anand Jain wrote:
> On 26/09/2023 00:58, David Sterba wrote:
> > On Thu, Sep 21, 2023 at 05:51:12AM +0800, Anand Jain wrote:
> >> v2: Splits the patch into two parts: one for the new code to reject
> >> devices with CHANGING_FSID_V2 and the second to remove the unused code.
> >>
> >> The btrfs-progs code to reunite devices with the CHANGING_FSID_V2 flag,
> >> which is ported from the kernel, can be found in the following btrfs-progs
> >> patchset:
> >>
> >>   [PATCH 0/4 v4] btrfs-progs: recover from failed metadata_uuid port kernel
> >>      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
> >>
> >> Furthermore, when the kernel stops supporting the CHANGING_FSID_V2 flag,
> >> we will need to update the btrfs-progs test case accordingly:
> >>
> >>      btrfs-progs: misc-tests/034-metadata-uuid remove kernel support
> >>
> >> v1:
> >> https://lore.kernel.org/linux-btrfs/02d59bdd7a8b778deb17e300354558498db59376.1692178060.git.anand.jain@oracle.com
> >>
> >>
> >> Anand Jain (2):
> >>    btrfs: reject device with CHANGING_FSID_V2
> >>    btrfs: remove unused code related to the CHANGING_FSID_V2 flag
> > 
> 
> 
> > Added to misc-next, thanks.
> 
> > I've updated the 2nd patch with some
> > historical background.
> 
> Now much more complete! Thanks.
> 
> > The temp-fsid patch now does not apply cleanly,
> > I'll do a refresh on top of this series. Once it's in for-next, please
> > have a look. Thanks.
> 
> Sure. But, temp-fsid v4 still has a subvol mount bug, as reported
> before. I have a fix that is in-memory only. I am trying to get a
> reasonable list of fstests passed before sending it out. Guilherme
> can add the super-block flag on top of this, although it is not
> mandatory from the btrfs internals pov. I will send out the
> in-memory based temp-fsid soon I get the fstests (with some fix)
> working today.

Feel free the send the patches even if there are still failing tests,
I'd like to get an idea of how are you fixing the remaining problem(s).
Thanks.

Re: [PATCH 0/2 v2] btrfs: reject device with CHANGING_FSID_V2 flag

[Anand Jain]

On 28/09/2023 01:43, David Sterba wrote:
> On Tue, Sep 26, 2023 at 07:56:57AM +0800, Anand Jain wrote:
>> On 26/09/2023 00:58, David Sterba wrote:
>>> On Thu, Sep 21, 2023 at 05:51:12AM +0800, Anand Jain wrote:
>>>> v2: Splits the patch into two parts: one for the new code to reject
>>>> devices with CHANGING_FSID_V2 and the second to remove the unused code.
>>>>
>>>> The btrfs-progs code to reunite devices with the CHANGING_FSID_V2 flag,
>>>> which is ported from the kernel, can be found in the following btrfs-progs
>>>> patchset:
>>>>
>>>>    [PATCH 0/4 v4] btrfs-progs: recover from failed metadata_uuid port kernel
>>>>       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
>>>>
>>>> Furthermore, when the kernel stops supporting the CHANGING_FSID_V2 flag,
>>>> we will need to update the btrfs-progs test case accordingly:
>>>>
>>>>       btrfs-progs: misc-tests/034-metadata-uuid remove kernel support
>>>>
>>>> v1:
>>>> https://lore.kernel.org/linux-btrfs/02d59bdd7a8b778deb17e300354558498db59376.1692178060.git.anand.jain@oracle.com
>>>>
>>>>
>>>> Anand Jain (2):
>>>>     btrfs: reject device with CHANGING_FSID_V2
>>>>     btrfs: remove unused code related to the CHANGING_FSID_V2 flag
>>>
>>
>>
>>> Added to misc-next, thanks.
>>
>>> I've updated the 2nd patch with some
>>> historical background.
>>
>> Now much more complete! Thanks.
>>
>>> The temp-fsid patch now does not apply cleanly,
>>> I'll do a refresh on top of this series. Once it's in for-next, please
>>> have a look. Thanks.
>>
>> Sure. But, temp-fsid v4 still has a subvol mount bug, as reported
>> before. I have a fix that is in-memory only. I am trying to get a
>> reasonable list of fstests passed before sending it out. Guilherme
>> can add the super-block flag on top of this, although it is not
>> mandatory from the btrfs internals pov. I will send out the
>> in-memory based temp-fsid soon I get the fstests (with some fix)
>> working today.
> 
> Feel free the send the patches even if there are still failing tests,
> I'd like to get an idea of how are you fixing the remaining problem(s).
> Thanks.

Yep, it is almost ready. A few minor cleanups and code optimizations
are still due. However, I have sent them for the review comments.
Below are the patches in the mailing list now. Also, after sending
them, I realized it would have been easier for the searches if the
subject included 'temp-fsid' instead of 'cloned-device'

  [PATCH 0/2] btrfs: support cloned-device mount capability

  [PATCH 0/2] btrfs-progs: mkfs: testing cloned-device


Thanks, Anand