Re: [PATCH] xfs: fix i_version handling in xfs

[Dave Chinner <david@fromorbit.com>]

On Thu, Aug 18, 2022 at 07:03:42AM -0400, Jeff Layton wrote:
> Dave, you keep talking about the xfs i_version counter as if there are
> other applications already relying on its behavior, but I don't see how
> that can be. There is no way for userland applications to fetch the
> counter currently.

You miss the point entirely: the behaviour is defined by the on-disk
format (the di_changecount filed), not the applications that are
using the kernel internal iversion API.

Just because there are no in-kernel users of the di_changecount
field in the XFS inode, it does not mean that it doesn't get used by
applications. Forensic analysis tools that walk filesystem images.

Did you not notice that xfs_trans_log_inode() forces an iversion
update if the inode core is marked for update:

	inode_maybe_inc_iversion(inode, flags & XFS_ILOG_CORE))
					^^^^^^^^^^^^^^^^^^^^^^
					this?

So every modification to the inode core (which almost all inode
modifications will do, except for pure timestamp updates) will bump
the iversion regardless of whether it was queried or not.

I use this information all the time for forensic analysis of broken
filesystem images. There are forensic tools that use expose this
information from filesystem images (e.g. xfs_db) so that we can use
it for forensic analysis.

See the problem? On-disk format di_changecount != NFS change
attribute. We can implement the NFS change attribute with the
existing di_changecount field, but if you want to constrain the
definition of how the NFS change attribute is calculated, we can't
necessarily implement that in di_changecount without changing the
on-disk format definition.

And, yes, this has implications for iversion being exposed to
userspace via statx(), as I've mentioned in reply to the v2 patch
you've posted. iversion is persistent information - you can't just
redefine it's behaviour without some fairly serious knock-on
effects for the subsystems that provide the persistent storage...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com