Re: [RFC PATCH v7 9/9] vfs: expose STATX_VERSION to userland

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2022-10-18 at 06:35 -0400, Jeff Layton wrote:
> On Tue, 2022-10-18 at 09:14 +1100, Dave Chinner wrote:
> > On Mon, Oct 17, 2022 at 06:57:09AM -0400, Jeff Layton wrote:
> > > From: Jeff Layton <jlayton@xxxxxxxxxx>
> > > 
> > > Claim one of the spare fields in struct statx to hold a 64-bit inode
> > > version attribute. When userland requests STATX_VERSION, copy the
> > > value from the kstat struct there, and stop masking off
> > > STATX_ATTR_VERSION_MONOTONIC.
> > 
> > Can we please make the name more sepcific than "version"? It's way
> > too generic and - we already have userspace facing "version" fields
> > for inodes that refer to the on-disk format version exposed in
> > various UAPIs. It's common for UAPI structures used for file
> > operations to have a "version" field that refers to the *UAPI
> > structure version* rather than file metadata or data being retrieved
> > from the file in question.
> > 
> > The need for an explanatory comment like this:
> > 
> > > +	__u64	stx_version; /* Inode change attribute */
> > 
> > demonstrates it is badly named. If you want it known as an inode
> > change attribute, then don't name the variable "version". In
> > reality, it really needs to be an opaque cookie, not something
> > applications need to decode directly to make sense of.
> > 
> 
> Fair enough. I started with this being named stx_change_attr and other
> people objected. I then changed to stx_ino_version, but the "_ino"
> seemed redundant.
> 
> I'm open to suggestions here. Naming things like this is hard.
> 

How about:

    STATX_CHANGE / statx->stx_change / STATX_ATTR_CHANGE_MONOTONIC

?
-- 
Jeff Layton <jlayton@xxxxxxxxxx>




[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux