| Message ID | 33e179f8b9341c88754df639b77dafaa1ffec0d1.1634829757.git.anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | provide fsid in sysfs devinfo | expand |
On Thu, Oct 21, 2021 at 11:31:17PM +0800, Anand Jain wrote: > In the case of the seed device, the fsid can be different from the mounted > sprout fsid. The userland has to read the device superblock to know the > fsid but, that idea fails if the device is missing. So add a sysfs > interface devinfo/<devid>/fsid to show the fsid of the device. > > For example: > $ cd /sys/fs/btrfs/b10b02a5-f9de-4276-b9e8-2bfd09a578a8 > > $ cat devinfo/1/fsid > c44d771f-639d-4df3-99ec-5bc7ad2af93b > $ cat devinfo/3/fsid > b10b02a5-f9de-4276-b9e8-2bfd09a578a8 From user perspective, it's another fsid, one is in the path, so I'm wondering if it should be named like read_fsid or sprout_fsid or if the seed/sprout information should be put into another directory completely.
On 17/11/2021 01:16, David Sterba wrote: > On Thu, Oct 21, 2021 at 11:31:17PM +0800, Anand Jain wrote: >> In the case of the seed device, the fsid can be different from the mounted >> sprout fsid. The userland has to read the device superblock to know the >> fsid but, that idea fails if the device is missing. So add a sysfs >> interface devinfo/<devid>/fsid to show the fsid of the device. >> >> For example: >> $ cd /sys/fs/btrfs/b10b02a5-f9de-4276-b9e8-2bfd09a578a8 >> >> $ cat devinfo/1/fsid >> c44d771f-639d-4df3-99ec-5bc7ad2af93b >> $ cat devinfo/3/fsid >> b10b02a5-f9de-4276-b9e8-2bfd09a578a8 > > From user perspective, it's another fsid, one is in the path, so I'm > wondering if it should be named like read_fsid or sprout_fsid or if the > seed/sprout information should be put into another directory completely. I am viewing it as fsid as per the device's sb. This fsid matches with the blkid(8) output. A path to the device's fsid will help to script. So I am not voting for sprout_fsid because it does not exist in the most common non-sprouted fsid. Now to show whether a device is seed or sprout, the user friendly approach will be like for example: /sys/fs/btrfs/<fsid>/devinfo/<devid>/device_state to show all that apply, for example: SEED|SPROUT|READ_ONLY|WRITABLE|ZONED|METADATA_ONLY|\ DATA_ONLY|READ_PREFERRED|REPLACE_TGT|REPLACE_SRC|\ SCRUB_RUNNING However, per kernel general rule of thumb one value per sys-fs file, so it should be for example: /sys/fs/btrfs/<fsid>/devinfo/<devid>/seed 0 /sys/fs/btrfs/<fsid>/devinfo/<devid>/sprout 1 So based on this my patch "btrfs: introduce new device-state read_preferred" in the mailing list already proposed /sys/fs/btrfs/<fsid>/devinfo/<devid>/read_preferred So to summarize, I can rename fsid to read_fsid. And, progressively in a separate patch, we can add <>/seed <>/sprout sysfs files as above. What do you think?
David, Gentle ping on this patch? Thanks, Anand On 17/11/2021 11:29, Anand Jain wrote: > > > On 17/11/2021 01:16, David Sterba wrote: >> On Thu, Oct 21, 2021 at 11:31:17PM +0800, Anand Jain wrote: >>> In the case of the seed device, the fsid can be different from the >>> mounted >>> sprout fsid. The userland has to read the device superblock to know the >>> fsid but, that idea fails if the device is missing. So add a sysfs >>> interface devinfo/<devid>/fsid to show the fsid of the device. >>> >>> For example: >>> $ cd /sys/fs/btrfs/b10b02a5-f9de-4276-b9e8-2bfd09a578a8 >>> >>> $ cat devinfo/1/fsid >>> c44d771f-639d-4df3-99ec-5bc7ad2af93b >>> $ cat devinfo/3/fsid >>> b10b02a5-f9de-4276-b9e8-2bfd09a578a8 >> >> From user perspective, it's another fsid, one is in the path, so I'm >> wondering if it should be named like read_fsid or sprout_fsid or if the >> seed/sprout information should be put into another directory completely. > > I am viewing it as fsid as per the device's sb. This fsid matches with > the blkid(8) output. A path to the device's fsid will help to script. > So I am not voting for sprout_fsid because it does not exist in the > most common non-sprouted fsid. > > Now to show whether a device is seed or sprout, the user friendly > approach will be like for example: > /sys/fs/btrfs/<fsid>/devinfo/<devid>/device_state > to show all that apply, for example: > SEED|SPROUT|READ_ONLY|WRITABLE|ZONED|METADATA_ONLY|\ > DATA_ONLY|READ_PREFERRED|REPLACE_TGT|REPLACE_SRC|\ > SCRUB_RUNNING > > However, per kernel general rule of thumb one value per sys-fs file, > so it should be for example: > /sys/fs/btrfs/<fsid>/devinfo/<devid>/seed > 0 > /sys/fs/btrfs/<fsid>/devinfo/<devid>/sprout > 1 > > So based on this my patch "btrfs: introduce new device-state > read_preferred" in the mailing list already proposed > /sys/fs/btrfs/<fsid>/devinfo/<devid>/read_preferred > > So to summarize, I can rename fsid to read_fsid. And, progressively > in a separate patch, we can add <>/seed <>/sprout sysfs files as above. > What do you think?
On Wed, Nov 17, 2021 at 11:29:26AM +0800, Anand Jain wrote: > On 17/11/2021 01:16, David Sterba wrote: > > On Thu, Oct 21, 2021 at 11:31:17PM +0800, Anand Jain wrote: > >> In the case of the seed device, the fsid can be different from the mounted > >> sprout fsid. The userland has to read the device superblock to know the > >> fsid but, that idea fails if the device is missing. So add a sysfs > >> interface devinfo/<devid>/fsid to show the fsid of the device. > >> > >> For example: > >> $ cd /sys/fs/btrfs/b10b02a5-f9de-4276-b9e8-2bfd09a578a8 > >> > >> $ cat devinfo/1/fsid > >> c44d771f-639d-4df3-99ec-5bc7ad2af93b > >> $ cat devinfo/3/fsid > >> b10b02a5-f9de-4276-b9e8-2bfd09a578a8 > > > > From user perspective, it's another fsid, one is in the path, so I'm > > wondering if it should be named like read_fsid or sprout_fsid or if the > > seed/sprout information should be put into another directory completely. > > I am viewing it as fsid as per the device's sb. That's a reasonable point. > This fsid matches with > the blkid(8) output. A path to the device's fsid will help to script. > So I am not voting for sprout_fsid because it does not exist in the > most common non-sprouted fsid. Yeah agreed, I've put this to the changelog as it explains the naming.
On Thu, Oct 21, 2021 at 11:31:17PM +0800, Anand Jain wrote: > In the case of the seed device, the fsid can be different from the mounted > sprout fsid. The userland has to read the device superblock to know the > fsid but, that idea fails if the device is missing. So add a sysfs > interface devinfo/<devid>/fsid to show the fsid of the device. > > For example: > $ cd /sys/fs/btrfs/b10b02a5-f9de-4276-b9e8-2bfd09a578a8 > > $ cat devinfo/1/fsid > c44d771f-639d-4df3-99ec-5bc7ad2af93b > $ cat devinfo/3/fsid > b10b02a5-f9de-4276-b9e8-2bfd09a578a8 > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > Reviewed-by: Josef Bacik <josef@toxicpanda.com> Added to misc-next, thanks.
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 28ff7fce1ac5..591d49deb552 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -1539,6 +1539,16 @@ static ssize_t btrfs_devinfo_writeable_show(struct kobject *kobj, } BTRFS_ATTR(devid, writeable, btrfs_devinfo_writeable_show); +static ssize_t btrfs_devinfo_fsid_show(struct kobject *kobj, + struct kobj_attribute *a, char *buf) +{ + struct btrfs_device *device = container_of(kobj, struct btrfs_device, + devid_kobj); + + return sysfs_emit(buf, "%pU\n", device->fs_devices->fsid); +} +BTRFS_ATTR(devid, fsid, btrfs_devinfo_fsid_show); + static ssize_t btrfs_devinfo_error_stats_show(struct kobject *kobj, struct kobj_attribute *a, char *buf) { @@ -1579,6 +1589,7 @@ static struct attribute *devid_attrs[] = { BTRFS_ATTR_PTR(devid, replace_target), BTRFS_ATTR_PTR(devid, scrub_speed_max), BTRFS_ATTR_PTR(devid, writeable), + BTRFS_ATTR_PTR(devid, fsid), NULL }; ATTRIBUTE_GROUPS(devid);