Received-SPF: pass (google.com: domain of linux-kernel+bounces-97874-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Sat, 9 Mar 2024 07:15:50 -0500
From: Kent Overstreet <kent.overstreet@linux.dev>
To: Jeff Layton <jlayton@kernel.org>
Cc: "Darrick J. Wong" <djwong@kernel.org>, Neal Gompa <neal@gompa.dev>, 
	linux-fsdevel@vger.kernel.org, linux-bcachefs@vger.kernel.org, linux-btrfs@vger.kernel.org, 
	linux-kernel@vger.kernel.org, Josef Bacik <josef@toxicpanda.com>, 
	Miklos Szeredi <mszeredi@redhat.com>, Christian Brauner <brauner@kernel.org>, 
	David Howells <dhowells@redhat.com>
Subject: Re: [PATCH v2] statx: stx_subvol
Message-ID: <yi6fha4mz325222zfud5mlxypvl6clkkh7ko3eoyofiz3oltf2@5mhb6jqmiqtb>
References: <20240308022914.196982-1-kent.overstreet@linux.dev>
 <CAEg-Je96OKs_LOXorNVj1a1=e+1f=-gw34v4VWNOmfKXc6PLSQ@mail.gmail.com>
 <i2oeask3rxxd5w4k7ikky6zddnr2qgflrmu52i7ah6n4e7va26@2qmghvmb732p>
 <CAEg-Je_URgYd6VJL5Pd=YDGQM=0T5tspfnTvgVTMG-Ec1fTt6g@mail.gmail.com>
 <2uk6u4w7dp4fnd3mrpoqybkiojgibjodgatrordacejlsxxmxz@wg5zymrst2td>
 <20240308165633.GO6184@frogsfrogsfrogs>
 <6czkpcm4gxcjik3drcy6eys6lannfk55oowdesem2qr3gfgobw@lblo3vzck43e>
 <4517677900bd6a29f4763abe868ab953b477772b.camel@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4517677900bd6a29f4763abe868ab953b477772b.camel@kernel.org>

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