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?
> > Should the XFS data and rt volumes be reported with different stx_vol
> > values?
>
> Maybe?
>
--
Jeff Layton <jlayton@xxxxxxxxxx>
