Re: [virtio-comment] [PATCH 1/1] ccw: ccw payload description

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Jan 30, 2014 at 10:22:16AM +1030, Rusty Russell wrote:
> Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> > On Tue, 21 Jan 2014 10:37:26 +1030
> > Rusty Russell <rusty@au1.ibm.com> wrote:
> >
> >> Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> >> > On Mon, 20 Jan 2014 11:17:17 +0200
> >> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >> >
> >> >> On Mon, Jan 20, 2014 at 02:28:00PM +1030, Rusty Russell wrote:
> >> >> > We'll just have to live with the lack of padding, and let's try not to
> >> >> > add any more packed-requiring structures.  If we deprecate
> >> >> > virtio-balloon, the rest will be contained in CCW.
> >> >> > 
> >> >> > Cheers,
> >> >> > Rusty.
> >> >> 
> >> >> For CCW, how about my idea of adding optional padding explicitly?
> >> >> In the compatibility section, we can explicitly specify the
> >> >> the length.
> >> >> Drivers can use offsetof(padding) and avoid packed attribute.
> >> >
> >> > Since Rusty does not like the tables, this looks like a good idea.
> >> 
> >> Surely making the padding optional makes it more complex, for no real
> >> gain?  It's not that much simpler in practice, is it?
> >> 
> >> This patch just removes the __attribute__((packed)) where it's useless,
> >> and notes explicitly where it's required:
> >
> > (...)
> >
> > Do we want to go the route of this patch now, together with my
> > clarifications in
> > https://lists.oasis-open.org/archives/virtio-comment/201401/msg00034.html ?
> 
> It's not quite a trivial change, and I know there's some debate.  So
> I've created an issue for it (VIRTIO-56), and we can discuss next
> meeting.
> 
> Thanks,
> Rusty.

I agree. For the record I agree with this change.

There's something that's bothering me.
We have re-written in-memory structures to say "le32" etc everywhere.
But it's wrong for the legacy interface.

Also, it now says "le" everywhere, so this isn't very
informative.

For the new interface, everything is le so there's real no need
to note this at each point.
But for the old interface, some fields are LE and some are
native.

So why don't we mark fields leXX if they are little endian in both
interfaces, and uXX if they are little endian in new one and
native in old one?

Alternatively we could invent our own type for "virtio endian".
It seemed like a nice idea to me originally but I had a hard time
coming up with with a nice name for it.

Thoughts?