On Nov 28, 2013, at 10:27 PM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> 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..)
Sorry, what I meant was "the compat ioctls been around long enough that it
might make sense to modify lsattr/chattr to _start_ trying to use them in
place of the old semi-broken IOC numbers". That might make sense regardless
of whether we add a separate 64-bit IOC number, since at least the definition
won't be broken. In a few years, the old "long" IOC number could be deprecated
and FS_IOC_{GET,SET}FLAGS could be assigned to the new number so that new apps
compile to use the new number.
>> 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...
I'm definitely in favour of the xstat() API, and have been for years. It's
just never managed to get accepted into the kernel despite how many times it's
been submitted. Every time it appears there is one group of people that say
"it should also do X" and another group that say "it does too much, take Y out".
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail
