Re: [PATCH v2] BTRFS/NFSD: provide more unique inode number for btrfs export

From: "NeilBrown" <neilb@suse.de>
To: "Amir Goldstein" <amir73il@gmail.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
	"Christoph Hellwig" <hch@infradead.org>,
	"Chuck Lever" <chuck.lever@oracle.com>,
	"Linux NFS Mailing List" <linux-nfs@vger.kernel.org>,
	"Josef Bacik" <josef@toxicpanda.com>,
	"linux-fsdevel" <linux-fsdevel@vger.kernel.org>,
	"Theodore Tso" <tytso@mit.edu>, "Jan Kara" <jack@suse.cz>
Subject: Re: [PATCH v2] BTRFS/NFSD: provide more unique inode number for btrfs export
Date: Tue, 14 Sep 2021 08:59:46 +1000	[thread overview]
Message-ID: <163157398661.3992.2107487416802405356@noble.neil.brown.name> (raw)
In-Reply-To: <CAOQ4uxgFf5c0to7f4cT9c9JwWisYRf-kxiZS4BuyXaQV=bLbJg@mail.gmail.com>

On Mon, 13 Sep 2021, Amir Goldstein wrote:
> 
> Right, so the right fix IMO would be to provide similar semantics
> to the NFS client, like your first patch set tried to do.
> 

Like every other approach, this sounds good and sensible ...  until
you examine the details.

For NFSv3 (RFC1813) this would be a protocol violation.
Section 3.3.3 (LOOKUP) says:
  A server will not allow a LOOKUP operation to cross a mountpoint to
  the root of a different filesystem, even if the filesystem is
  exported.

The filesystem is represented by the fsid, so this implies that the fsid
of an object reported by LOOKUP must be the same as the fsid of the
directory used in the LOOKUP.

Linux NFS does allow this restriction to be bypassed with the "crossmnt"
export option.  Maybe if crossmnt were given it would be defensible to
change the fsid - if crossmnt is not given, we leave the current
behaviour.  Note that this is a hack and while it is extremely useful,
it does not produce a seemly experience.  You can get exactly the same
problems with "find" - just not as uniformly (mounting with "-o noac"
makes them uniform).

For NFSv4, we need to provide a "mounted-on" fileid for any mountpoint. 
btrfs doesn't have a mounted-on fileid that can be used.  We can fake
something that might work reasonably well - but it would be fake.  (but
then ... btrfs already provided bogus information in getdents when there
is a subvol root in the directory).

But these are relatively minor.  The bigger problem is /proc/mounts.  If
btrfs maintainers were willing to have every active subvolume appear in
/proc/mounts, then I would be happy to fiddle the NFS fsid and allow
every active NFS/btrfs subvolume to appear in /proc/mounts on the NFS
client.  But they aren't.  So I am not.

> > And I really don't see how an nfs export option would help...  Different
> > people within and organisation and using the same export might have
> > different expectations.
> 
> That's true.
> But if admin decides to export a specific btrfs mount as a non-unified
> filesystem, then NFS clients can decide whether ot not to auto-mount the
> exported subvolumes and different users on the client machine can decide
> if they want to rsync or rsync --one-file-system, just as they would with
> local btrfs.
> 
> And maybe I am wrong, but I don't see how the decision on whether to
> export a non-unified btrfs can be made a btrfs option or a nfsd global
> option, that's why I ended up with export option.

Just because a btrfs option and global nfsd option are bad, that doesn't
mean an export option must be good.  It needs to be presented and
defended on its own merits.

My current opinion (and I must admit I am feeling rather jaded about the
whole thing), is that while btrfs is a very interesting and valuable
experiment in fs design, it contains core mistakes that cannot be
incrementally fixed.  It should be marked as legacy with all current
behaviour declared as intentional and not subject to change.  This would
make way for a new "betrfs" which was designed based on all that we have
learned.  It would use the same code base, but present a more coherent
interface.  Exactly what that interface would be has yet to be decided,
but we would not be bound to maintain anything just because btrfs
supports it.

NeilBrown