Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Christian Brauner <brauner@kernel.org>]

On Tue, Dec 12, 2023 at 08:32:55AM +0200, Amir Goldstein wrote:
> On Tue, Dec 12, 2023 at 7:53 AM Dave Chinner <david@fromorbit.com> wrote:
> >
> > On Tue, Dec 12, 2023 at 11:59:51AM +1100, NeilBrown wrote:
> > > On Tue, 12 Dec 2023, Kent Overstreet wrote:
> > > > On Tue, Dec 12, 2023 at 10:53:07AM +1100, NeilBrown wrote:
> > > > > On Tue, 12 Dec 2023, Kent Overstreet wrote:
> > > > > > On Tue, Dec 12, 2023 at 09:43:27AM +1100, NeilBrown wrote:
> > > > > > > On Sat, 09 Dec 2023, Kent Overstreet wrote:
> > > > > > Thoughts?
> > > > > >
> > > > >
> > > > > I'm completely in favour of exporting the (full) filehandle through
> > > > > statx. (If the application asked for the filehandle, it will expect a
> > > > > larger structure to be returned.  We don't need to use the currently
> > > > > reserved space).
> > > > >
> > > > > I'm completely in favour of updating user-space tools to use the
> > > > > filehandle to check if two handles are for the same file.
> > > > >
> > > > > I'm not in favour of any filesystem depending on this for correct
> > > > > functionality today.  As long as the filesystem isn't so large that
> > > > > inum+volnum simply cannot fit in 64 bits, we should make a reasonable
> > > > > effort to present them both in 64 bits.  Depending on the filehandle is a
> > > > > good plan for long term growth, not for basic functionality today.
> > > >
> > > > My standing policy in these situations is that I'll do the stopgap/hacky
> > > > measure... but not before doing actual, real work on the longterm
> > > > solution :)
> > >
> > > Eminently sensible.
> > >
> > > >
> > > > So if we're all in favor of statx as the real long term solution, how
> > > > about we see how far we get with that?
> > > >
> > >
> > > I suggest:
> > >
> > >  STATX_ATTR_INUM_NOT_UNIQUE - it is possible that two files have the
> > >                               same inode number

This is just ugly with questionable value. A constant reminder of how
broken this is. Exposing the subvolume id also makes this somewhat redundant.

> > >
> > >
> > >  __u64 stx_vol     Volume identifier.  Two files with same stx_vol and
> > >                    stx_ino MUST be the same.  Exact meaning of volumes
> > >                    is filesys-specific

Exposing this makes sense. I've mentioned that I'd be open to exporting
the subvolume id in statx before even though I know some people don't
like that. I still stand by that. Especially now that we have btrfs and
bcachefs that both have the concept of a subvolume id it makes sense to
put it into statx().

> > >
> > >  STATX_VOL         Want stx_vol
> > >
> > >   __u8 stx_handle_len  Length of stx_handle if present
> > >   __u8 stx_handle[128] Unique stable identifier for this file.  Will
> > >                        NEVER be reused for a different file.
> > >                        This appears AFTER __statx_pad2, beyond
> > >                        the current 'struct statx'.
> > >  STATX_HANDLE      Want stx_handle_len and stx_handle. Buffer for
> > >                    receiving statx info has at least
> > >                    sizeof(struct statx)+128 bytes.

No, we're should not be reinventing another opaque handle. And even if
it really doesn't belong into statx().

> >
> > Hmmm.
> >
> > Doesn't anyone else see or hear the elephant trumpeting loudly in
> > the middle of the room?
> >
> > I mean, we already have name_to_handle_at() for userspace to get a
> > unique, opaque, filesystem defined file handle for any given file.

I agree.

> > It's the same filehandle that filesystems hand to the nfsd so nfs
> > clients can uniquely identify the file they are asking the nfsd to
> > operate on.
> >
> > The contents of these filehandles is entirely defined by the file
> > system and completely opaque to the user. The only thing that
> > parses the internal contents of the handle is the filesystem itself.
> > Therefore, as long as the fs encodes the information it needs into the
> > handle to determine what subvol/snapshot the inode belongs to when
> > the handle is passed back to it (e.g. from open_by_handle_at()) then
> > nothing else needs to care how it is encoded.
> >
> > So can someone please explain to me why we need to try to re-invent
> > a generic filehandle concept in statx when we already have a
> > have working and widely supported user API that provides exactly
> > this functionality?
> 
> Yeh.
> 
> Not to mention that since commit 64343119d7b8 ("exportfs: support encoding
> non-decodeable file handles by default"), exporting file handles as strong
> object identifiers is not limited to filesystems that support NFS export.
> 
> All fs have a default implementation of encode_fh() by encoding a file id
> of type FILEID_INO64_GEN from { i_ino, i_generation } and any fs can
> define its own encode_fh() operation (e.g. to include subvol id) even without
> implementing the decode fh operations needed for NFS export.

I also would like both btrfs and bcachefs to agree.