Re: [PATCH][TRIVIAL] btrfs-progs: Allow autodetect_object_types() to handle the link.

[Boris Burkov <boris@bur.io>]

On Wed, Jan 05, 2022 at 07:23:33PM +0100, Goffredo Baroncelli wrote:
> On 05/01/2022 18.50, Boris Burkov wrote:
> > On Wed, Jan 05, 2022 at 02:32:58PM +0100, Goffredo Baroncelli wrote:
> > > From: Goffredo Baroncelli <kreijack@inwind.it>
> [...]
> > 
> > I took a look at the original lstat and it doesn't seem super strongly
> > motivated. It is used to detect a subvolume object (ino==256) so I don't
> > see why we would hate to have property get/set work on a symlink to a
> > subvol.
> 
> It is not so simple: think about a case where the symlink points to a
> subvolume of *another* filesystem.
> 
> Now, "btrfs prop get" returns the property (e.g. the label) of the *underlining*
> filesystem. If we change statl to stat, it still return the property of the
> underlining filesystem, thinking that it is a subvolume (of a foreign filesystem).
> 
> If fact I added an exception of that rule, because if we pass a block device, we
> don't consider the underling filesystem, but the filesystem contained in the block
> device. But there is a precedence in get/set label.
> 
> Anyway, symlink created some confusion, and I bet that in btrfs there are areas
> with incoherence around symlink between the pointed object and the underling
> filesystem.

Good point. I agree that btrfs (the tool) is not the most rigorous with
symlinks. In this very function, we check if it is a btrfs object by
opening the file without O_NOFOLLOW, but then we do this lstat.

I'm not exactly sure what you are alluding to regarding the precedent set
by label, but I tested links and labels and it seems to exhibit the link
following behavior.

mkfs.btrfs -f /dev/loop0
mkfs.btrfs -f -L LOOP /dev/loop1
mount /dev/loop0 /mnt/0
mount /dev/loop1 /mnt/1
ln -s /mnt/1 /mnt/0/lnk
btrfs property get /mnt/0 label
label=
btrfs property get /mnt/1 label
label=LOOP
btrfs property get /mnt/0/lnk label
label=LOOP
btrfs property get /mnt/0/lnk ro
ERROR: object is not compatible with property: ro

So it looks like root detection follows links but subvol detection does
not. Without testing, but judging by the code, I think inode follows
symlinks too. So with all that said, I think the most consistent is to
make subvol follow the symlink, unless you have a really confusing
unexpected behavior in mind with links out to another btrfs that I am
failing to see.

Thanks,
Boris

> 
> BR
> G.Baroncelli
> 
> -- 
> gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
> Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5