[PATCH 0/4] btrfs: sysfs and unsupported temp-fsid features for clones

[PATCH 0/4] btrfs: sysfs and unsupported temp-fsid features for clones

[Anand Jain]

Seed and device-add are the two features that must be unsupported
if a cloned device is using temp-fsid to mount, as they conflict
with multi-device functionality.

Additionally, add sysfs files for the temp-fsid feature.

Anand Jain (4):
  btrfs: comment for temp-fsid, fsid, and metadata_uuid
  btrfs: disable seed feature for temp-fsid
  btrfs: disable the device add feature for temp-fsid
  btrfs: show temp_fsid feature in sysfs

 fs/btrfs/ioctl.c   |  6 ++++++
 fs/btrfs/sysfs.c   | 20 ++++++++++++++++++++
 fs/btrfs/volumes.c |  8 ++++++++
 fs/btrfs/volumes.h |  4 ++++
 4 files changed, 38 insertions(+)

-- 
2.38.1

[PATCH 1/4] btrfs: comment for temp-fsid, fsid, and metadata_uuid

[Anand Jain]

Add a comment to explain the relationship between temp-fsid, fsid, and
metadata_uuid.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index fef46c0dbf19..5dd4ad775e5d 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -299,6 +299,10 @@ struct btrfs_fs_devices {
 	 *   - When the BTRFS_FEATURE_INCOMPAT_METADATA_UUID flag is set:
 	 *       fs_devices->fsid == sb->fsid
 	 *       fs_devices->metadata_uuid == sb->metadata_uuid
+	 *
+	 *   - When in-memory fs_devices->temp_fsid is true
+	 *	 fs_devices->fsid = random
+	 *	 fs_devices->metadata_uuid == sb->fsid
 	 */
 	u8 metadata_uuid[BTRFS_FSID_SIZE];
 
-- 
2.38.1

[PATCH 2/4] btrfs: disable seed feature for temp-fsid

[Anand Jain]

A seed device is an integral component of the sprout device, which
functions as a multi-device filesystem. Therefore, temp-fsid feature
is not supported.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 81b735f4efc1..1fdfa9153e30 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -571,6 +571,14 @@ static struct btrfs_fs_devices *find_fsid_by_device(
 	if (btrfs_super_num_devices(disk_super) != 1)
 		return fsid_fs_devices;
 
+	/*
+	 * A seed device is an integral component of the sprout device, which
+	 * functions as a multi-device filesystem. So, temp-fsid feature is
+	 * not supported.
+	 */
+	if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING)
+		return fsid_fs_devices;
+
 	/* Try to find a fs_devices by matching devt. */
 	list_for_each_entry(devt_fs_devices, &fs_uuids, fs_list) {
 		struct btrfs_device *device;
-- 
2.38.1

[PATCH 3/4] btrfs: disable the device add feature for temp-fsid

[Anand Jain]

The device addition operation will transform the cloned temp-fsid mounted
device into a multi-device filesystem. Therefore, it is marked as
unsupported.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/ioctl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 848b7e6f6421..7d8d217cf5fd 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2635,6 +2635,12 @@ static long btrfs_ioctl_add_dev(struct btrfs_fs_info *fs_info, void __user *arg)
 		return -EINVAL;
 	}
 
+	if (fs_info->fs_devices->temp_fsid) {
+		btrfs_err(fs_info,
+			  "device add not supported on cloned temp-fsid mount");
+		return -EINVAL;
+	}
+
 	if (!btrfs_exclop_start(fs_info, BTRFS_EXCLOP_DEV_ADD)) {
 		if (!btrfs_exclop_start_try_lock(fs_info, BTRFS_EXCLOP_DEV_ADD))
 			return BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS;
-- 
2.38.1

[PATCH 4/4] btrfs: show temp_fsid feature in sysfs

[Anand Jain]

This adds sysfs objects to indicate temp_fsid feature support and
its status.

  /sys/fs/btrfs/features/temp_fsid
  /sys/fs/btrfs/<UUID>/temp_fsid

  For example:

     Consider two cloned and mounted devices.

	$ blkid /dev/sdc[1-2]
	/dev/sdc1: UUID="509ad44b-ad2a-4a8a-bc8d-fe69db7220d5" ..
	/dev/sdc2: UUID="509ad44b-ad2a-4a8a-bc8d-fe69db7220d5" ..

     One gets actual fsid, and the other gets the temp_fsid when
     mounted.

	$ btrfs filesystem show -m
	Label: none  uuid: 509ad44b-ad2a-4a8a-bc8d-fe69db7220d5
		Total devices 1 FS bytes used 54.14MiB
		devid    1 size 300.00MiB used 144.00MiB path /dev/sdc1

	Label: none  uuid: 33bad74e-c91b-43a5-aef8-b3cab97ae63a
		Total devices 1 FS bytes used 54.14MiB
		devid    1 size 300.00MiB used 144.00MiB path /dev/sdc2

     Their sysfs as below.

	$ cat /sys/fs/btrfs/features/temp_fsid
	0

	$ cat /sys/fs/btrfs/509ad44b-ad2a-4a8a-bc8d-fe69db7220d5/temp_fsid
	0

	$ cat /sys/fs/btrfs/33bad74e-c91b-43a5-aef8-b3cab97ae63a/temp_fsid
	1

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/sysfs.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index e07be193323a..7f9a4790e013 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -425,6 +425,15 @@ static ssize_t acl_show(struct kobject *kobj, struct kobj_attribute *a, char *bu
 }
 BTRFS_ATTR(static_feature, acl, acl_show);
 
+static ssize_t temp_fsid_supported_show(struct kobject *kobj,
+					struct kobj_attribute *a, char *buf)
+{
+	int ret = 0;
+
+	return sysfs_emit(buf, "%d\n", ret);
+}
+BTRFS_ATTR(static_feature, temp_fsid, temp_fsid_supported_show);
+
 /*
  * Features which only depend on kernel version.
  *
@@ -438,6 +447,7 @@ static struct attribute *btrfs_supported_static_feature_attrs[] = {
 	BTRFS_ATTR_PTR(static_feature, send_stream_version),
 	BTRFS_ATTR_PTR(static_feature, supported_rescue_options),
 	BTRFS_ATTR_PTR(static_feature, supported_sectorsizes),
+	BTRFS_ATTR_PTR(static_feature, temp_fsid),
 	NULL
 };
 
@@ -1205,6 +1215,15 @@ static ssize_t btrfs_generation_show(struct kobject *kobj,
 }
 BTRFS_ATTR(, generation, btrfs_generation_show);
 
+static ssize_t btrfs_temp_fsid_show(struct kobject *kobj,
+				    struct kobj_attribute *a, char *buf)
+{
+	struct btrfs_fs_info *fs_info = to_fs_info(kobj);
+
+	return sysfs_emit(buf, "%d\n", fs_info->fs_devices->temp_fsid);
+}
+BTRFS_ATTR(, temp_fsid, btrfs_temp_fsid_show);
+
 static const char * const btrfs_read_policy_name[] = { "pid" };
 
 static ssize_t btrfs_read_policy_show(struct kobject *kobj,
@@ -1307,6 +1326,7 @@ static const struct attribute *btrfs_attrs[] = {
 	BTRFS_ATTR_PTR(, read_policy),
 	BTRFS_ATTR_PTR(, bg_reclaim_threshold),
 	BTRFS_ATTR_PTR(, commit_stats),
+	BTRFS_ATTR_PTR(, temp_fsid),
 	NULL,
 };
 
-- 
2.38.1

Re: [PATCH 4/4] btrfs: show temp_fsid feature in sysfs

[David Sterba]

On Wed, Oct 04, 2023 at 11:00:27PM +0800, Anand Jain wrote:
> This adds sysfs objects to indicate temp_fsid feature support and
> its status.
> 
>   /sys/fs/btrfs/features/temp_fsid
>   /sys/fs/btrfs/<UUID>/temp_fsid
> 
>   For example:
> 
>      Consider two cloned and mounted devices.
> 
> 	$ blkid /dev/sdc[1-2]
> 	/dev/sdc1: UUID="509ad44b-ad2a-4a8a-bc8d-fe69db7220d5" ..
> 	/dev/sdc2: UUID="509ad44b-ad2a-4a8a-bc8d-fe69db7220d5" ..
> 
>      One gets actual fsid, and the other gets the temp_fsid when
>      mounted.
> 
> 	$ btrfs filesystem show -m
> 	Label: none  uuid: 509ad44b-ad2a-4a8a-bc8d-fe69db7220d5
> 		Total devices 1 FS bytes used 54.14MiB
> 		devid    1 size 300.00MiB used 144.00MiB path /dev/sdc1
> 
> 	Label: none  uuid: 33bad74e-c91b-43a5-aef8-b3cab97ae63a
> 		Total devices 1 FS bytes used 54.14MiB
> 		devid    1 size 300.00MiB used 144.00MiB path /dev/sdc2
> 
>      Their sysfs as below.
> 
> 	$ cat /sys/fs/btrfs/features/temp_fsid
> 	0
> 
> 	$ cat /sys/fs/btrfs/509ad44b-ad2a-4a8a-bc8d-fe69db7220d5/temp_fsid
> 	0
> 
> 	$ cat /sys/fs/btrfs/33bad74e-c91b-43a5-aef8-b3cab97ae63a/temp_fsid
> 	1

So the fsid used for the directory is always the new one, is there a way
to read which is the original filesystem's fsid? In this case it would
be the 509ad44b-... We could print it in that file instead of '1',
though it could be confusing that it's not the temp_fsid but the
original one, file name mismatches the contents on first look.

> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/sysfs.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index e07be193323a..7f9a4790e013 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -425,6 +425,15 @@ static ssize_t acl_show(struct kobject *kobj, struct kobj_attribute *a, char *bu
>  }
>  BTRFS_ATTR(static_feature, acl, acl_show);
>  
> +static ssize_t temp_fsid_supported_show(struct kobject *kobj,
> +					struct kobj_attribute *a, char *buf)
> +{
> +	int ret = 0;
> +
> +	return sysfs_emit(buf, "%d\n", ret);

This can be

	return sysfs_emit(buf, "0\n");

Re: [PATCH 0/4] btrfs: sysfs and unsupported temp-fsid features for clones

[David Sterba]

On Wed, Oct 04, 2023 at 11:00:23PM +0800, Anand Jain wrote:
> Seed and device-add are the two features that must be unsupported
> if a cloned device is using temp-fsid to mount, as they conflict
> with multi-device functionality.
> 
> Additionally, add sysfs files for the temp-fsid feature.
> 
> Anand Jain (4):
>   btrfs: comment for temp-fsid, fsid, and metadata_uuid
>   btrfs: disable seed feature for temp-fsid
>   btrfs: disable the device add feature for temp-fsid
>   btrfs: show temp_fsid feature in sysfs

Added to misc-next, thanks. The eventual change to the sysfs file can be
done later.

How are we going to proceed with the patch from Guilherme?

Re: [PATCH 4/4] btrfs: show temp_fsid feature in sysfs

[Anand Jain]

On 10/6/23 22:55, David Sterba wrote:
> On Wed, Oct 04, 2023 at 11:00:27PM +0800, Anand Jain wrote:
>> This adds sysfs objects to indicate temp_fsid feature support and
>> its status.
>>
>>    /sys/fs/btrfs/features/temp_fsid
>>    /sys/fs/btrfs/<UUID>/temp_fsid
>>
>>    For example:
>>
>>       Consider two cloned and mounted devices.
>>
>> 	$ blkid /dev/sdc[1-2]
>> 	/dev/sdc1: UUID="509ad44b-ad2a-4a8a-bc8d-fe69db7220d5" ..
>> 	/dev/sdc2: UUID="509ad44b-ad2a-4a8a-bc8d-fe69db7220d5" ..
>>
>>       One gets actual fsid, and the other gets the temp_fsid when
>>       mounted.
>>
>> 	$ btrfs filesystem show -m
>> 	Label: none  uuid: 509ad44b-ad2a-4a8a-bc8d-fe69db7220d5
>> 		Total devices 1 FS bytes used 54.14MiB
>> 		devid    1 size 300.00MiB used 144.00MiB path /dev/sdc1
>>
>> 	Label: none  uuid: 33bad74e-c91b-43a5-aef8-b3cab97ae63a
>> 		Total devices 1 FS bytes used 54.14MiB
>> 		devid    1 size 300.00MiB used 144.00MiB path /dev/sdc2
>>
>>       Their sysfs as below.
>>
>> 	$ cat /sys/fs/btrfs/features/temp_fsid
>> 	0
>>
>> 	$ cat /sys/fs/btrfs/509ad44b-ad2a-4a8a-bc8d-fe69db7220d5/temp_fsid
>> 	0
>>
>> 	$ cat /sys/fs/btrfs/33bad74e-c91b-43a5-aef8-b3cab97ae63a/temp_fsid
>> 	1
> 
> So the fsid used for the directory is always the new one, is there a way
> to read which is the original filesystem's fsid? In this case it would
> be the 509ad44b-... We could print it in that file instead of '1',
> though it could be confusing that it's not the temp_fsid but the
> original one, file name mismatches the contents on first look.

Instead, can we emit 'fsid' in another kobject altogether?
Furthermore, we also have a 'metadata_uuid' kobject. Here
is how they relate.


1. normally:

  $ cat /sys/fs/btrfs/<meta-fsid>/fsid
  <meta-fsid>
  $ cat /sys/fs/btrfs/<meta-fsid>/metadata_uuid
  <meta-fsid>


2. metadata-uuid flag is set:

  $ cat /sys/fs/btrfs/<sb-fsid>/fsid
  <sb-fsid>
  $ cat /sys/fs/btrfs/<sb-fsid>/metadata_uuid
  <meta-fsid>


3. normal + temp-fsid:

  $ cat /sys/fs/btrfs/<temp-fsid>/fsid
  <meta-fsid>
  $ cat /sys/fs/btrfs/<temp-fsid>/metadata_uuid
  <meta-fsid>


4. metadata-uuid flag is set + temp-fsid:

  $ cat /sys/fs/btrfs/<temp-fsid>/fsid
  <sb-fsid>
  $ cat /sys/fs/btrfs/<temp-fsid>/metadata_uuid
  <meta-fsid>

> 
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   fs/btrfs/sysfs.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
>> index e07be193323a..7f9a4790e013 100644
>> --- a/fs/btrfs/sysfs.c
>> +++ b/fs/btrfs/sysfs.c
>> @@ -425,6 +425,15 @@ static ssize_t acl_show(struct kobject *kobj, struct kobj_attribute *a, char *bu
>>   }
>>   BTRFS_ATTR(static_feature, acl, acl_show);
>>   
>> +static ssize_t temp_fsid_supported_show(struct kobject *kobj,
>> +					struct kobj_attribute *a, char *buf)
>> +{
>> +	int ret = 0;
>> +
>> +	return sysfs_emit(buf, "%d\n", ret);
> 
> This can be
> 
> 	return sysfs_emit(buf, "0\n");

Changes in the misc-next looks better.

Thanks, Anand

Re: [PATCH 4/4] btrfs: show temp_fsid feature in sysfs

[Anand Jain]

On 10/7/23 17:12, Anand Jain wrote:
> 
> 
> On 10/6/23 22:55, David Sterba wrote:
>> On Wed, Oct 04, 2023 at 11:00:27PM +0800, Anand Jain wrote:
>>> This adds sysfs objects to indicate temp_fsid feature support and
>>> its status.
>>>
>>>    /sys/fs/btrfs/features/temp_fsid
>>>    /sys/fs/btrfs/<UUID>/temp_fsid
>>>
>>>    For example:
>>>
>>>       Consider two cloned and mounted devices.
>>>
>>>     $ blkid /dev/sdc[1-2]
>>>     /dev/sdc1: UUID="509ad44b-ad2a-4a8a-bc8d-fe69db7220d5" ..
>>>     /dev/sdc2: UUID="509ad44b-ad2a-4a8a-bc8d-fe69db7220d5" ..
>>>
>>>       One gets actual fsid, and the other gets the temp_fsid when
>>>       mounted.
>>>
>>>     $ btrfs filesystem show -m
>>>     Label: none  uuid: 509ad44b-ad2a-4a8a-bc8d-fe69db7220d5
>>>         Total devices 1 FS bytes used 54.14MiB
>>>         devid    1 size 300.00MiB used 144.00MiB path /dev/sdc1
>>>
>>>     Label: none  uuid: 33bad74e-c91b-43a5-aef8-b3cab97ae63a
>>>         Total devices 1 FS bytes used 54.14MiB
>>>         devid    1 size 300.00MiB used 144.00MiB path /dev/sdc2
>>>
>>>       Their sysfs as below.
>>>
>>>     $ cat /sys/fs/btrfs/features/temp_fsid
>>>     0
>>>
>>>     $ cat /sys/fs/btrfs/509ad44b-ad2a-4a8a-bc8d-fe69db7220d5/temp_fsid
>>>     0
>>>
>>>     $ cat /sys/fs/btrfs/33bad74e-c91b-43a5-aef8-b3cab97ae63a/temp_fsid
>>>     1
>>
>> So the fsid used for the directory is always the new one, is there a way
>> to read which is the original filesystem's fsid? In this case it would
>> be the 509ad44b-... We could print it in that file instead of '1',
>> though it could be confusing that it's not the temp_fsid but the
>> original one, file name mismatches the contents on first look.
> 
> Instead, can we emit 'fsid' in another kobject altogether?
> Furthermore, we also have a 'metadata_uuid' kobject. Here
> is how they relate.
> 
> 
> 1. normally:
> 
>   $ cat /sys/fs/btrfs/<meta-fsid>/fsid
>   <meta-fsid>
>   $ cat /sys/fs/btrfs/<meta-fsid>/metadata_uuid
>   <meta-fsid>
> 
> 
> 2. metadata-uuid flag is set:
> 
>   $ cat /sys/fs/btrfs/<sb-fsid>/fsid
>   <sb-fsid>
>   $ cat /sys/fs/btrfs/<sb-fsid>/metadata_uuid
>   <meta-fsid>
> 
> 
> 3. normal + temp-fsid:
> 
>   $ cat /sys/fs/btrfs/<temp-fsid>/fsid
>   <meta-fsid>
>   $ cat /sys/fs/btrfs/<temp-fsid>/metadata_uuid
>   <meta-fsid>
> 
> 
> 4. metadata-uuid flag is set + temp-fsid:
> 
>   $ cat /sys/fs/btrfs/<temp-fsid>/fsid
>   <sb-fsid>
>   $ cat /sys/fs/btrfs/<temp-fsid>/metadata_uuid
>   <meta-fsid>



Let's consider this thought:

The proposal above implies that /sys/fs/btrfs/<temp-fsid>/temp_fsid
will continue to function as described in this patch. Otherwise,
determining whether temp_fsid is enabled becomes challenging.

As part of the temp-fsid feature, we plan to introduce two additional
kobjects. However, if this results in an excessive number of kobjects,
then...

We have the option to rename /sys/fs/btrfs/<temp-fsid>/temp_fsid
to /sys/fs/btrfs/<temp-fsid>/temp-fsid_enabled_super_fsid to display
the actual sb::fsid.

Thanks, Anand

Re: [PATCH 0/4] btrfs: sysfs and unsupported temp-fsid features for clones

[Anand Jain]

On 10/6/23 23:07, David Sterba wrote:
> On Wed, Oct 04, 2023 at 11:00:23PM +0800, Anand Jain wrote:
>> Seed and device-add are the two features that must be unsupported
>> if a cloned device is using temp-fsid to mount, as they conflict
>> with multi-device functionality.
>>
>> Additionally, add sysfs files for the temp-fsid feature.
>>
>> Anand Jain (4):
>>    btrfs: comment for temp-fsid, fsid, and metadata_uuid
>>    btrfs: disable seed feature for temp-fsid
>>    btrfs: disable the device add feature for temp-fsid
>>    btrfs: show temp_fsid feature in sysfs
> 
> Added to misc-next, thanks. The eventual change to the sysfs file can be
> done later.
> 
> How are we going to proceed with the patch from Guilherme?

The last step is to ensure that the temp-fsid feature is restricted
with the temp-fsid superblock flag. Guilherme's patches
(kernel, mkfs, and tune) already handle it but need a rebase.
Can Guilherme send an RFC patch for feedback from others and
copy suggested-by. Because, I haven't found a compelling reason
for the restriction, except to improve the user experience.

His fstests patch will be accepted. And progressively we should
add more coverage when the fstests configuration does not include
the temp-fsid environment.

Lastly, in fact even for this patchset:

   Co-developed-by: Guilherme G. Piccoli <gpiccoli@igalia.com>

Thanks, Anand

Re: [PATCH 0/4] btrfs: sysfs and unsupported temp-fsid features for clones

[Guilherme G. Piccoli]

On 07/10/2023 12:30, Anand Jain wrote:
> [...]
>> How are we going to proceed with the patch from Guilherme?
> 
> The last step is to ensure that the temp-fsid feature is restricted
> with the temp-fsid superblock flag. Guilherme's patches
> (kernel, mkfs, and tune) already handle it but need a rebase.
> Can Guilherme send an RFC patch for feedback from others and
> copy suggested-by. Because, I haven't found a compelling reason
> for the restriction, except to improve the user experience.
> 
> His fstests patch will be accepted. And progressively we should
> add more coverage when the fstests configuration does not include
> the temp-fsid environment.
> 
> Lastly, in fact even for this patchset:
> 
>    Co-developed-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
> 
> Thanks, Anand
> 

Thanks Anand and David. The first thing I need to do, is to build
misc-next and test it if if works with my setup, to double-check Anand's
approach fits the use-case (it seems to, so far). Then, I guess we'll
need to see if there's a missing piece on that - in the other thread
Anand mentioned maybe the superblock flag would be useful after all, so
if that's necessary, I can send it of course.

Finally, this week I'm away from my regular system and cannot provide
the test results, next week I can do that for sure.

Thank you both for looping me in and for the details and clarifications!
Cheers,


Guilherme

Re: [PATCH 0/4] btrfs: sysfs and unsupported temp-fsid features for clones

[Anand Jain]

>> Can Guilherme send an RFC patch for feedback from others and
>> copy suggested-by. Because, I haven't found a compelling reason
>> for the restriction, except to improve the user experience.

My comments about the superblock flag are above.

User experiences are subjective, so we need others to comment;
an RFC will help.


> Thanks Anand and David. The first thing I need to do, is to build
> misc-next and test it if if works with my setup, to double-check Anand's
> approach fits the use-case (it seems to, so far). Then, I guess we'll
> need to see if there's a missing piece on that - in the other thread
> Anand mentioned maybe the superblock flag would be useful after all, so
> if that's necessary, I can send it of course.
>  > Finally, this week I'm away from my regular system and cannot provide
> the test results,


> next week I can do that for sure.

That's great; much appreciated.

Thanks, Anand

Re: [PATCH 0/4] btrfs: sysfs and unsupported temp-fsid features for clones

[David Sterba]

On Mon, Oct 09, 2023 at 01:37:22PM +0530, Anand Jain wrote:
> >> Can Guilherme send an RFC patch for feedback from others and
> >> copy suggested-by. Because, I haven't found a compelling reason
> >> for the restriction, except to improve the user experience.
> 
> My comments about the superblock flag are above.
> 
> User experiences are subjective, so we need others to comment;
> an RFC will help.

A few things changed, the incompat bit was supposed to prevent
accidentally duplicated fsids but with your recent changes this is safe.
This would need to let Guilherme check if the A/B use case still works
but this seems to be so as I'm reading the changelog.

In a controlled environment the incompat bit will not bring much value
other than yet another sanity check preventing some user error, but
related only to the multiple devices.

Re: [PATCH 0/4] btrfs: sysfs and unsupported temp-fsid features for clones

[Anand Jain]

On 10/10/23 05:29, David Sterba wrote:
> On Mon, Oct 09, 2023 at 01:37:22PM +0530, Anand Jain wrote:
>>>> Can Guilherme send an RFC patch for feedback from others and
>>>> copy suggested-by. Because, I haven't found a compelling reason
>>>> for the restriction, except to improve the user experience.
>>
>> My comments about the superblock flag are above.
>>
>> User experiences are subjective, so we need others to comment;
>> an RFC will help.
> 
> A few things changed, the incompat bit was supposed to prevent
> accidentally duplicated fsids but with your recent changes this is safe.
> This would need to let Guilherme check if the A/B use case still works
> but this seems to be so as I'm reading the changelog.
> 


> In a controlled environment the incompat bit will not bring much value
> other than yet another sanity check preventing some user error, > but
> related only to the multiple devices.


Agreed. Anyway, the kernel would continue to fail the mount of the
duplicate-fsid for a multi-device filesystem.

And

    $ mkfs.btrfs -U <duplicate-fsid> /dev/sda1 /dev/sda2 ..

for a multi-device filesystem will also fail.

Therefore, the only avenue for the user to make a mistake is
by using dd to copy for a multi-device setup.

Thanks, Anand

Re: [PATCH 0/4] btrfs: sysfs and unsupported temp-fsid features for clones

[Guilherme G. Piccoli]

On 10/10/2023 01:59, David Sterba wrote:
> On Mon, Oct 09, 2023 at 01:37:22PM +0530, Anand Jain wrote:
>>>> Can Guilherme send an RFC patch for feedback from others and
>>>> copy suggested-by. Because, I haven't found a compelling reason
>>>> for the restriction, except to improve the user experience.
>>
>> My comments about the superblock flag are above.
>>
>> User experiences are subjective, so we need others to comment;
>> an RFC will help.
> 
> A few things changed, the incompat bit was supposed to prevent
> accidentally duplicated fsids but with your recent changes this is safe.
> This would need to let Guilherme check if the A/B use case still works
> but this seems to be so as I'm reading the changelog.
> 
> In a controlled environment the incompat bit will not bring much value
> other than yet another sanity check preventing some user error, but
> related only to the multiple devices.

Hi David and Anand, I've manage to test misc-next of today, that
includes both this patchset as well as the "support cloned-device mount
capability​" one.

It seems to be working fine for our use case, though I'll test a bit
more on Deck. I was able to mount the same filesystem (spread in 2 nvme
devices) at the same time, in any order...the second one always get the
temp-fsid. Tested also re-mounting the devices on other locations, and
it seems all consistent, with no error observed.

I also question the value of the incompat flag, not seeing much use for
that..looping Qu Wenruo as they first suggested this flag-based
approach, in case there is some more feedback...

Anyway, thanks for your improved approach Anand and to David: is it
expected to land on 6.7?
Cheers,


Guilherme

Re: [PATCH 0/4] btrfs: sysfs and unsupported temp-fsid features for clones

[David Sterba]

On Wed, Oct 18, 2023 at 03:37:54PM +0200, Guilherme G. Piccoli wrote:
> On 10/10/2023 01:59, David Sterba wrote:
> > On Mon, Oct 09, 2023 at 01:37:22PM +0530, Anand Jain wrote:
> >>>> Can Guilherme send an RFC patch for feedback from others and
> >>>> copy suggested-by. Because, I haven't found a compelling reason
> >>>> for the restriction, except to improve the user experience.
> >>
> >> My comments about the superblock flag are above.
> >>
> >> User experiences are subjective, so we need others to comment;
> >> an RFC will help.
> > 
> > A few things changed, the incompat bit was supposed to prevent
> > accidentally duplicated fsids but with your recent changes this is safe.
> > This would need to let Guilherme check if the A/B use case still works
> > but this seems to be so as I'm reading the changelog.
> > 
> > In a controlled environment the incompat bit will not bring much value
> > other than yet another sanity check preventing some user error, but
> > related only to the multiple devices.
> 
> Hi David and Anand, I've manage to test misc-next of today, that
> includes both this patchset as well as the "support cloned-device mount
> capability​" one.
> 
> It seems to be working fine for our use case, though I'll test a bit
> more on Deck. I was able to mount the same filesystem (spread in 2 nvme
> devices) at the same time, in any order...the second one always get the
> temp-fsid. Tested also re-mounting the devices on other locations, and
> it seems all consistent, with no error observed.

Great, thanks.

> I also question the value of the incompat flag, not seeing much use for
> that..looping Qu Wenruo as they first suggested this flag-based
> approach, in case there is some more feedback...

Yeah at this point I don't see the need for the incompat bit, which is
the better outcome.

> Anyway, thanks for your improved approach Anand and to David: is it
> expected to land on 6.7?

Yes, what's in misc-next is queued for 6.7, also we have the whole
development cycle to fix remaining bugs.

Re: [PATCH 0/4] btrfs: sysfs and unsupported temp-fsid features for clones

[Guilherme G. Piccoli]

On 19/10/2023 01:04, David Sterba wrote:
> [...]
>> Anyway, thanks for your improved approach Anand and to David: is it
>> expected to land on 6.7?
> 
> Yes, what's in misc-next is queued for 6.7, also we have the whole
> development cycle to fix remaining bugs.

Thanks a bunch for confirming and all guidance through this feature process.