RFC, 32-bit compat handlers for EXT4_IOC_GROUP_ADD
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
Quoted 10 lines · Eric Sandeen
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).
Quoted 3 lines
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.
Quoted 8 lines · Andreas Dilger
Quoted 3 lines
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 "/").
Quoted 7 lines
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
Quoted 16 lines · Theodore Tso
Quoted 7 lines · Andreas Dilger
Quoted 3 lines
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 "/").
Quoted 7 lines
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
Quoted 7 lines
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