"Michael S. Tsirkin"<address@hidden> wrote:
On Sun, Mar 21, 2010 at 06:11:43PM +0000, Jamie Lokier wrote:
Michael S. Tsirkin wrote:
That's version 1 of my patch. Version 2 removed even need for macro
completely by moving allocations to the caller.
The downside of moving allocations are: (1) it's one more call in the
caller, to allocate the type, (2) it needs a virtual destructor for
each type to free the object, which can clutter the code if there is
no other reason for virtual destructors.
I don't think those are necessarily bad, but they can remove from the
neatness of existing code. Personally I favour an occasional macro
using sizeof/offsetof/container_of if the result is a natural and
sensible API to all of its callers.
-- Jamie
We need to free in caller anyway because structure is used
after cleanup. This can be changed by restructuring code ...
I don't have a strong preference, anything is better
than the hack relying on field being at offset 0 in structure.
It is not a hack.
Is the only sane way of sharing virtio common field without having to
export all virtio particular structs :(
I really hate how this is going :(
a- we have code that assumes that virtio is the 1st element of all
virtio structs.
b- we have a patch that codifies that virtio is used in that way (using
DO_UPCAST()) (me)
c- we arrived to the point where being it at the beggining of the struct
is the bigger cast in the universe (I present everybody thinking that
to look at rest of qemu). Patch is done that makes it possible to
alloc memory outside of virtio_common_init(). No code on virtio.c or
virtio-pci.c is changed to use this new offset. (michael).
d- Anthony arrives to the discussion stating that it should exist a
VirtIOPCIBus, that way virtio devices should be hanging of that bus
(that requires lots of changes).
e- kraxel arrives to the discussion stating that initializing in the
middle of one struct is the right thing to do, except if you are
qdev, that then it is not. I point to his suggestion that it makes
things still uglier (no answer yet, but it was weekend for him).
f- I show using kraxel example how ugly things go (sizeof (struct A) +
sizeof (struct B) - sizeof (struct C)). And here is when things fall
apart IMHO. Michael states that it is ok to have to had:
- an offset field
- a pointer in one struct to inside the same struct (vdev)
but assuming and stating and using an offset 0 is ugly.
Have I lost anything? Is there a realistic way of getting struct
VirtIODevice (and VirtIOBlock, ...) inside VirtIOPCIProxy that don't
imply using an offset + a pointer + exporting the struct definitions
only to calculate its size?