On 29/07/2021 02:39, NeilBrown wrote: > On Wed, 28 Jul 2021, g.btrfs@xxxxxxxxxxx wrote: >> On 28/07/2021 08:01, NeilBrown wrote: >>> On Wed, 28 Jul 2021, Wang Yugui wrote: >>>> Hi, >>>> >>>> This patchset works well in 5.14-rc3. >>> >>> Thanks for testing. >>> >>>> >>>> 1, fixed dummy inode(255, BTRFS_FIRST_FREE_OBJECTID - 1 ) is changed to >>>> dynamic dummy inode(18446744073709551358, or 18446744073709551359, ...) >>> >>> The BTRFS_FIRST_FREE_OBJECTID-1 was a just a hack, I never wanted it to >>> be permanent. >>> The new number is ULONG_MAX - subvol_id (where subvol_id starts at 257 I >>> think). >>> This is a bit less of a hack. It is an easily available number that is >>> fairly unique. >>> >>>> >>>> 2, btrfs subvol mount info is shown in /proc/mounts, even if nfsd/nfs is >>>> not used. >>>> /dev/sdc btrfs 94G 3.5M 93G 1% /mnt/test >>>> /dev/sdc btrfs 94G 3.5M 93G 1% /mnt/test/sub1 >>>> /dev/sdc btrfs 94G 3.5M 93G 1% /mnt/test/sub2 >>>> >>>> This is a visiual feature change for btrfs user. >>> >>> Hopefully it is an improvement. But it is certainly a change that needs >>> to be carefully considered. >> >> Would this change the behaviour of findmnt? I have several scripts that >> depend on findmnt to select btrfs filesystems. Just to take a couple of >> examples (using the example shown above): my scripts would depend on >> 'findmnt --target /mnt/test/sub1 -o target' providing /mnt/test, not the >> subvolume; and another script would depend on 'findmnt -t btrfs >> --mountpoint /mnt/test/sub1' providing no output as the directory is not >> an /etc/fstab mount point for a btrfs filesystem. > > Yes, I think it does change the behaviour of findmnt. > If the sub1 automount has not been triggered, > findmnt --target /mnt/test/sub1 -o target > will report "/mnt/test". > After it has been triggered, it will report "/mnt/test/sub1" > > Similarly "findmnt -t btrfs --mountpoint /mnt/test/sub1" will report > nothing if the automount hasn't been triggered, and will report full > details of /mnt/test/sub1 if it has. > >> >> Maybe findmnt isn't affected? Or maybe the change is worth making >> anyway? But it needs to be carefully considered if it breaks existing >> user interfaces. >> > I hope the change is worth making anyway, but breaking findmnt would not > be a popular move. I agree. I use findmnt, but I also use NFS mounted btrfs disks so I am keen to see this deployed. But people who don't maintain their own scripts and need a third party to change them might disagree! > This is unfortunate.... btrfs is "broken" and people/code have adjusted > to that breakage so that "fixing" it will be traumatic. > > The only way I can find to get findmnt to ignore the new entries in > /proc/self/mountinfo is to trigger a parse error such as by replacing the > " - " with " -- " > but that causes a parse error message to be generated, and will likely > break other tools. > (... or I could check if current->comm is "findmnt", and suppress the > extra entries, but that is even more horrible!!) > > A possible option is to change findmnt to explicitly ignore the new > "internal" mounts (unless some option is given) and then delay the > kernel update until that can be rolled out. That sounds good as a permanent fix for findmnt. Some sort of '--include-subvols' option. Particularly if it were possible to default it using an environment variable so a script can be written to work with both the old and the new versions of findmnt. Unfortunately it won't help any other program which does similar searches through /proc/self/mountinfo. How about creating two different files? Say, /proc/self/mountinfo and /proc/self/mountinfo.internal (better filenames may be available!). The .internal file could be just the additional internal mounts, or it could be the complete list. Or something like /proc/self/mountinfo.without-subvols and /proc/self/mountinfo.with-subvols and a sysctl setting to choose which is made visible as /proc/self/mountinfo. Graham
