Re: [v5 5/5] Adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support for ext4

[Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org>]

On Mon, Oct 27, 2014 at 09:09:19AM +0800, Li Xi wrote:
> On Mon, Oct 27, 2014 at 5:56 AM, Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> wrote:
> > On Sun, Oct 26, 2014 at 01:22:53PM +0800, Li Xi wrote:
> >> This patch adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR ioctl interface
> >> support for ext4. The interface is kept consistent with
> >> XFS_IOC_FSGETXATTR/XFS_IOC_FSGETXATTR.
> >
> > What you haven't mentioned is that you also changed the fsxattr
> > interface structure to add functionality and new behaviours that
> > isn't supported by XFS or existing applications that use the
> > interface.
> >
> > There is no need to modify the interface *at all* for ext4 to use
> > it. Fields that ext4 does not use can be zeroed on getxattr, and
> > ignored on setxattr - you do not need to add new fields to say what
> > fields are valid.
> Sorry, I don't want to change the interfaces either. But, the problem
> is that zero might be valid value for some fields. How can we
> distinguish an unsupported attribute and an attribute whose value
> is zero?

You don't. Userspace has no concept of what parts of the struct
fsxattr are valid or not, nor what are valid values the filesystem
will accept or reject.

> It is common case the only part of the fields are supported.
> So, for example, if we don't have valid flags, how can use space
> application tell kernel which attributes should be skipped when it
> tries to set only a part of atrributes?

It *doesn't*. Userspace requires the kernel to initialise the struct
fsxattr before it tries to modify anything, just like
fcntl(F_[GS]ETFL) and other similar "file flag change" syscall APIs.

IOWs, you have to initialise the struct fsxattr by calling
FS_IOC_FSGETXATTR before you call FS_IOC_FSSETXATTR. The
intialisation sets all the fields to the current (correct) values,
and hence when the set call is made all the fields then have the
same/correct values in them except for what the application changed.

E.g. this code from xfs_quota to clear the project ID on a given
file or directory:

	if ((fd = open(path, O_RDONLY|O_NOCTTY)) == -1) {
		// error handling ....
        } else if (xfsctl(path, fd, XFS_IOC_FSGETXATTR, &fsx) < 0) {
		// error handling ....
        }

        fsx.fsx_projid = 0;
        fsx.fsx_xflags &= ~XFS_XFLAG_PROJINHERIT;
        if (xfsctl(path, fd, XFS_IOC_FSSETXATTR, &fsx) < 0) {
		// error handling ....
        }
        close(fd);

This ensures that *only* the project ID and the specific project ID
inheritance flag is cleared, and none of the other inode flags or
state are modified....

Cheers,

Dave.
-- 
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org