On Sat, Mar 09, 2024 at 06:46:54AM -0500, Jeff Layton wrote: > On Fri, 2024-03-08 at 12:13 -0500, Kent Overstreet wrote: > > On Fri, Mar 08, 2024 at 08:56:33AM -0800, Darrick J. Wong wrote: > > > On Fri, Mar 08, 2024 at 11:48:31AM -0500, Kent Overstreet wrote: > > > > It's a new feature, not a bugfix, this should never get backported. And > > > > I the bcachefs maintainer wrote the patch, and I'm submitting it to the > > > > VFS maintainer, so if it's fine with him it's fine with me. > > > > > > But then how am I supposed to bikeshed the structure of the V2 patchset > > > by immediately asking you to recombine the patches and spit out a V3? > > > > > > </sarcasm> > > > > > > But, seriously, can you update the manpage too? > > > > yeah, where's that at? > > > > https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git > > > > > Is stx_subvol a u64 > > > cookie where userspace mustn't try to read anything into its contents? > > > Just like st_ino and st_dev are (supposed) to be? > > > > Actually, that's up for debate. I'm considering having the readdir() > > equivalent for walking subvolumes return subvolume IDs, and then there'd > > be a separate call to open by ID. > > > > Al's idea was to return open fds to child subvolumes, then userspace can > > get the path from /proc; that's also a possibility. > > > > The key thing is that with subvolumes it's actually possible to do an > > open_by_id() call with correct security checks on pathwalking - because > > we don't have hardlinks so there's no ambiguity. > > > > Or we might do it getdents() style and return the path directly. > > > > But I think userspace is going to want to work with the volume > > identifiers directly, which is partly why I'm considering why other > > options might be cleaner. > > > > Another thing to consider: where we're going with this is giving > > userspace a good efficient interrface for recursive tree traversal of > > subvolumes, but it might not be a bad idea to do that for mountpoints as > > well - similar problems, similar scalability issues that we might want > > to solve eventually. > > > > All of that's fine, but Darrick's question is about whether we should > ensure that these IDs are considered _opaque_. I think they should be. > > We don't want to anyone to fall into the trap of trying to convey extra > info to userland about the volumes via this value. It should only be > good for uniquely identifying the volume. > > We'll also need to document the scope of uniqueness. I assume we'll want > to define this as only being unique within a single filesystem? IOW, if > I have 2 bcachefs filesystems that are on independent devices, these > values may collide? Someone wanting to uniquely identify a subvolume on > a system will need to check both the st_dev and the st_vol, correct? they're small integers, not UUIDs, so yes
