Re: Enlarging w/ xfs_growfs: XFS_IOC_FSGROWFSDATA xfsctl failed: Inappropriate ioctl for device

Nick Bowler
2018-12-13T00:22:02Z

On 2018-12-12, Brian Foster <bfoster@redhat.com> wrote:
> On Tue, Dec 11, 2018 at 03:20:23PM -0500, Nick Bowler wrote:
>> Right after I write this, I realize that it's almost certainly
>> the case that architectures which _don't_ define BROKEN_X86_ALIGNMENT
>> will have matching ioctl numbers between e.g., XFS_IOC_ALLOCSP and
>> XFS_IOC_ALLOCSP_32.
[...]
> So the two struct [compat_]xfs_flock64 variants are essentially the same
> with the exception of internal alignment padding that leaves a hole
> after the first 4 bytes in the 64-bit variant. So on x86_64, these are
> essentially two different (sized) structures and ioctl commands. On some
> non-broken alignment arch, the packing may be implied and thus these
> definitions evaluate down to the same ioctl command. If so, then having
> those separate but equivalent value commands in the same switch
> statement results in a compilation error. Am I following that correctly?
>
> If so, then it does seem we need an ifdef to exlude those definitions so
> long as we follow the one huge switch statement implementation. But is
> there anything that fundamentally prevents a multiple switch statement
> implementation to avoid such syntax errors?

I think nothing fundamentally prevents this approach and it is an option.

The only point I see is that, without the configuration ifdefs,
the impact of the change is expanded; there are (presumably benign)
functional changes on more architectures, most of which I don't have
the ability to test. Making it dependent on the arch configuration
means nothing should change anywhere except on amd64 kernels that have
x32 binary support turned on.

> Note that in the end I don't care too much about whether we have an
> ifdef or not. I'm more interested in the most simple and elegant
> implementation and it just seemed that trying to dilute the existing
> ifdef may push us in the opposite direction (as opposed to something
> that for example called into the "native" ioctl and fell back to the _32
> variants on failure before returning an error). There may be technical
> complications to that or using some form of the ifdef may just end up
> being the more simple approach after all.

OK, since I have some test results I'll submit the current one for
RFC; afterwards I'll try to follow up with a less ifdeffy variant for
comparison purposes, then we can decide which version is better. :)

Cheers,
Nick