Re: [PATCH] vfs: report an inode version in statx for IS_I_VERSION inodes

From: Jeff Layton <jlayton@kernel.org>
To: Dave Chinner <david@fromorbit.com>
Cc: viro@zeniv.linux.org.uk, linux-api@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org,
	David Howells <dhowells@redhat.com>,
	Frank Filz <ffilzlnx@mindspring.com>
Subject: Re: [PATCH] vfs: report an inode version in statx for IS_I_VERSION inodes
Date: Wed, 24 Aug 2022 06:17:22 -0400	[thread overview]
Message-ID: <7901f60f7ea67d757caa5ba26588f01f308caaac.camel@kernel.org> (raw)
In-Reply-To: <20220823215333.GC3144495@dread.disaster.area>

On Wed, 2022-08-24 at 07:53 +1000, Dave Chinner wrote:
> On Fri, Aug 19, 2022 at 07:56:41AM -0400, Jeff Layton wrote:
> > From: Jeff Layton <jlayton@redhat.com>
> > 
> > The NFS server and IMA both rely heavily on the i_version counter, but
> > it's largely invisible to userland, which makes it difficult to test its
> > behavior. This value would also be of use to userland NFS servers, and
> > other applications that want a reliable way to know if there was an
> > explicit change to an inode since they last checked.
> > 
> > Claim one of the spare fields in struct statx to hold a 64-bit inode
> > version attribute. This value must change with any explicit, observeable
> > metadata or data change. Note that atime updates are excluded from this,
> > unless it is due to an explicit change via utimes or similar mechanism.
> > 
> > When statx requests this attribute on an IS_I_VERSION inode, do an
> > inode_query_iversion and fill the result in the field. Also, update the
> > test-statx.c program to display the inode version and the mountid.
> > 
> > Cc: David Howells <dhowells@redhat.com>
> > Cc: Frank Filz <ffilzlnx@mindspring.com>
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> 
> NAK.
> 
> THere's no definition of what consitutes an "inode change" and this
> exposes internal filesystem implementation details (i.e. on disk
> format behaviour) directly to userspace. That means when the
> internal filesystem behaviour changes, userspace applications will
> see changes in stat->ino_version changes and potentially break them.
> 
> We *need a documented specification* for the behaviour we are exposing to
> userspace here, and then individual filesystems needs to opt into
> providing this information as they are modified to conform to the
> behaviour we are exposing directly to userspsace.
> 
> Jeff - can you please stop posting iversion patches to different
> subsystems as individual, unrelated patchsets and start posting all
> the changes - statx, ext4, xfs, man pages, etc as a single patchset
> so the discussion can be centralised in one place and not spread
> over half a dozen disconnected threads?
> 

Sure. Give me a few days and I'll post a more coherent set of patches.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>