On Thu, Apr 09, 2015 at 02:34:50PM -0700, Omar Sandoval wrote:I'm OK with your patchset, just as you mentioned, concurrently mount with rename is not such a common thing.
Here's version 2 of providing the subvolume name and ID in /proc/mounts.
It turns out that getting the name of a subvolume reliably is a bit
trickier than it would seem because of how mounting subvolumes by ID is
implemented. In particular, in that case, the dentry we get for the root
of the mount is not necessarily attached to the dentry tree, which means
that the obvious solution of just dumping the dentry does not work. The
solution I put together makes the tradeoff of churning a bit more code
in order to avoid implementing this with weird hacks.
Changes from v1 (https://lkml.org/lkml/2015/4/8/16):
- Put subvol= last in show_options
- Change commit log to remove comment about userspace having no way to
know which subvolume is mounted, as David pointed out you can use
btrfs inspect-internal rootid <mountpoint>
- Split up patch 2
- Minor coding style fixes
This still applies to v4.0-rc7. Tested manually and with the script
below (updated from v1).
Omar Sandoval (6):
Btrfs: lock superblock before remounting for rw subvol
Btrfs: remove all subvol options before mounting top-level
Btrfs: clean up error handling in mount_subvol()
Btrfs: fail on mismatched subvol and subvolid mount options
Btrfs: unify subvol= and subvolid= mounting
Btrfs: show subvol= and subvolid= in /proc/mounts
fs/btrfs/super.c | 376 ++++++++++++++++++++++++++++++++++++-------------------
fs/seq_file.c | 1 +
2 files changed, 251 insertions(+), 126 deletions(-)
Just wanted to revive this so we can hopefully come up with a solution
we agree on in time for 4.2.
Just to recap, my approach (and also Qu Wenruo's original approach) is
to convert subvolid= mounts to subvol= mounts at mount time, which makes
showing the subvolume in /proc/mounts easy. The benefit of this approach
is that looking at mount information, which is supposed to be a
lightweight operation, is simple and always works. Additionally, we'll
have the info in a convenient format in /proc/mounts in addition to
/proc/$PID/mountinfo. The only caveat is that a mount by subvolid can
fail if the mount races with a rename of the subvolume.
Qu Wenruo's second approach was to instead convert the subvolid to a
subvolume path when reading /proc/$PID/mountinfo. The benefit of this
approach is that mounts by subvolid will always succeed in the face of
concurrent renames. However, instead, getting the subvolume path in
mountinfo can now fail, and it makes what should probably be a
lightweight operation somewhat complex.
In terms of the code, I think the original approach is cleaner: the
heavy lifting is done when mounting instead of when reading a proc file.
Additionally, I don't think that the concurrent rename race will be much
of a problem in practice. I can't imagine that too many people are
actively renaming subvolumes at the same time as they are mounting them,
and even if they are, I don't think it's so surprising that it would
fail. On the other hand, reading mount info while renaming subvolumes
might be marginally more common, and personally, if that failed, I'd be
Orthogonal to that decision is the precedence of subvolid= and subvol=.
Although it's true that mount options usually have last-one-wins
behavior, I think David's argument regarding the principle of least
surprise is solid. Namely, someone's going to be unhappy with a
seemingly arbitrary decision when they don't match.
Sorry for the long-winded email! Thoughts, David, Qu?