Re: Argument type for FS_IOC_GETFLAGS/FS_IOC_SETFLAGS ioctls

[Dave Chinner <david@fromorbit.com>]

On Thu, Nov 28, 2013 at 11:54:12PM -0500, Theodore Ts'o wrote:
> On Thu, Nov 28, 2013 at 05:53:10PM -0700, Andreas Dilger wrote:
> > 
> > Note that there are already FS_IOC32_{GET,SET}{VERSION,FLAGS} ioctl
> > definitions for  that date back to 2.6.19 or so that correctly use "int"
> > for the size argument.  Those are unfortunately(?) under CONFIG_COMPAT
> > instead of just being inline with the normal ioctl definitions, so I'm
> > not sure if those are available on the systems in question.  CONFIG_COMPAT
> > was the default for RHEL5 and SLES10 kernels, but the compat ioctl code
> > was only in ext4 and not ext3 in RHEL5 at least.
> 
> This were introduced to support 32-bit userspace programs (where
> sizeof(long) == sizeof(int) == 4) with a 64-bit kernel.  The intent
> was not to "fix" the ioctl, so much as it was to enable 32-bit
> programs.  The compat code is in ext3 as well, although it uses
> EXT3_IOC32_[SG]ETFLAGS.
> 
> > I suspect those have already been around long enough for chattr/lsattr to
> > start trying to use them first, and fall back to using the "broken" IOC
> > numbers if they fail.
> 
> Nope, chattr/lsattr does not try using them first, because the intent
> wasn't to "fix" the "broken" IOC numbers.  If you look in the sources,
> e2fsprogs is only using EXT2_IOC_[SG]ETFLAGS.  (These ioctls were
> originally only defined for ext2, and intended for use only for
> ext[23].  They got adopted by other file systems, and then they get
> moved into linux/fs..)
> 
> > > P.S.  If we were going to create a new ioctl, what I'd suggest is that
> > > the new ioctl explicitly use a 64-bit type, instead of using "long" or
> > > "int", to avoid the compat ioctl hair to allow 64-bit kernels to
> > > support 32-bit userspace programs.
> > 
> > Unfortunately, I don't think it would be possible to just use:
> > 
> > #define FS_IOC_GETFLAGS_NEW		_IOR('f', 1, __u64)
> > 
> > since this would conflict with the existing "long" definition on 64-bit
> > platforms that actually expect an "int" argument.  It definitely would
> > be desirable to get a 64-bit attributes API, since we are very close to
> > running out of space in the 32-bit flags definitions.
> 
> Sure, I was thinking about doing something like this instead:
> 
> #define FS_IOC_GETFLAGS_WIDE		_IOR('f', 32, __u64)
> #define FS_IOC_SETFLAGS_WIDE		_IOR('f', 32, __u64)
> 
> And I agree that a good reason to do this is to get 64 bits worth of
> attributes....

Why create a new ioctl for getting these generic attributes out of
the kernel? Isn't that the problem xstat() is supposed to solve?

And if it's truly generic stuff, then a syscall pair with enough
bitmap space for the forseeable future is more appropriate than a
new ioctl....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com