> Er... "fsx_fileattr" is the struct that the system call uses?
>
> That's a little confusing considering that xfs already has a
> xfs_fill_fsxattr function that actually fills a struct fileattr.
> That could be renamed xfs_fill_fileattr.
>
> I dunno. There's a part of me that would really rather that the
> file_getattr and file_setattr syscalls operate on a struct file_attr.
Agreed, I'm pretty sure I suggested this during an earlier review. Fits
in line with struct mount_attr and others. Fwiw, struct fileattr (the
kernel internal thing) should've really been struct file_kattr or struct
kernel_file_attr. This is a common pattern now:
struct mount_attr vs struct mount_kattr
struct clone_args vs struct kernel_clone_kargs
etc.
>
> More whining/bikeshedding to come.
>
> <snip stuff that looks ok to me>
>
> <<well, I still dislike the CLASS(fd, fd)(fd) syntax...>>
Noted, and duly ignored...
>
> > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> > index 0098b0ce8ccb..0784f2033ba4 100644
> > --- a/include/uapi/linux/fs.h
> > +++ b/include/uapi/linux/fs.h
> > @@ -148,6 +148,24 @@ struct fsxattr {
> > unsigned char fsx_pad[8];
> > };
> >
> > +/*
> > + * Variable size structure for file_[sg]et_attr().
> > + *
> > + * Note. This is alternative to the structure 'struct fileattr'/'struct fsxattr'.
> > + * As this structure is passed to/from userspace with its size, this can
> > + * be versioned based on the size.
> > + */
> > +struct fsx_fileattr {
> > + __u32 fsx_xflags; /* xflags field value (get/set) */
>
> Should this to be __u64 from the start? Seeing as (a) this struct is
Agreed. I changed that.
