Re: [PATCH] btrfs-progs: Fix get_partition_sector_size_sysfs() to handle loopback and device mapper devices

[Qu Wenruo <wqu@suse.com>]

在 2025/8/1 20:33, Zoltan Racz 写道:
> Commit e39ed66 added get_partition_sector_size_sysfs() used by "btrfs device usage"
> which returns the sector size of a partition (or its parent). After more testing
> it turned out it couldn`t handle loopback or mapper devices. This patch adds a fix
> for them.

After more digging into the sysfs implementation and how util-linux is 
handling the sysfs block device size, it turns out that 
/sys/block/<dev>/size is always in sector unit (512 bytes).

It's not related to any block size in the queue directory.


The core code implementing this is from block/genhd.c, function 
part_size_show():

	sysfs_emit(buf, "%llu\n", bdev_nr_sectors(dev_to_bdev(dev));

BTW, to my surprise, this very basic member is not explained in any 
sysfs related docs...

So we don't need all those complex workaround at all, just a simple 
shift with SECTOR_SHIFT will solve the problem.

I'll update your previous fix to use /sys/block/<dev>/size with left 
shift directly.

Thanks,
Qu

> 
> Signed-off-by: Zoltan Racz <racz.zoli@gmail.com>
> ---
>   common/device-utils.c | 48 +++++++++++++++++++++++++++++--------------
>   1 file changed, 33 insertions(+), 15 deletions(-)
> 
> diff --git a/common/device-utils.c b/common/device-utils.c
> index dd781bc5..a75194bf 100644
> --- a/common/device-utils.c
> +++ b/common/device-utils.c
> @@ -353,26 +353,44 @@ static ssize_t get_partition_sector_size_sysfs(const char *name)
>   	char sysfs[PATH_MAX] = {};
>   	char sizebuf[128];
>   
> -	snprintf(link_path, PATH_MAX, "/sys/class/block/%s/..", name);
> +	/*
> +	 * First we look for hw_sector_size directly directly under
> +	 * /sys/class/block/[partition_name]/queue. In case of loopback and
> +	 * device mapper devices there is no parent device (like /dev/sda1 -> /dev/sda),
> +	 * and the partition`s sysfs folder itself contains informations regarding
> +	 * the sector size
> +	 */
> +	snprintf(sysfs, PATH_MAX, "/sys/class/block/%s/queue/hw_sector_size", name);
> +	sysfd = open(sysfs, O_RDONLY);
>   
> -	if (!realpath(link_path, real_path)) {
> -		error("Failed to resolve realpath of %s: %s\n", link_path, strerror(errno));
> -		return -1;
> -	}
> +	if (sysfd < 0) {
> +		/*
> +		 * If we couldn`t find it, it means our partition is created on a real
> +		 * device and we need to find its parent
> +		 */
> +		snprintf(link_path, PATH_MAX, "/sys/class/block/%s/..", name);
>   
> -	dev_name = basename(real_path);
> +		if (!realpath(link_path, real_path)) {
> +			error("Failed to resolve realpath of %s: %s\n", link_path, strerror(errno));
> +			return -1;
> +		}
>   
> -	if (!dev_name) {
> -		error("Failed to determine basename for path %s\n", real_path);
> -		return -1;
> -	}
> +		dev_name = basename(real_path);
>   
> -	snprintf(sysfs, PATH_MAX, "/sys/class/block/%s/queue/hw_sector_size", dev_name);
> +		if (!dev_name) {
> +			error("Failed to determine basename for path %s\n", real_path);
> +			return -1;
> +		}
>   
> -	sysfd = open(sysfs, O_RDONLY);
> -	if (sysfd < 0) {
> -		error("Error opening %s to determine dev sector size: %s\n", real_path, strerror(errno));
> -		return -1;
> +		memset(sysfs, 0, PATH_MAX);
> +		snprintf(sysfs, PATH_MAX, "/sys/class/block/%s/queue/hw_sector_size", dev_name);
> +
> +		sysfd = open(sysfs, O_RDONLY);
> +
> +		if (sysfd < 0) {
> +			error("Error opening %s to determine dev sector size: %s\n", real_path, strerror(errno));
> +			return -1;
> +		}
>   	}
>   
>   	ret = sysfs_read_file(sysfd, sizebuf, sizeof(sizebuf));