Created attachment 219540 [details] convert 32 bit zfs_iocparm_t to 64 bit When 32 bit applications use libzfs to access ZFS data, they always fail in zfs_open(), with 32 bit "zfs list" and "zpool list" crashing. This is because /usr/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c function zfsdev_ioctl() checks for: ---snip--- if (len != sizeof(zfs_iocparm_t)) { ---snip--- On amd64, sizeof(zfs_iocparm_t) is 24, as in /usr/src/sys/cddl/contrib/opensolaris/common/zfs/zfs_ioctl_compat.h we have: ---snip--- typedef struct zfs_iocparm { uint32_t zfs_ioctl_version; uint64_t zfs_cmd; uint64_t zfs_cmd_size; } zfs_iocparm_t; ---snip--- which has its fields aligned to 8 byte boundaries. On i386, they are aligned to 4 byte boundaries, sizeof(zfs_iocparm_t) is only 20, and every ioctl fails with EINVAL. This affects Wine as it runs many 32 bit apps. Since Windows is case-insensitive, Wine has to slowly scan the entire directory when doing file I/O on a case-sensitive filesystem, to match filenames case-insensitively. ZFS can create filesystems with casesensitivity=insensitive, allowing Wine to skip the slow scan and speeding up file I/O considerably for some apps. When developing a patch to Wine to check that casesensitivity property and use it when insensitive (https://source.winehq.org/patches/data/195726) I noticed zfs_open() always fails, which led me to this bug. The attached patch correctly converts the 32 bit zfs_iocparm_t to 64 bit, and allows at least Wine to work correctly. I haven't yet had a chance to test whether it fixes 32 bit versions of the "zpool" and "zfs" tools (which probably use more elaborate ioctls).
zfs-devel@ is disused.
With further tests, I see that 32 bit "zpool" and "zfs" commands go from crashing to fully working with this patch. I can successfully zpool create, zpool destroy, zpool list, zpool status, zfs list, zfs create, zfs get all, etc.
Is this something that should go upstream (OpenZFS) ?
(In reply to Kubilay Kocak from comment #3) That code has changed a lot in FreeBSD 13 and upstream. 32 bit "zpool list" still crashes, as per truss: 21250: openat(AT_FDCWD,"/dev/zfs",O_RDWR,00) = 5 (0x5) 21250: ioctl(3,0xc0145a04 { IORW 0x5a('Z'), 4, 20 },0xffffb338) ERR#22 'Invalid argument' where /var/log/messages has: Aug 17 02:37:31 pc kernel: len 20 vecnum: 4 sizeof (zfs_cmd_t) 4528 probably from: if (len != sizeof (zfs_iocparm_t)) { printf("len %d vecnum: %d sizeof (zfs_cmd_t) %ju\n", len, vecnum, (uintmax_t)sizeof (zfs_cmd_t)); return (EINVAL); } in sys/contrib/openzfs/module/os/freebsd/zfs/kmod_core.c That has another bug in logging, as zfs_iocparm_t and zfs_cmd_t are different structs. We should be printing sizeof(zfs_iocparm_t), not sizeof(zfs_cmd_t). This bug probably doesn't affect Linux, as it uses zfs_cmd_t directly, which has the same field sizes/alignments on 32 and 64 bit already. We wrap zfs_cmd_t in our zfs_iocparm_t, and add this bug in the process. It will take me a while to test a new patch.
This is wrong for anything other than X86; it's only i386 that has this stupid ABI quirk. PowerPC freebsd32 would break if you added that unconditionally, for example, as would MIPS if MFC'ed to stable branches (though not sure how well supported freebsd32 is there).
This is amongst bug reports that need special attention; see <https://lists.freebsd.org/archives/freebsd-fs/2023-April/002047.html>. With last year's 12.4 as the final RELEASE from stable/12, please: * is there – realistically – anything more to do on the FreeBSD side for stable/12? (In reply to Damjan Jovanovic from comment #4) Please, what's the current situation with regard to upstream?