Re: [PATCH] btrfs-progs: remove loopback device resolution

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2025/1/16 22:12, David Sterba 写道:
> On Thu, Jan 16, 2025 at 08:13:01PM +1030, Qu Wenruo wrote:
>> [BUG]
>> mkfs.btrfs has a built-in loopback device resolution, to avoid the same
>> file being added to the same fs, using loopback device and the file
>> itself.
>>
>> But it has one big bug:
>>
>> - It doesn't detect partition on loopback devices correctly
>>    The function is_loop_device() only utilize major number to detect a
>>    loopback device.
>>    But partitions on loopback devices doesn't use the same major number
>>    as the loopback device:
>>
>>    NAME            MAJ:MIN RM  SIZE RO TYPE  MOUNTPOINTS
>>    loop0             7:0    0    5G  0 loop
>>    |-loop0p1       259:3    0  128M  0 part
>>    `-loop0p2       259:4    0  4.9G  0 part
>>
>>    Thus `/dev/loop0p1` will not be treated as a loopback device, thus it
>>    will not even resolve the source file.
>>
>>    And this can not even be fixed, as if we do extra "/dev/loop*" based
>>    file lookup, `/dev/loop0p1` and `/dev/loop0p2` will resolve to the
>>    same source file, and refuse to mkfs on two different partitions.
>>
>> [FIX]
>> The loopback file detection is the baby sitting that no one asks for.
>>
>> Just as I explained, it only brings new bugs, and we will never fix all
>> ways that an experienced user can come up with.
>> And I didn't see any other mkfs tool doing such baby sitting.
>>
>> So remove the loopback file resolution, just regular is_same_blk_file()
>> is good enough.
>
> The loop device resolution had some bugs in the past and was added for a
> reason that's not mentioned in the changelogs.
>
> The commits have some details why it's done, they also mention that
> partitioned devices are resolved so it's not that there's no support for
> that.
>
> 0cf3b78f404b01 ("btrfs-progs: Fix partitioned loop devices resolving")
> abdb0ced0123d4 ("Btrfs-progs: fix resolving of loop devices")

The problem of those two fixes are pretty simple, they just do not work
in the first place.

resolve_loop_device() is only triggered if is_loop_dev() returns >0 values.

But as I explained, resolve_loop_device() won't return >0 if a
partitioned loopback device is passed.


So why those patches are introduced in the first place?

Thanks,
Qu
> 09559bfe7bcd43 ("multidevice support for check_mounted")
>
> and some fixup commits.
>
> So before removing the code completely I'd like to see if there's a use
> case that can be broken, but I don't have anything in particular.
> There's only mkfs-tests/006-partitioned-loopdev that's quite simple and
> does not try to do any tricks with symlinks or partitions on the
> devices.
>