Re: Argument type for FS_IOC_GETFLAGS/FS_IOC_SETFLAGS ioctls

[Dave Chinner <david@fromorbit.com>]

On Fri, Nov 29, 2013 at 09:22:05AM -0500, Theodore Ts'o wrote:
> On Fri, Nov 29, 2013 at 04:27:48PM +1100, Dave Chinner wrote:
> > > 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?
> 
> Well, need to set and get these file flags, and historically we've
> used a bitmask for this purpose.  And these aren't so much attributes
> as flags, really, i.e:

Yes, I know the history of this shitty interface. Thank you for
explaining to everyone else how we got into this mess....

> > 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....
> 
> You mean something where we take a char * and a length? 

I don't really care how a bitmap gets specified - I'd even argue
that we shouldn't use a bitmap but a special attribute namespace.

> We could, but
> (a) it would be incompatible with existing FS_IOC_[GS]ETFLAGS, and (b)
> it's not clear the complexity is worth it.

Compatibility is not an issue here - if we have to change existing
applications to make use of more flags, then we may as well fix all
the problems the ioctl interface entails at the same time.

> P.S. One of the reasons why there's a certain amount of wastage with
> this ioctl is that some of the bit fields were originally used as the
> file system level encouding for the file flags in ext[234].  This
> could be argued to be bad design, but we didn't ask for this
> ext[234]-specific ioctl to get hijcaked for use by other file systems,
> either.

Hijacked?  Ted, I think you need to tone down the rhetoric a bit.

Filesystems wanted to be compatible with the lsattr/ chattr tools
that shipped by default on all systems via e2fsprogs rather than
re-inventing their own wheel. Yes, it has made a mess of the
interface, but that's because nobody has stepped up to say "no,
don't do that, those are ext2 on-disk definitions" when it was
needed.

Let's take the opportunity to make a clean break so we don't have to
care what any specific filesystem does or stores on disk. With an
attribute namespace it's easy to add filesystem specific flags and
it's trivial to make them generic without breaking backwards
compatibility....

> If we do create the 64-bit version of this ioctl, we won't
> have this problem with the upper 32-bits --- and indeed it would be
> preferable if other file-system specific flags for btrfs, f2fs, et.al,
> got allocated from the MSB end of the 64-bit ioctl.

Ugh, that'll just screw it up even more. And if we put the ~10 XFS
flags in there that aren't supported by FS_IOC_GETFLAGS, and all the
others from other filesystems, we'll be out of space in a couple of
kernel releases...

> Or we could design an entirely new ioctl that uses a completely new
> bitmask allocation scheme, or even a plan9 style set of ascii messages
> which are passed back and forth between userspace and the kernel ---
> or even insist that btrfs was wrong, that they shouldn't have been
> allocating flags out of this legacy ioctl, but should have been using
> the existing xattr interface with a new namespace that was either
> btrfs specific or a new vfsflag namspace.

Yup, you've pretty much stated all the reasons why an expanded
bitmap is a bad idea, and why adding an attribute flag namespace is
probably a better way to expose all of these generic and
filesystem-specific flags in a generic manner.

And FWIW, an attribute based approach means you don't need to get
the flags before setting them to ensure you don't reset flags you
don't care about, so it's safer from that perspective, too...

> The options and opportunities for bike shedding are endless.   :-)

I'm not interested in bike shedding - let's just solve the problem
once and for all....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com