[PATCH 0/2] btrfs-progs: read device fsid from the sysfs

[PATCH 0/2] btrfs-progs: read device fsid from the sysfs

[Anand Jain]

The following test case fails as it trying to read the fsid from the sb
for a missing device.

   $ mkfs.btrfs -f -draid1 -mraid1 $DEV1 $DEV2 
   $ btrfstune -S 1 $DEV1 
   $ wipefs -a $DEV2 
   $ btrfs dev scan --forget 
   $ mount -o degraded $DEV1 /btrfs 
   $ btrfs device add $DEV3 /btrfs -f 

   $ btrfs fi us /btrfs 
     ERROR: unexpected number of devices: 1 >= 1 
     ERROR: if seed device is used, try running this command as root
 
The kernel patch [1] in the mailing list provided a sysfs interface
to read the fsid of the device, so use it instead.

 [1]  btrfs: sysfs add devinfo/fsid to retrieve fsid from the device

This patch also retains the old method that is to read the SB for
backward compatibility purposes.

Anand Jain (2):
  btrfs-progs: prepare helper device_is_seed
  btrfs-progs: read fsid from the sysfs in device_is_seed

 cmds/filesystem-usage.c | 47 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 42 insertions(+), 5 deletions(-)

-- 
2.31.1

[PATCH 1/2] btrfs-progs: prepare helper device_is_seed

[Anand Jain]

load_device_info() checks if the device is a seed device by reading
superblock::fsid and comparing it with the mount fsid, and it fails
to work if the device is missing (a RAID1 seed fs). Move this part
of the code into a new helper function device_is_seed() in
preparation to make device_is_seed() work with the new sysfs
devinfo/<devid>/fsid interface.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/filesystem-usage.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/cmds/filesystem-usage.c b/cmds/filesystem-usage.c
index 6c0f9aa977d6..0dfc798e8dcc 100644
--- a/cmds/filesystem-usage.c
+++ b/cmds/filesystem-usage.c
@@ -25,6 +25,7 @@
 #include <getopt.h>
 #include <fcntl.h>
 #include <linux/limits.h>
+#include <uuid/uuid.h>
 
 #include "common/utils.h"
 #include "kerncompat.h"
@@ -705,6 +706,21 @@ out:
 	return ret;
 }
 
+static int device_is_seed(const char *dev_path, u8 *mnt_fsid)
+{
+	uuid_t fsid;
+	int ret;
+
+	ret = dev_to_fsid(dev_path, fsid);
+	if (ret)
+		return ret;
+
+	if (memcmp(mnt_fsid, fsid, BTRFS_FSID_SIZE))
+		return 0;
+
+	return -1;
+}
+
 /*
  *  This function loads the device_info structure and put them in an array
  */
@@ -715,7 +731,6 @@ static int load_device_info(int fd, struct device_info **device_info_ptr,
 	struct btrfs_ioctl_fs_info_args fi_args;
 	struct btrfs_ioctl_dev_info_args dev_info;
 	struct device_info *info;
-	u8 fsid[BTRFS_UUID_SIZE];
 
 	*device_info_count = 0;
 	*device_info_ptr = NULL;
@@ -759,8 +774,8 @@ static int load_device_info(int fd, struct device_info **device_info_ptr,
 		 * Ignore any other error including -EACCES, which is seen when
 		 * a non-root process calls dev_to_fsid(path)->open(path).
 		 */
-		ret = dev_to_fsid((const char *)dev_info.path, fsid);
-		if (!ret && memcmp(fi_args.fsid, fsid, BTRFS_FSID_SIZE) != 0)
+		ret = device_is_seed((const char *)dev_info.path, fi_args.fsid);
+		if (!ret)
 			continue;
 
 		info[ndevs].devid = dev_info.devid;
-- 
2.31.1

[PATCH 2/2] btrfs-progs: read fsid from the sysfs in device_is_seed

[Anand Jain]

The kernel patch [1] added a sysfs interface to read the device fsid from
the kernel, which is a better way to know the fsid of the device (rather
than reading the superblock). It also works if in case the device is
missing. Furthermore, the sysfs interface is readable from the non-root
user.

So use this new sysfs interface here to read the fsid.

[1]
btrfs: sysfs add devinfo/fsid to retrieve fsid from the device

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/filesystem-usage.c | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/cmds/filesystem-usage.c b/cmds/filesystem-usage.c
index 0dfc798e8dcc..f658c27b9609 100644
--- a/cmds/filesystem-usage.c
+++ b/cmds/filesystem-usage.c
@@ -40,6 +40,7 @@
 #include "common/help.h"
 #include "common/device-utils.h"
 #include "common/open-utils.h"
+#include "common/path-utils.h"
 
 /*
  * Add the chunk info to the chunk_info list
@@ -706,14 +707,33 @@ out:
 	return ret;
 }
 
-static int device_is_seed(const char *dev_path, u8 *mnt_fsid)
+static int device_is_seed(int fd, const char *dev_path, u64 devid, u8 *mnt_fsid)
 {
+	char fsidparse[BTRFS_UUID_UNPARSED_SIZE];
+	char fsid_path[PATH_MAX];
+	char devid_str[20];
 	uuid_t fsid;
-	int ret;
+	int ret = -1;
+	int sysfs_fd;
+
+	snprintf(devid_str, 20, "%llu", devid);
+	/* devinfo/<devid>/fsid */
+	path_cat3_out(fsid_path, "devinfo", devid_str, "fsid");
+
+	/* /sys/fs/btrfs/<fsid>/devinfo/<devid>/fsid */
+	sysfs_fd = sysfs_open_fsid_file(fd, fsid_path);
+	if (sysfs_fd >= 0) {
+		sysfs_read_file(sysfs_fd, fsidparse, BTRFS_UUID_UNPARSED_SIZE);
+		fsidparse[BTRFS_UUID_UNPARSED_SIZE - 1] = 0;
+		ret = uuid_parse(fsidparse, fsid);
+		close(sysfs_fd);
+	}
 
-	ret = dev_to_fsid(dev_path, fsid);
-	if (ret)
-		return ret;
+	if (ret) {
+		ret = dev_to_fsid(dev_path, fsid);
+		if (ret)
+			return ret;
+	}
 
 	if (memcmp(mnt_fsid, fsid, BTRFS_FSID_SIZE))
 		return 0;
@@ -768,13 +788,15 @@ static int load_device_info(int fd, struct device_info **device_info_ptr,
 		}
 
 		/*
-		 * Skip seed device by checking device's fsid (requires root).
-		 * And we will skip only if dev_to_fsid is successful and dev
+		 * Skip seed device by checking device's fsid (requires root if
+		 * kernel is not patched to provide fsid from the sysfs).
+		 * And we will skip only if device_is_seed is successful and dev
 		 * is a seed device.
 		 * Ignore any other error including -EACCES, which is seen when
 		 * a non-root process calls dev_to_fsid(path)->open(path).
 		 */
-		ret = device_is_seed((const char *)dev_info.path, fi_args.fsid);
+		ret = device_is_seed(fd, (const char *)dev_info.path, i,
+				     fi_args.fsid);
 		if (!ret)
 			continue;
 
-- 
2.31.1

Re: [PATCH 2/2] btrfs-progs: read fsid from the sysfs in device_is_seed

[Josef Bacik]

On Tue, Oct 19, 2021 at 08:23:45AM +0800, Anand Jain wrote:
> The kernel patch [1] added a sysfs interface to read the device fsid from
> the kernel, which is a better way to know the fsid of the device (rather
> than reading the superblock). It also works if in case the device is
> missing. Furthermore, the sysfs interface is readable from the non-root
> user.
> 
> So use this new sysfs interface here to read the fsid.
> 
> [1]
> btrfs: sysfs add devinfo/fsid to retrieve fsid from the device
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  cmds/filesystem-usage.c | 38 ++++++++++++++++++++++++++++++--------
>  1 file changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/cmds/filesystem-usage.c b/cmds/filesystem-usage.c
> index 0dfc798e8dcc..f658c27b9609 100644
> --- a/cmds/filesystem-usage.c
> +++ b/cmds/filesystem-usage.c
> @@ -40,6 +40,7 @@
>  #include "common/help.h"
>  #include "common/device-utils.h"
>  #include "common/open-utils.h"
> +#include "common/path-utils.h"
>  
>  /*
>   * Add the chunk info to the chunk_info list
> @@ -706,14 +707,33 @@ out:
>  	return ret;
>  }
>  
> -static int device_is_seed(const char *dev_path, u8 *mnt_fsid)
> +static int device_is_seed(int fd, const char *dev_path, u64 devid, u8 *mnt_fsid)
>  {
> +	char fsidparse[BTRFS_UUID_UNPARSED_SIZE];
> +	char fsid_path[PATH_MAX];
> +	char devid_str[20];
>  	uuid_t fsid;
> -	int ret;
> +	int ret = -1;
> +	int sysfs_fd;
> +
> +	snprintf(devid_str, 20, "%llu", devid);
> +	/* devinfo/<devid>/fsid */
> +	path_cat3_out(fsid_path, "devinfo", devid_str, "fsid");
> +
> +	/* /sys/fs/btrfs/<fsid>/devinfo/<devid>/fsid */
> +	sysfs_fd = sysfs_open_fsid_file(fd, fsid_path);
> +	if (sysfs_fd >= 0) {
> +		sysfs_read_file(sysfs_fd, fsidparse, BTRFS_UUID_UNPARSED_SIZE);
> +		fsidparse[BTRFS_UUID_UNPARSED_SIZE - 1] = 0;
> +		ret = uuid_parse(fsidparse, fsid);
> +		close(sysfs_fd);
> +	}
>  
> -	ret = dev_to_fsid(dev_path, fsid);

Why not just have dev_to_fsid() use the sysfs thing so all callers can benefit
from it, and then have it fall back to the reading of the super block?  Thanks,

Josef

Re: [PATCH 2/2] btrfs-progs: read fsid from the sysfs in device_is_seed

[Anand Jain]

On 19/10/2021 22:03, Josef Bacik wrote:
> On Tue, Oct 19, 2021 at 08:23:45AM +0800, Anand Jain wrote:
>> The kernel patch [1] added a sysfs interface to read the device fsid from
>> the kernel, which is a better way to know the fsid of the device (rather
>> than reading the superblock). It also works if in case the device is
>> missing. Furthermore, the sysfs interface is readable from the non-root
>> user.
>>
>> So use this new sysfs interface here to read the fsid.
>>
>> [1]
>> btrfs: sysfs add devinfo/fsid to retrieve fsid from the device
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   cmds/filesystem-usage.c | 38 ++++++++++++++++++++++++++++++--------
>>   1 file changed, 30 insertions(+), 8 deletions(-)
>>
>> diff --git a/cmds/filesystem-usage.c b/cmds/filesystem-usage.c
>> index 0dfc798e8dcc..f658c27b9609 100644
>> --- a/cmds/filesystem-usage.c
>> +++ b/cmds/filesystem-usage.c
>> @@ -40,6 +40,7 @@
>>   #include "common/help.h"
>>   #include "common/device-utils.h"
>>   #include "common/open-utils.h"
>> +#include "common/path-utils.h"
>>   
>>   /*
>>    * Add the chunk info to the chunk_info list
>> @@ -706,14 +707,33 @@ out:
>>   	return ret;
>>   }
>>   
>> -static int device_is_seed(const char *dev_path, u8 *mnt_fsid)
>> +static int device_is_seed(int fd, const char *dev_path, u64 devid, u8 *mnt_fsid)
>>   {
>> +	char fsidparse[BTRFS_UUID_UNPARSED_SIZE];
>> +	char fsid_path[PATH_MAX];
>> +	char devid_str[20];
>>   	uuid_t fsid;
>> -	int ret;
>> +	int ret = -1;
>> +	int sysfs_fd;
>> +
>> +	snprintf(devid_str, 20, "%llu", devid);
>> +	/* devinfo/<devid>/fsid */
>> +	path_cat3_out(fsid_path, "devinfo", devid_str, "fsid");
>> +
>> +	/* /sys/fs/btrfs/<fsid>/devinfo/<devid>/fsid */
>> +	sysfs_fd = sysfs_open_fsid_file(fd, fsid_path);
>> +	if (sysfs_fd >= 0) {
>> +		sysfs_read_file(sysfs_fd, fsidparse, BTRFS_UUID_UNPARSED_SIZE);
>> +		fsidparse[BTRFS_UUID_UNPARSED_SIZE - 1] = 0;
>> +		ret = uuid_parse(fsidparse, fsid);
>> +		close(sysfs_fd);
>> +	}
>>   
>> -	ret = dev_to_fsid(dev_path, fsid);
> 
> Why not just have dev_to_fsid() use the sysfs thing so all callers can benefit
> from it, and then have it fall back to the reading of the super block?  Thanks,

If we are using sysfs to read fsid it means the device is mounted.

cmd_filesystem_show() uses dev_to_fsid() only if the device is 
unmounted. So we can't use fsid by sysfs here.

There is no other user of dev_to_fsid().

Thanks, Anand

> 
> Josef
> 

Re: [PATCH 2/2] btrfs-progs: read fsid from the sysfs in device_is_seed

[Josef Bacik]

On Wed, Oct 20, 2021 at 10:40:14AM +0800, Anand Jain wrote:
> 
> 
> On 19/10/2021 22:03, Josef Bacik wrote:
> > On Tue, Oct 19, 2021 at 08:23:45AM +0800, Anand Jain wrote:
> > > The kernel patch [1] added a sysfs interface to read the device fsid from
> > > the kernel, which is a better way to know the fsid of the device (rather
> > > than reading the superblock). It also works if in case the device is
> > > missing. Furthermore, the sysfs interface is readable from the non-root
> > > user.
> > > 
> > > So use this new sysfs interface here to read the fsid.
> > > 
> > > [1]
> > > btrfs: sysfs add devinfo/fsid to retrieve fsid from the device
> > > 
> > > Signed-off-by: Anand Jain <anand.jain@oracle.com>
> > > ---
> > >   cmds/filesystem-usage.c | 38 ++++++++++++++++++++++++++++++--------
> > >   1 file changed, 30 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/cmds/filesystem-usage.c b/cmds/filesystem-usage.c
> > > index 0dfc798e8dcc..f658c27b9609 100644
> > > --- a/cmds/filesystem-usage.c
> > > +++ b/cmds/filesystem-usage.c
> > > @@ -40,6 +40,7 @@
> > >   #include "common/help.h"
> > >   #include "common/device-utils.h"
> > >   #include "common/open-utils.h"
> > > +#include "common/path-utils.h"
> > >   /*
> > >    * Add the chunk info to the chunk_info list
> > > @@ -706,14 +707,33 @@ out:
> > >   	return ret;
> > >   }
> > > -static int device_is_seed(const char *dev_path, u8 *mnt_fsid)
> > > +static int device_is_seed(int fd, const char *dev_path, u64 devid, u8 *mnt_fsid)
> > >   {
> > > +	char fsidparse[BTRFS_UUID_UNPARSED_SIZE];
> > > +	char fsid_path[PATH_MAX];
> > > +	char devid_str[20];
> > >   	uuid_t fsid;
> > > -	int ret;
> > > +	int ret = -1;
> > > +	int sysfs_fd;
> > > +
> > > +	snprintf(devid_str, 20, "%llu", devid);
> > > +	/* devinfo/<devid>/fsid */
> > > +	path_cat3_out(fsid_path, "devinfo", devid_str, "fsid");
> > > +
> > > +	/* /sys/fs/btrfs/<fsid>/devinfo/<devid>/fsid */
> > > +	sysfs_fd = sysfs_open_fsid_file(fd, fsid_path);
> > > +	if (sysfs_fd >= 0) {
> > > +		sysfs_read_file(sysfs_fd, fsidparse, BTRFS_UUID_UNPARSED_SIZE);
> > > +		fsidparse[BTRFS_UUID_UNPARSED_SIZE - 1] = 0;
> > > +		ret = uuid_parse(fsidparse, fsid);
> > > +		close(sysfs_fd);
> > > +	}
> > > -	ret = dev_to_fsid(dev_path, fsid);
> > 
> > Why not just have dev_to_fsid() use the sysfs thing so all callers can benefit
> > from it, and then have it fall back to the reading of the super block?  Thanks,
> 
> If we are using sysfs to read fsid it means the device is mounted.
> 
> cmd_filesystem_show() uses dev_to_fsid() only if the device is unmounted. So
> we can't use fsid by sysfs here.
> 
> There is no other user of dev_to_fsid().
> 

Ah ok that's reasonable then, you can add

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

to the series then.  Thanks,

Josef

Re: [PATCH 0/2] btrfs-progs: read device fsid from the sysfs

[Josef Bacik]

On Tue, Oct 19, 2021 at 08:23:43AM +0800, Anand Jain wrote:
> The following test case fails as it trying to read the fsid from the sb
> for a missing device.
> 
>    $ mkfs.btrfs -f -draid1 -mraid1 $DEV1 $DEV2 
>    $ btrfstune -S 1 $DEV1 
>    $ wipefs -a $DEV2 
>    $ btrfs dev scan --forget 
>    $ mount -o degraded $DEV1 /btrfs 
>    $ btrfs device add $DEV3 /btrfs -f 
> 
>    $ btrfs fi us /btrfs 
>      ERROR: unexpected number of devices: 1 >= 1 
>      ERROR: if seed device is used, try running this command as root
>  

I'd like to see this as either a btrfs-progs test or an fstest.  I think I
prefer the fstest because you're adding a sysfs file, so maybe a couple of
tests, one to validate the sysfs file behaves properly, and then one to do this
test as well.  Thanks,

Josef

Re: [PATCH 0/2] btrfs-progs: read device fsid from the sysfs

[Anand Jain]

On 19/10/2021 22:04, Josef Bacik wrote:
> On Tue, Oct 19, 2021 at 08:23:43AM +0800, Anand Jain wrote:
>> The following test case fails as it trying to read the fsid from the sb
>> for a missing device.
>>
>>     $ mkfs.btrfs -f -draid1 -mraid1 $DEV1 $DEV2
>>     $ btrfstune -S 1 $DEV1
>>     $ wipefs -a $DEV2
>>     $ btrfs dev scan --forget
>>     $ mount -o degraded $DEV1 /btrfs
>>     $ btrfs device add $DEV3 /btrfs -f
>>
>>     $ btrfs fi us /btrfs
>>       ERROR: unexpected number of devices: 1 >= 1
>>       ERROR: if seed device is used, try running this command as root
>>   
> 
> I'd like to see this as either a btrfs-progs test or an fstest.  I think I
> prefer the fstest because you're adding a sysfs file, so maybe a couple of
> tests, one to validate the sysfs file behaves properly, and then one to do this
> test as well.  Thanks,
> 

  Right. I will do that.

Thanks, Anand

> Josef
> 

Re: [PATCH 0/2] btrfs-progs: read device fsid from the sysfs

[Anand Jain]

Gentle ping?

The related kernel patch is already in misc-next.
I don't find this btrfs-progs patch-set in the devel branch.

Thanks, Anand


On 19/10/2021 08:23, Anand Jain wrote:
> The following test case fails as it trying to read the fsid from the sb
> for a missing device.
> 
>     $ mkfs.btrfs -f -draid1 -mraid1 $DEV1 $DEV2
>     $ btrfstune -S 1 $DEV1
>     $ wipefs -a $DEV2
>     $ btrfs dev scan --forget
>     $ mount -o degraded $DEV1 /btrfs
>     $ btrfs device add $DEV3 /btrfs -f
> 
>     $ btrfs fi us /btrfs
>       ERROR: unexpected number of devices: 1 >= 1
>       ERROR: if seed device is used, try running this command as root
>   
> The kernel patch [1] in the mailing list provided a sysfs interface
> to read the fsid of the device, so use it instead.
> 
>   [1]  btrfs: sysfs add devinfo/fsid to retrieve fsid from the device
> 
> This patch also retains the old method that is to read the SB for
> backward compatibility purposes.
> 
> Anand Jain (2):
>    btrfs-progs: prepare helper device_is_seed
>    btrfs-progs: read fsid from the sysfs in device_is_seed
> 
>   cmds/filesystem-usage.c | 47 ++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 42 insertions(+), 5 deletions(-)
> 

Re: [PATCH 0/2] btrfs-progs: read device fsid from the sysfs

[Nikolay Borisov]

On 6.01.22 г. 2:04, Anand Jain wrote:
> 
> Gentle ping?
> 
> The related kernel patch is already in misc-next.
> I don't find this btrfs-progs patch-set in the devel branch.

And what about introducing a test case for this change?

> 
> Thanks, Anand
> 
> 
> On 19/10/2021 08:23, Anand Jain wrote:
>> The following test case fails as it trying to read the fsid from the sb
>> for a missing device.
>>
>>     $ mkfs.btrfs -f -draid1 -mraid1 $DEV1 $DEV2
>>     $ btrfstune -S 1 $DEV1
>>     $ wipefs -a $DEV2
>>     $ btrfs dev scan --forget
>>     $ mount -o degraded $DEV1 /btrfs
>>     $ btrfs device add $DEV3 /btrfs -f
>>
>>     $ btrfs fi us /btrfs
>>       ERROR: unexpected number of devices: 1 >= 1
>>       ERROR: if seed device is used, try running this command as root
>>   The kernel patch [1] in the mailing list provided a sysfs interface
>> to read the fsid of the device, so use it instead.
>>
>>   [1]  btrfs: sysfs add devinfo/fsid to retrieve fsid from the device
>>
>> This patch also retains the old method that is to read the SB for
>> backward compatibility purposes.
>>
>> Anand Jain (2):
>>    btrfs-progs: prepare helper device_is_seed
>>    btrfs-progs: read fsid from the sysfs in device_is_seed
>>
>>   cmds/filesystem-usage.c | 47 ++++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 42 insertions(+), 5 deletions(-)
>>
> 

Re: [PATCH 0/2] btrfs-progs: read device fsid from the sysfs

[Anand Jain]

On 11/01/2022 23:19, Nikolay Borisov wrote:
> 
> 
> On 6.01.22 г. 2:04, Anand Jain wrote:
>>
>> Gentle ping?

   Gentle ping again.

>>
>> The related kernel patch is already in misc-next.
>> I don't find this btrfs-progs patch-set in the devel branch.
> 
> And what about introducing a test case for this change?

   btrfs/248 did it.

Thanks, Anand


>>
>> Thanks, Anand
>>
>>
>> On 19/10/2021 08:23, Anand Jain wrote:
>>> The following test case fails as it trying to read the fsid from the sb
>>> for a missing device.
>>>
>>>      $ mkfs.btrfs -f -draid1 -mraid1 $DEV1 $DEV2
>>>      $ btrfstune -S 1 $DEV1
>>>      $ wipefs -a $DEV2
>>>      $ btrfs dev scan --forget
>>>      $ mount -o degraded $DEV1 /btrfs
>>>      $ btrfs device add $DEV3 /btrfs -f
>>>
>>>      $ btrfs fi us /btrfs
>>>        ERROR: unexpected number of devices: 1 >= 1
>>>        ERROR: if seed device is used, try running this command as root
>>>    The kernel patch [1] in the mailing list provided a sysfs interface
>>> to read the fsid of the device, so use it instead.
>>>
>>>    [1]  btrfs: sysfs add devinfo/fsid to retrieve fsid from the device
>>>
>>> This patch also retains the old method that is to read the SB for
>>> backward compatibility purposes.
>>>
>>> Anand Jain (2):
>>>     btrfs-progs: prepare helper device_is_seed
>>>     btrfs-progs: read fsid from the sysfs in device_is_seed
>>>
>>>    cmds/filesystem-usage.c | 47 ++++++++++++++++++++++++++++++++++++-----
>>>    1 file changed, 42 insertions(+), 5 deletions(-)
>>>
>>

Re: [PATCH 0/2] btrfs-progs: read device fsid from the sysfs

[Wang Yugui]

Hi,

> On 11/01/2022 23:19, Nikolay Borisov wrote:
> >
> >
> > On 6.01.22 г. 2:04, Anand Jain wrote:
> >>
> >> Gentle ping?
> 
>    Gentle ping again.
> 
> >>
> >> The related kernel patch is already in misc-next.
> >> I don't find this btrfs-progs patch-set in the devel branch.
> >
> > And what about introducing a test case for this change?
> 
>    btrfs/248 did it.
> 
> Thanks, Anand

Now fstests btrfs/249 always fail because of dmesg output,  any advice?
kernel: 6.0.0-rc2 and 5.15.62

[ 1063.381649] run fstests btrfs/249 at 2022-08-26 08:04:47
[ 1063.593122] BTRFS info (device sdb1): using crc32c (crc32c-intel) checksum algorithm
[ 1063.609363] BTRFS info (device sdb1): using free space tree
[ 1063.618584] BTRFS info (device sdb1): enabling ssd optimizations
[ 1064.768430] BTRFS: device fsid 00ebd666-e838-4377-b9b6-aa69449297af devid 1 transid 6 /dev/sdb2 scanned by systemd-udevd (3602)
[ 1064.781504] BTRFS: device fsid 00ebd666-e838-4377-b9b6-aa69449297af devid 2 transid 6 /dev/sdb3 scanned by systemd-udevd (3605)
[ 1065.065938] BTRFS: device fsid 00ebd666-e838-4377-b9b6-aa69449297af devid 2 transid 7 /dev/sdb3 scanned by systemd-udevd (3602)
[ 1065.088577] BTRFS info (device sdb3): using crc32c (crc32c-intel) checksum algorithm
[ 1065.097656] BTRFS info (device sdb3): allowing degraded mounts
[ 1065.112161] BTRFS info (device sdb3): using free space tree
[ 1065.119832] BTRFS warning (device sdb3): devid 1 uuid 37e015cd-b874-4878-b07e-e12360622063 is missing
[ 1065.132956] BTRFS warning (device sdb3): devid 1 uuid 37e015cd-b874-4878-b07e-e12360622063 is missing
[ 1065.148748] BTRFS info (device sdb3): enabling ssd optimizations
[ 1065.156311] BTRFS info (device sdb3): checking UUID tree
[ 1065.162942] BTRFS warning (device sdb3): Skipping commit of aborted transaction.
[ 1065.171228] ------------[ cut here ]------------
[ 1065.176739] BTRFS: Transaction aborted (error -28)
[ 1065.176773] WARNING: CPU: 5 PID: 1665 at fs/btrfs/transaction.c:1942 btrfs_commit_transaction.cold.40+0x9c/0x326 [btrfs]
[ 1065.188622] Modules linked in: XXXXXXX
[ 1065.188694]  wmi i2c_dev ipmi_devintf ipmi_msghandler
[ 1065.282180] Unloaded tainted modules: XXXXXXX
[ 1065.381811]  pcc_cpufreq():1 acpi_cpufreq():1 pcc_cpufreq():1 acpi_cpufreq():1 pcc_cpufreq():1 acpi_cpufreq():1 acpi_cpufreq():1 fjes():1 fjes():1 fjes():1 fjes():1 fjes():1 fjes():1
[ 1065.495646] CPU: 5 PID: 1665 Comm: kworker/u81:0 Tainted: G        W          6.0.0-2.3.el7.x86_64 #1
[ 1065.506159] Hardware name: Dell Inc. Precision T7610/0NK70N, BIOS A18 09/11/2019
[ 1065.514868] Workqueue: events_unbound btrfs_async_reclaim_metadata_space [btrfs]
[ 1065.523678] RIP: 0010:btrfs_commit_transaction.cold.40+0x9c/0x326 [btrfs]
[ 1065.531855] Code: 30 0a 00 00 03 72 25 41 83 ff fb 0f 84 dc 01 00 00 41 83 ff e2 0f 84 d2 01 00 00 44 89 fe 48 c7 c7 f8 2c bc c4 e8 ac 9b d4 ca <0f> 0b 44 89 f9 ba 96 07 00 00 4c 89 e7 48 c7 c6 b0 68 ba c4 e8 ea
[ 1065.553335] RSP: 0018:ffffaeb5cca4fd28 EFLAGS: 00010282
[ 1065.559946] RAX: 0000000000000000 RBX: ffff8bf409dd1e00 RCX: 0000000000000027
[ 1065.568461] RDX: 0000000000000027 RSI: ffff8c12ef95f860 RDI: ffff8c12ef95f868
[ 1065.576978] RBP: ffff8c1445ceb000 R08: 0000000000000000 R09: c0000000fffdffff
[ 1065.585495] R10: 0000000000000001 R11: ffffaeb5cca4fbc0 R12: ffff8bf411d56f08
[ 1065.594009] R13: ffff8bf411d56e58 R14: ffff8bf411d56e58 R15: 00000000ffffffe4
[ 1065.602502] FS:  0000000000000000(0000) GS:ffff8c12ef940000(0000) knlGS:0000000000000000
[ 1065.611957] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1065.619084] CR2: 00007f55c95fd550 CR3: 0000000472a10003 CR4: 00000000001706e0
[ 1065.627604] Call Trace:
[ 1065.631460]  <TASK>
[ 1065.634955]  ? start_transaction+0xd3/0x5f0 [btrfs]
[ 1065.641288]  flush_space+0x54e/0x5f0 [btrfs]
[ 1065.647029]  ? pick_next_task_fair+0xb0/0x420
[ 1065.652782]  ? put_prev_entity+0x22/0xe0
[ 1065.658096]  ? btrfs_get_alloc_profile+0xc9/0x1b0 [btrfs]
[ 1065.664954]  btrfs_async_reclaim_metadata_space+0xfb/0x240 [btrfs]
[ 1065.672599]  process_one_work+0x1b0/0x390
[ 1065.678005]  worker_thread+0x3c/0x370
[ 1065.683055]  ? process_one_work+0x390/0x390
[ 1065.688606]  kthread+0xe3/0x110
[ 1065.693125]  ? kthread_complete_and_exit+0x20/0x20
[ 1065.699291]  ret_from_fork+0x1f/0x30
[ 1065.704233]  </TASK>
[ 1065.707783] ---[ end trace 0000000000000000 ]---
[ 1065.713762] BTRFS: error (device sdb3: state A) in cleanup_transaction:1942: errno=-28 No space left
[ 1065.724284] BTRFS warning (device sdb3: state EA): btrfs_uuid_scan_kthread failed -5
[ 1065.733433] BTRFS error (device sdb3: state EA): commit super ret -30


Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/08/26

Re: [PATCH 0/2] btrfs-progs: read device fsid from the sysfs

[Anand Jain]

It worked on my end. I am on 6.0.0-rc1+.

Without the btrfs-progs patch mentioned in the test case:

----------------
btrfs/249       [failed, exit status 1]- output mismatch (see 
/xfstests-dev/results//btrfs/249.out.bad)
     --- tests/btrfs/249.out	2022-03-19 20:34:36.307019541 +0800
     +++ /xfstests-dev/results//btrfs/249.out.bad	2022-08-26 
17:23:53.912993906 +0800
     @@ -1,2 +1,5 @@
      QA output created by 249
     -Silence is golden
     +ERROR: unexpected number of devices: 1 >= 1
     +ERROR: if seed device is used, try running this command as root
     +FAILED: btrfs filesystem usage, ret 1. Check btrfs.ko and 
btrfs-progs version.
     +(see /xfstests-dev/results//btrfs/249.full for details)
     ...
     (Run 'diff -u /xfstests-dev/tests/btrfs/249.out 
/xfstests-dev/results//btrfs/249.out.bad'  to see the entire diff)
Ran: btrfs/249
Failures: btrfs/249
Failed 1 of 1 tests
----------------


With the btrfs-progs patch mentioned in the test case:

----------------
btrfs/249        1s
Ran: btrfs/249
Passed all 1 tests
----------------


> [ 1064.768430] BTRFS: device fsid 00ebd666-e838-4377-b9b6-aa69449297af devid 1 transid 6 /dev/sdb2 scanned by systemd-udevd (3602)
> [ 1064.781504] BTRFS: device fsid 00ebd666-e838-4377-b9b6-aa69449297af devid 2 transid 6 /dev/sdb3 scanned by systemd-udevd (3605)
> [ 1065.065938] BTRFS: device fsid 00ebd666-e838-4377-b9b6-aa69449297af devid 2 transid 7 /dev/sdb3 scanned by systemd-udevd (3602)

Two processes 3605 and 3602 are scanning the same device /dev/sdb3.
One finds transid 6, and the other 7.

You may want to re-test on a clean machine again.

Thanks, Anand

Re: [PATCH 0/2] btrfs-progs: read device fsid from the sysfs

[Wang Yugui]

Hi,

> It worked on my end. I am on 6.0.0-rc1+.
> 
> Without the btrfs-progs patch mentioned in the test case:
> 
> ----------------
> btrfs/249       [failed, exit status 1]- output mismatch (see /xfstests-dev/results//btrfs/249.out.bad)
>      --- tests/btrfs/249.out	2022-03-19 20:34:36.307019541 +0800
>      +++ /xfstests-dev/results//btrfs/249.out.bad	2022-08-26 17:23:53.912993906 +0800
>      @@ -1,2 +1,5 @@
>       QA output created by 249
>      -Silence is golden
>      +ERROR: unexpected number of devices: 1 >= 1
>      +ERROR: if seed device is used, try running this command as root
>      +FAILED: btrfs filesystem usage, ret 1. Check btrfs.ko and btrfs-progs version.
>      +(see /xfstests-dev/results//btrfs/249.full for details)
>      ...
>      (Run 'diff -u /xfstests-dev/tests/btrfs/249.out /xfstests-dev/results//btrfs/249.out.bad'  to see the entire diff)
> Ran: btrfs/249
> Failures: btrfs/249
> Failed 1 of 1 tests
> ----------------
> 
> 
> With the btrfs-progs patch mentioned in the test case:
> 
> ----------------
> btrfs/249        1s
> Ran: btrfs/249
> Passed all 1 tests
> ----------------
> 
> 
> > [ 1064.768430] BTRFS: device fsid 00ebd666-e838-4377-b9b6-aa69449297af devid 1 transid 6 /dev/sdb2 scanned by systemd-udevd (3602)
> > [ 1064.781504] BTRFS: device fsid 00ebd666-e838-4377-b9b6-aa69449297af devid 2 transid 6 /dev/sdb3 scanned by systemd-udevd (3605)
> > [ 1065.065938] BTRFS: device fsid 00ebd666-e838-4377-b9b6-aa69449297af devid 2 transid 7 /dev/sdb3 scanned by systemd-udevd (3602)
> 
> Two processes 3605 and 3602 are scanning the same device /dev/sdb3.
> One finds transid 6, and the other 7.
> 
> You may want to re-test on a clean machine again.

btrfs/249 passed on a clean machine here.

Thanks a lot.

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2022/08/27

Re: [PATCH 0/2] btrfs-progs: read device fsid from the sysfs

[Qu Wenruo]

On 2021/10/19 08:23, Anand Jain wrote:
> The following test case fails as it trying to read the fsid from the sb
> for a missing device.
>
>     $ mkfs.btrfs -f -draid1 -mraid1 $DEV1 $DEV2
>     $ btrfstune -S 1 $DEV1

I just wonder, should we even allow seed device setting for multi-device
btrfs?

This doesn't sound correct to me, and can lead to split-brain problems.

Thanks,
Qu
>     $ wipefs -a $DEV2
>     $ btrfs dev scan --forget
>     $ mount -o degraded $DEV1 /btrfs
>     $ btrfs device add $DEV3 /btrfs -f
>
>     $ btrfs fi us /btrfs
>       ERROR: unexpected number of devices: 1 >= 1
>       ERROR: if seed device is used, try running this command as root
>
> The kernel patch [1] in the mailing list provided a sysfs interface
> to read the fsid of the device, so use it instead.
>
>   [1]  btrfs: sysfs add devinfo/fsid to retrieve fsid from the device
>
> This patch also retains the old method that is to read the SB for
> backward compatibility purposes.
>
> Anand Jain (2):
>    btrfs-progs: prepare helper device_is_seed
>    btrfs-progs: read fsid from the sysfs in device_is_seed
>
>   cmds/filesystem-usage.c | 47 ++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 42 insertions(+), 5 deletions(-)
>

Re: [PATCH 0/2] btrfs-progs: read device fsid from the sysfs

[Anand Jain]

On 10/08/2022 18:18, Qu Wenruo wrote:
> 
> 
> On 2021/10/19 08:23, Anand Jain wrote:
>> The following test case fails as it trying to read the fsid from the sb
>> for a missing device.
>>
>>     $ mkfs.btrfs -f -draid1 -mraid1 $DEV1 $DEV2
>>     $ btrfstune -S 1 $DEV1
> 
> I just wonder, should we even allow seed device setting for multi-device
> btrfs?

Hmm. A desktop use-case which gets reset back to the golden image at EOD
or a user logout may use fault tolerance to reduce unplanned downtime.
I say we keep this feature. Further, if we allow a seed device to
replace by another seed device it will help also.

> This doesn't sound correct to me, and can lead to split-brain problems.

Split-brain can happen even without seed devices. There is a patch for
split-brain detection and avoiding. I love to revive it if there are
enough interests.

Thanks, Anand

> Thanks,
> Qu
>>     $ wipefs -a $DEV2
>>     $ btrfs dev scan --forget
>>     $ mount -o degraded $DEV1 /btrfs
>>     $ btrfs device add $DEV3 /btrfs -f
>>
>>     $ btrfs fi us /btrfs
>>       ERROR: unexpected number of devices: 1 >= 1
>>       ERROR: if seed device is used, try running this command as root
>>
>> The kernel patch [1] in the mailing list provided a sysfs interface
>> to read the fsid of the device, so use it instead.
>>
>>   [1]  btrfs: sysfs add devinfo/fsid to retrieve fsid from the device
>>
>> This patch also retains the old method that is to read the SB for
>> backward compatibility purposes.
>>
>> Anand Jain (2):
>>    btrfs-progs: prepare helper device_is_seed
>>    btrfs-progs: read fsid from the sysfs in device_is_seed
>>
>>   cmds/filesystem-usage.c | 47 ++++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 42 insertions(+), 5 deletions(-)
>>

Re: [PATCH 0/2] btrfs-progs: read device fsid from the sysfs

[Qu Wenruo]

On 2022/8/10 19:18, Anand Jain wrote:
>
>
> On 10/08/2022 18:18, Qu Wenruo wrote:
>>
>>
>> On 2021/10/19 08:23, Anand Jain wrote:
>>> The following test case fails as it trying to read the fsid from the sb
>>> for a missing device.
>>>
>>>     $ mkfs.btrfs -f -draid1 -mraid1 $DEV1 $DEV2
>>>     $ btrfstune -S 1 $DEV1
>>
>> I just wonder, should we even allow seed device setting for multi-device
>> btrfs?
>
> Hmm. A desktop use-case which gets reset back to the golden image at EOD
> or a user logout may use fault tolerance to reduce unplanned downtime.
> I say we keep this feature. Further, if we allow a seed device to
> replace by another seed device it will help also.

OK, then I'm mostly fine with the patchset.

But on the other hand, what if the kernel doesn't support the new sysfs?
Shouldn't we need a fallback plan?
Like to use tree-search ioctl to grab all device items from the chunk tree?

Thanks,
Qu

>
>> This doesn't sound correct to me, and can lead to split-brain problems.
>
> Split-brain can happen even without seed devices. There is a patch for
> split-brain detection and avoiding. I love to revive it if there are
> enough interests.
>
> Thanks, Anand
>
>> Thanks,
>> Qu
>>>     $ wipefs -a $DEV2
>>>     $ btrfs dev scan --forget
>>>     $ mount -o degraded $DEV1 /btrfs
>>>     $ btrfs device add $DEV3 /btrfs -f
>>>
>>>     $ btrfs fi us /btrfs
>>>       ERROR: unexpected number of devices: 1 >= 1
>>>       ERROR: if seed device is used, try running this command as root
>>>
>>> The kernel patch [1] in the mailing list provided a sysfs interface
>>> to read the fsid of the device, so use it instead.
>>>
>>>   [1]  btrfs: sysfs add devinfo/fsid to retrieve fsid from the device
>>>
>>> This patch also retains the old method that is to read the SB for
>>> backward compatibility purposes.
>>>
>>> Anand Jain (2):
>>>    btrfs-progs: prepare helper device_is_seed
>>>    btrfs-progs: read fsid from the sysfs in device_is_seed
>>>
>>>   cmds/filesystem-usage.c | 47 ++++++++++++++++++++++++++++++++++++-----
>>>   1 file changed, 42 insertions(+), 5 deletions(-)
>>>