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

[David Sterba <dsterba@suse.cz>]

On Fri, Jan 17, 2025 at 07:22:09AM +1030, Qu Wenruo wrote:
> 
> 
> 在 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?

I don't know, all I'm asking here is to keep protection against some
accidental use of loop device with mkfs. I looks similar to what we do
with regular block devices, the same device cannot be specified twice.
If the loop device is buggy then it'll be better to fix it then delete
it.