RFC, 32-bit compat handlers for EXT4_IOC_GROUP_ADD

RFC, 32-bit compat handlers for EXT4_IOC_GROUP_ADD

[Eric Sandeen]

Unfortunately the argument to EXT4_IOC_GROUP_ADD contains padding on
64-bit arches, not present on 32-bit arches:

struct ext4_new_group_input {
	__u32            group;                /*     0     4 */

	/* XXX 4 bytes hole, try to pack */

	__u64            block_bitmap;         /*     8     8 */
	__u64            inode_bitmap;         /*    16     8 */
	__u64            inode_table;          /*    24     8 */
	__u32            blocks_count;         /*    32     4 */
	__u16            reserved_blocks;      /*    36     2 */
	__u16            unused;               /*    38     2 */

	/* size: 40, cachelines: 1 */
	/* sum members: 36, holes: 1, sum holes: 4 */
	/* last cacheline: 40 bytes */
};

due to the new 64-bit types, which want to align on 64-bit boundaries.
So, when called from an ia32 executable on a 64-bit kernel, the size of
the arg is "wrong" and the ioctl fails:

ioctl32(resize2fs:3595): Unknown cmd fd(99) cmd(80186608){t:'f';sz:24}
arg(ff8ce8a0) on /mnt/tmp

The simple/straightforward fix is to add a compat handler, which I've
written:

 ext4.h  |   12 ++++++++++++
 ioctl.c |   25 ++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

this bloats the kernel just a bit; we have to add a compat arg structure
which is packed, then copy in the elements, and dispatch it to the
native ioctl handler.

It's too bad this wasn't caught sooner, but I wonder if maybe it's still
not too late; can we change the structure in the kernel and have newer
e2fsprogs try both the old & new?  The new interface would add 32 bits
of padding to the structure; this would leave it unchanged on 64-bit
boxes so everything would continue to work.  Newer e2fsprogs would try
both, so still work on older kernels.  32-bit compat would have a simple
handler, *but* this *would* break resize of ext4 on native 32-bit
machines with older e2fsprogs (the kernel would have a padded struct;
older userspace would not, and the ioctl would fail).

How far out of "dev" are we?  I'm leaning towards saying "oh well, would
have been nicer the other way" but going ahead and just putting the
compat handler into the kernel.

Thoughts?

-Eric

Re: RFC, 32-bit compat handlers for EXT4_IOC_GROUP_ADD

[Andreas Dilger]

On Dec 04, 2008  16:37 -0600, Eric Sandeen wrote:
> It's too bad this wasn't caught sooner, but I wonder if maybe it's still
> not too late; can we change the structure in the kernel and have newer
> e2fsprogs try both the old & new?  The new interface would add 32 bits
> of padding to the structure; this would leave it unchanged on 64-bit
> boxes so everything would continue to work.  Newer e2fsprogs would try
> both, so still work on older kernels.  32-bit compat would have a simple
> handler, *but* this *would* break resize of ext4 on native 32-bit
> machines with older e2fsprogs (the kernel would have a padded struct;
> older userspace would not, and the ioctl would fail).
> 
> How far out of "dev" are we?  I'm leaning towards saying "oh well, would
> have been nicer the other way" but going ahead and just putting the
> compat handler into the kernel.

I would be OK with changing to the "proper" struct layout.  Not being able
to resize with an older e2fsprogs + newer kernel isn't going to cause any
serious problems (unlike e.g. not being able to mount or e2fsck "/").

If we are seriously worried about compatibility, we could add the compat
handler for 32-bit kernels (should have a different IOC number anyways
because of the struct size) and add some arbitrary check like:

#ifdef LINUX_KERNEL_VERSION > KERNEL_VERSION(2,6,40)
#warning remove this old compat code
#endif

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: RFC, 32-bit compat handlers for EXT4_IOC_GROUP_ADD

[Theodore Tso]

On Fri, Dec 05, 2008 at 06:02:04PM -0700, Andreas Dilger wrote:
> > How far out of "dev" are we?  I'm leaning towards saying "oh well, would
> > have been nicer the other way" but going ahead and just putting the
> > compat handler into the kernel.
> 
> I would be OK with changing to the "proper" struct layout.  Not being able
> to resize with an older e2fsprogs + newer kernel isn't going to cause any
> serious problems (unlike e.g. not being able to mount or e2fsck "/").
> 
> If we are seriously worried about compatibility, we could add the compat
> handler for 32-bit kernels (should have a different IOC number anyways
> because of the struct size) and add some arbitrary check like:
> 
> #ifdef LINUX_KERNEL_VERSION > KERNEL_VERSION(2,6,40)
> #warning remove this old compat code
> #endif

Given that a bunch of distro's have shipped e2fsprogs 1.41.x which we
advertised as being ext4 compatibility, I think we need to keep the
compatibility code.  If we want to add the complexity for the 32-bit
side, with a 2-3 year timeout, that seems like a reasonable
compromise.

							- Ted

Re: RFC, 32-bit compat handlers for EXT4_IOC_GROUP_ADD

[Eric Sandeen]

Theodore Tso wrote:
> On Fri, Dec 05, 2008 at 06:02:04PM -0700, Andreas Dilger wrote:
>>> How far out of "dev" are we?  I'm leaning towards saying "oh well, would
>>> have been nicer the other way" but going ahead and just putting the
>>> compat handler into the kernel.
>> I would be OK with changing to the "proper" struct layout.  Not being able
>> to resize with an older e2fsprogs + newer kernel isn't going to cause any
>> serious problems (unlike e.g. not being able to mount or e2fsck "/").
>>
>> If we are seriously worried about compatibility, we could add the compat
>> handler for 32-bit kernels (should have a different IOC number anyways
>> because of the struct size) and add some arbitrary check like:
>>
>> #ifdef LINUX_KERNEL_VERSION > KERNEL_VERSION(2,6,40)
>> #warning remove this old compat code
>> #endif
> 
> Given that a bunch of distro's have shipped e2fsprogs 1.41.x which we
> advertised as being ext4 compatibility, I think we need to keep the
> compatibility code.  If we want to add the complexity for the 32-bit
> side, with a 2-3 year timeout, that seems like a reasonable
> compromise.
> 
> 							- Ted

I tend to agree, unfortunately... I'll send the current compat patch,
then, and when/if I get motivated, add another cleaner interface+number
I guess...

-Eric