Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 24 Aug 2022 07:53:33 +1000
From:   Dave Chinner <david@fromorbit.com>
To:     Jeff Layton <jlayton@kernel.org>
Cc:     viro@zeniv.linux.org.uk, linux-api@vger.kernel.org,
        linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org,
        Jeff Layton <jlayton@redhat.com>,
        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
Message-ID: <20220823215333.GC3144495@dread.disaster.area>
References: <20220819115641.14744-1-jlayton@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220819115641.14744-1-jlayton@kernel.org>
Precedence: bulk

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?

-Dave.
-- 
Dave Chinner
david@fromorbit.com