Re: [PATCH] xfsprogs: guard fsxattr definition for newer kernels

Dave Chinner
2016-02-09T21:44:40Z

On Tue, Feb 09, 2016 at 03:27:18PM -0600, Eric Sandeen wrote:
>
>
> On 2/9/16 3:10 PM, Dave Chinner wrote:
> > On Tue, Feb 09, 2016 at 01:57:09PM -0600, Eric Sandeen wrote:
> >> On 2/9/16 1:55 PM, Dave Chinner wrote:
> >>> On Tue, Feb 09, 2016 at 11:40:57AM -0600, Eric Sandeen wrote:
> >>>> After 334e580,
> >>>> fs: XFS_IOC_FS[SG]SETXATTR to FS_IOC_FS[SG]ETXATTR promotion
> >>>>
> >>>> the file include/linux/fs.h now defines struct fsxattr.
> >>>>
> >>>> It defines FS_IOC_FSGETXATTR as well, so use that to wrap
> >>>> our local definition, and skip it if the kernel is providing
> >>>> it so that we don't get multiple definitions.
> >>>>
> >>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >>>> ---
> >>>>
> >>>> Should the kernel also #define HAVE_FSXATTR to help existing
> >>>> xfsprogs-devel installations?
> >>>>
> >>>> (And what if headers are included in the other order? Should
> >>>> we try to guard on the kernel side or no?)
> >>>
> >>> I've already sent a patch to fix this - it was with the foreign
> >>> filesystem xfs_quota patch....
> >>
> >> Oh, sorry, spaced it.
> >>
> >> What do you think of the HAVE_FSXATTR definition in fs.h?
> >
> > Which fs.h? The include/linux/fs.h file does not have such
> > guards...
>
> If include/linux/fs.h defined HAVE_FSXATTR, a subsequent inclusion
> of xfs_fs.h would not redefine the structure, because it is
> guarded with that (for irix!)

That's why I changed it to check if the ioctl is defined, rather
than checking for HAVE_FSXATTR.

Looking at it, what I probably should do is something more "special"
in the platform headers, so the xfs_fs.h is kept identical across
kernel and userspace...