Re: [PATCH] btrfs-progs: mkfs: use path_canonicalize for input device

[Anand Jain <anand.jain@oracle.com>]

On 16/5/25 09:43, Qu Wenruo wrote:
> 
> 
> 在 2025/5/15 23:46, Anand Jain 写道:
>> Canonicalize the input device's path before using it.
>> So that we show the device path that matches with the /proc/fs/
>>
>> Before:
>>    $ mkfs.btrfs -fq /dev/vg_fstests/lv1 /dev/sdb > /dev/null
>>
>>    $ blkid --uuid c3bf2107-292d-4c7f-a288-0fa922ebd71a
>>    /dev/mapper/vg_fstests-lv1
>>
>>    $ mount --verbose /dev/vg_fstests/lv1 /mnt/scratch
>>    mount: /dev/mapper/vg_fstests-lv1 mounted on /mnt/scratch.
>>
>>    $ cat /proc/self/mounts | grep scratch
>>    /dev/vg_fstests/lv1 /mnt/scratch btrfs 
>> rw,relatime,space_cache=v2,subvolid=5,subvol=/ 0 0
>>
>> After:
>>    $ mkfs.btrfs -fq /dev/vg_fstests/lv1 /dev/sdb > /dev/null
>>
>>    $ blkid --uuid c774b4a6-3ad2-4b15-834a-894dfc898aa9
>>    /dev/mapper/vg_fstests-lv1
>>
>>    $ mount --verbose /dev/vg_fstests/lv1 /mnt/scratch
>>    mount: /dev/mapper/vg_fstests-lv1 mounted on /mnt/scratch.
>>
>>    $ cat /proc/self/mounts | grep scratch
>>    /dev/mapper/vg_fstests-lv1 /mnt/scratch btrfs 
>> rw,relatime,space_cache=v2,subvolid=5,subvol=/ 0 0
> 
> I do not think this is the correct way to go.
> 
> It looks more like a bug in libblkid.

This is a bug in util-linux, as I've already reported in the Link below.

Before kernel patch 2e8b6bc0ab41 ("btrfs: avoid unnecessary device path
update for the same device"), the mount command could update the device
path. After this patch, such updates are blocked — the path set at
mkfs.btrfs time is now retained. Meanwhile, mount still resolves /dev/
dm-0 to its equivalent /dev/mapper/..., which matches what tools like
findmnt report. So idea in this patch to register with the mapper
device path.

In util-linux 2.37.4, if findmnt's path doesn't match what
show_devname() returns, mount -a (by UUID) fails with -EBUSY. In 2.40.2,
the failure is avoided — but verbose is misleadingly reports
"successfully mounted" even when the device is already mounted. By
contrast, ext4 and xfs behave correctly — they return "already mounted",
which is accurate and expected.

Thanks, Anand

> 
> Please explain why the problem happens, not workaround it without any 
> reasons.
> 
> Thanks,
> Qu
> 
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> Link: https://lore.kernel.org/linux- 
>> btrfs/5f401c48-29f5-403e-8c39-50188028ad00@oracle.com/
>> ---
>>   mkfs/main.c | 13 +++++++------
>>   1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/mkfs/main.c b/mkfs/main.c
>> index 4c2ce98c784c..e6466d88313a 100644
>> --- a/mkfs/main.c
>> +++ b/mkfs/main.c
>> @@ -1537,7 +1537,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>>       }
>>       for (i = 0; i < device_count; i++) {
>> -        file = argv[optind++];
>> +        file = path_canonicalize(argv[optind++]);
>>           if (source_dir && path_exists(file) == 0)
>>               ret = 0;
>> @@ -1553,7 +1553,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>>       optind = saved_optind;
>>       device_count = argc - optind;
>> -    file = argv[optind++];
>> +    file = path_canonicalize(argv[optind++]);
>>       ssd = device_get_rotational(file);
>>       if (opt_zoned) {
>>           if (!zone_size(file)) {
>> @@ -1752,7 +1752,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>>       for (i = saved_optind; i < saved_optind + device_count; i++) {
>>           char *path;
>> -        path = argv[i];
>> +        path = path_canonicalize(argv[i]);
>>           ret = test_minimum_size(path, min_dev_size);
>>           if (ret < 0) {
>>               error("failed to check size for %s: %m", path);
>> @@ -1816,7 +1816,8 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>>       opt_oflags = O_RDWR;
>>       for (i = 0; i < device_count; i++) {
>>           if (opt_zoned &&
>> -            zoned_model(argv[optind + i - 1]) == ZONED_HOST_MANAGED) {
>> +            zoned_model(path_canonicalize(argv[optind + i - 1])) ==
>> +                            ZONED_HOST_MANAGED) {
>>               opt_oflags |= O_DIRECT;
>>               break;
>>           }
>> @@ -1824,7 +1825,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>>       /* Start threads */
>>       for (i = 0; i < device_count; i++) {
>> -        prepare_ctx[i].file = argv[optind + i - 1];
>> +        prepare_ctx[i].file = path_canonicalize(argv[optind + i - 1]);
>>           prepare_ctx[i].byte_count = byte_count;
>>           prepare_ctx[i].dev_byte_count = byte_count;
>>           ret = pthread_create(&t_prepare[i], NULL, prepare_one_device,
>> @@ -2198,7 +2199,7 @@ out:
>>           optind = saved_optind;
>>           device_count = argc - optind;
>>           while (device_count-- > 0) {
>> -            file = argv[optind++];
>> +            file = path_canonicalize(argv[optind++]);
>>               if (path_is_block_device(file) == 1)
>>                   btrfs_register_one_device(file);
>>           }
>