James Bottomley <> writes:
> On Tue, 2014-01-21 at 14:34 +0200, Michael S. Tsirkin wrote:
>> > > What does this add to what I wrote above?
>> >
>> > I'd like to be able to drop the __attribute__((packed)), and just make
>> > that the default.
>>
>> To me __attribute__((packed)) seems helpful, and harmless.
>> If someone wants to pack everything in driver and bloat up
>> code unnecessarily, one always has the option.
>
> To me as well. As a driver writer, I start to worry when I *don't* see
> an __attribute__((packed)) on any data structure shared between the
> driver and the device (whether or not it's a virtual device).
> Conversely, relying on the C compiler to do standard padding is
> dangerous (and error prone). For instance, the padding requirements for
> 64 bit words on x86 is 32 bits, but it's 64 bits on every other
> architecture, so
Agreed, but just to be clear:
- We always use explicit length types in the spec.
- We have only one case where we have padding, and it's explicit
(virtio_blk's "unused" field).
- CCW has two places (and virtio_balloon one case) where the struct is
unnaturally *short*: these must be explicitly called out IMHO.
I'm happy with Cornelia's explicit verbiage around the different types,
though I can't say it's confused anyone yet.
BTW, there are several suspect uses of "u16" in the spec, in particular:
CCW_CMD_SET_VIRTIO_REV:
struct virtio_rev_info {
__u16 revision;
__u16 length;
__u8 data[];
};
(I'll address the VIRTIO_NET_F_MQ ones in a separate patch).
Thanks,
Rusty.
