* [PATCH 1/2] btrfs-progs: prepare helper device_is_seed
2023-02-13 9:37 [PATCH v2 0/2] btrfs-progs: read device fsid from the sysfs Anand Jain
@ 2023-02-13 9:37 ` Anand Jain
2023-02-21 23:16 ` David Sterba
2023-02-13 9:37 ` [PATCH 2/2] btrfs-progs: read fsid from the sysfs in device_is_seed Anand Jain
2023-02-21 21:39 ` [PATCH v2 0/2] btrfs-progs: read device fsid from the sysfs David Sterba
2 siblings, 1 reply; 7+ messages in thread
From: Anand Jain @ 2023-02-13 9:37 UTC (permalink / raw)
To: linux-btrfs; +Cc: wqu, Josef Bacik
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>
Reviewed-by: Josef Bacik <josef@toxicpanda.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 5810324f245e..bef9a1129a63 100644
--- a/cmds/filesystem-usage.c
+++ b/cmds/filesystem-usage.c
@@ -27,6 +27,7 @@
#include <fcntl.h>
#include <dirent.h>
#include <limits.h>
+#include <uuid/uuid.h>
#include "kernel-lib/sizes.h"
#include "kernel-shared/ctree.h"
#include "kernel-shared/disk-io.h"
@@ -700,6 +701,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
*/
@@ -710,7 +726,6 @@ static int load_device_info(int fd, struct device_info **devinfo_ret,
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];
*devcount_ret = 0;
*devinfo_ret = NULL;
@@ -754,8 +769,8 @@ static int load_device_info(int fd, struct device_info **devinfo_ret,
* 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.39.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/2] btrfs-progs: prepare helper device_is_seed
2023-02-13 9:37 ` [PATCH 1/2] btrfs-progs: prepare helper device_is_seed Anand Jain
@ 2023-02-21 23:16 ` David Sterba
0 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2023-02-21 23:16 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs, wqu, Josef Bacik
On Mon, Feb 13, 2023 at 05:37:41PM +0800, Anand Jain wrote:
> 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>
> Reviewed-by: Josef Bacik <josef@toxicpanda.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 5810324f245e..bef9a1129a63 100644
> --- a/cmds/filesystem-usage.c
> +++ b/cmds/filesystem-usage.c
> @@ -27,6 +27,7 @@
> #include <fcntl.h>
> #include <dirent.h>
> #include <limits.h>
> +#include <uuid/uuid.h>
> #include "kernel-lib/sizes.h"
> #include "kernel-shared/ctree.h"
> #include "kernel-shared/disk-io.h"
> @@ -700,6 +701,21 @@ out:
> return ret;
> }
>
> +static int device_is_seed(const char *dev_path, u8 *mnt_fsid)
> +{
> + uuid_t fsid;
I've switched this to a u8[BTFFS_UUID_SIZE] buffer, as it was in the
other funcion.
> + int ret;
> +
> + ret = dev_to_fsid(dev_path, fsid);
> + if (ret)
> + return ret;
> +
> + if (memcmp(mnt_fsid, fsid, BTRFS_FSID_SIZE))
IMO strcmp and memcmp should use the == 0 or != 0 explicitly so it's
closer to the meaning.
> + return 0;
> +
> + return -1;
> +}
> +
> /*
> * This function loads the device_info structure and put them in an array
> */
> @@ -710,7 +726,6 @@ static int load_device_info(int fd, struct device_info **devinfo_ret,
> 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];
>
> *devcount_ret = 0;
> *devinfo_ret = NULL;
> @@ -754,8 +769,8 @@ static int load_device_info(int fd, struct device_info **devinfo_ret,
> * 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.39.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] btrfs-progs: read fsid from the sysfs in device_is_seed
2023-02-13 9:37 [PATCH v2 0/2] btrfs-progs: read device fsid from the sysfs Anand Jain
2023-02-13 9:37 ` [PATCH 1/2] btrfs-progs: prepare helper device_is_seed Anand Jain
@ 2023-02-13 9:37 ` Anand Jain
2023-02-21 23:17 ` David Sterba
2023-02-21 21:39 ` [PATCH v2 0/2] btrfs-progs: read device fsid from the sysfs David Sterba
2 siblings, 1 reply; 7+ messages in thread
From: Anand Jain @ 2023-02-13 9:37 UTC (permalink / raw)
To: linux-btrfs; +Cc: wqu, Josef Bacik
The kernel commit a26d60dedf9a ("btrfs: sysfs: add devinfo/fsid to
retrieve actual fsid from the device" introduced a sysfs interface
to access the device's fsid from the kernel. This is a more
reliable method to obtain the fsid compared to reading the
superblock, and it even works if the device is not present.
Additionally, this sysfs interface can be read by non-root users.
Therefore, it is recommended to utilize this new sysfs interface to
retrieve the fsid.
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
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 bef9a1129a63..e7fa18dc82dc 100644
--- a/cmds/filesystem-usage.c
+++ b/cmds/filesystem-usage.c
@@ -39,6 +39,7 @@
#include "common/help.h"
#include "common/device-utils.h"
#include "common/messages.h"
+#include "common/path-utils.h"
#include "cmds/filesystem-usage.h"
#include "cmds/commands.h"
@@ -701,14 +702,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;
@@ -763,13 +783,15 @@ static int load_device_info(int fd, struct device_info **devinfo_ret,
}
/*
- * 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.39.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] btrfs-progs: read fsid from the sysfs in device_is_seed
2023-02-13 9:37 ` [PATCH 2/2] btrfs-progs: read fsid from the sysfs in device_is_seed Anand Jain
@ 2023-02-21 23:17 ` David Sterba
0 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2023-02-21 23:17 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs, wqu, Josef Bacik
On Mon, Feb 13, 2023 at 05:37:42PM +0800, Anand Jain wrote:
> The kernel commit a26d60dedf9a ("btrfs: sysfs: add devinfo/fsid to
> retrieve actual fsid from the device" introduced a sysfs interface
> to access the device's fsid from the kernel. This is a more
> reliable method to obtain the fsid compared to reading the
> superblock, and it even works if the device is not present.
> Additionally, this sysfs interface can be read by non-root users.
>
> Therefore, it is recommended to utilize this new sysfs interface to
> retrieve the fsid.
>
> Reviewed-by: Josef Bacik <josef@toxicpanda.com>
> 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 bef9a1129a63..e7fa18dc82dc 100644
> --- a/cmds/filesystem-usage.c
> +++ b/cmds/filesystem-usage.c
> @@ -39,6 +39,7 @@
> #include "common/help.h"
> #include "common/device-utils.h"
> #include "common/messages.h"
> +#include "common/path-utils.h"
> #include "cmds/filesystem-usage.h"
> #include "cmds/commands.h"
>
> @@ -701,14 +702,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");
This could potentially fail so the return value needs to be checked.
> +
> + /* /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;
> + }
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/2] btrfs-progs: read device fsid from the sysfs
2023-02-13 9:37 [PATCH v2 0/2] btrfs-progs: read device fsid from the sysfs Anand Jain
2023-02-13 9:37 ` [PATCH 1/2] btrfs-progs: prepare helper device_is_seed Anand Jain
2023-02-13 9:37 ` [PATCH 2/2] btrfs-progs: read fsid from the sysfs in device_is_seed Anand Jain
@ 2023-02-21 21:39 ` David Sterba
2023-02-22 3:16 ` Anand Jain
2 siblings, 1 reply; 7+ messages in thread
From: David Sterba @ 2023-02-21 21:39 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs, wqu
On Mon, Feb 13, 2023 at 05:37:40PM +0800, Anand Jain wrote:
> v2:
> Almost a resend; no code was altered, except for the change log.
>
> The following test scenario (as in fstests btrfs/249) shows an issue
> where the "usage" subcommand fails to retrieve the fsid from the
> superblock for a missing device.
>
> Create a raid1 seed device while one of its device missing.
> $ mkfs.btrfs -f -draid1 -mraid1 $DEV1 $DEV2
> $ btrfstune -S 1 $DEV1
> $ wipefs -a $DEV2
>
> Mount the seed device
> $ btrfs device scan --forget
> $ mount -o degraded $DEV1 /btrfs
>
> Add a sprout device
> $ btrfs device add $DEV3 /btrfs -f
>
> The usage subommand fails because we try to read superblock for the missing
> device
> $ btrfs filesystem usage /btrfs
> ERROR: unexpected number of devices: 1 >= 1
> ERROR: if seed device is used, try running this command as root
>
> The commit a26d60dedf9a ("btrfs: sysfs: add devinfo/fsid to retrieve
> actual fsid from the device") introduced a sysfs interface for
> retrieving the fsid of a device. This change allows for the reading of the
> device fsid through the sysfs interface in the kernel, while retaining the
> old method of reading the superblock from the disk for backward
> compatibility during normal, non-missing device conditions.
>
> Anand Jain (2):
> btrfs-progs: prepare helper device_is_seed
> btrfs-progs: read fsid from the sysfs in device_is_seed
Added to devel with some fixups, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 0/2] btrfs-progs: read device fsid from the sysfs
2023-02-21 21:39 ` [PATCH v2 0/2] btrfs-progs: read device fsid from the sysfs David Sterba
@ 2023-02-22 3:16 ` Anand Jain
0 siblings, 0 replies; 7+ messages in thread
From: Anand Jain @ 2023-02-22 3:16 UTC (permalink / raw)
To: dsterba; +Cc: linux-btrfs, wqu
On 22/02/2023 05:39, David Sterba wrote:
> On Mon, Feb 13, 2023 at 05:37:40PM +0800, Anand Jain wrote:
>> v2:
>> Almost a resend; no code was altered, except for the change log.
>>
>> The following test scenario (as in fstests btrfs/249) shows an issue
>> where the "usage" subcommand fails to retrieve the fsid from the
>> superblock for a missing device.
>>
>> Create a raid1 seed device while one of its device missing.
>> $ mkfs.btrfs -f -draid1 -mraid1 $DEV1 $DEV2
>> $ btrfstune -S 1 $DEV1
>> $ wipefs -a $DEV2
>>
>> Mount the seed device
>> $ btrfs device scan --forget
>> $ mount -o degraded $DEV1 /btrfs
>>
>> Add a sprout device
>> $ btrfs device add $DEV3 /btrfs -f
>>
>> The usage subommand fails because we try to read superblock for the missing
>> device
>> $ btrfs filesystem usage /btrfs
>> ERROR: unexpected number of devices: 1 >= 1
>> ERROR: if seed device is used, try running this command as root
>>
>> The commit a26d60dedf9a ("btrfs: sysfs: add devinfo/fsid to retrieve
>> actual fsid from the device") introduced a sysfs interface for
>> retrieving the fsid of a device. This change allows for the reading of the
>> device fsid through the sysfs interface in the kernel, while retaining the
>> old method of reading the superblock from the disk for backward
>> compatibility during normal, non-missing device conditions.
>>
>> Anand Jain (2):
>> btrfs-progs: prepare helper device_is_seed
>> btrfs-progs: read fsid from the sysfs in device_is_seed
>
> Added to devel with some fixups, thanks.
Fixups look good in the devel branch.
Thanks, Anand
^ permalink raw reply [flat|nested] 7+ messages in thread