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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html