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

From: Dave Chinner <david@fromorbit.com>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH] xfsprogs: guard fsxattr definition for newer kernels
Date: Wed, 10 Feb 2016 09:37:13 +1100	[thread overview]
Message-ID: <20160209223713.GI14668@dastard> (raw)
In-Reply-To: <56BA5E0E.7030209@sandeen.net>

On Tue, Feb 09, 2016 at 03:45:50PM -0600, Eric Sandeen wrote:
> 
> 
> On 2/9/16 3:44 PM, Dave Chinner wrote:
> > 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.
> 
> Right, but I'm talking about protecting older, existing versions of
> xfsprogs headers which use HAVE_FSXATTR as the guard.

Nothing we can really do about that. There's no way we can get
random unused defines into general linux uapi header files.

If a distro updates their kernel to 4.5 and hence introduces this
general definition, then they also be upgrading xfsprogs to match.
If a user has upgraded their kernel and then tries to build xfsprogs
from source, then they are also going to need to update xfsprogs...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs