[Qemu-devel] Re: [PATCH 06/41] virtio: Use DO_UPCAST instead of a cast

[Juan Quintela]

"Michael S. Tsirkin" <address@hidden> wrote:
> On Wed, Dec 02, 2009 at 07:19:17PM +0100, Juan Quintela wrote:
>> "Michael S. Tsirkin" <address@hidden> wrote:
>> > On Wed, Dec 02, 2009 at 01:04:04PM +0100, Juan Quintela wrote:
>> >> virtio_common_init() creates a struct with the right size, DO_UPCAST
>> >> is the appropiate thing here
>> >> 
>> >> Signed-off-by: Juan Quintela <address@hidden>
>> >
>> > BTW why not container_of? That one does not require
>> > field to be at the beginning of structure.
>> 
>> VirtIO devices (and PCIDevices) are declared in this way:
>> 
>> typedef struct VirtIOBalloon
>> {
>>     VirtIODevice vdev;
>>     VirtQueue *ivq, *dvq;
>>     uint32_t num_pages;
>>     uint32_t actual;
>> } VirtIOBalloon;
>> 
>> 
>> I.e. the virtioDevice is always the 1st element, otherwise things don't
>> work.  There are code that requires it to be the 1st element.
>
> I know. But I think we should slowly fix these assumptions, and not
> introduce more of them. IOW: don't use DO_UPCAST.

It is inherent in how to implement OOP in C.  You want to use things as
pci devices and as pci specific devices.  Then you need to put the pci
common fields at the beggining.  Yes, for virtio devices they want to be
pci and virtio devices, but neither pci or virtio nor qemu in general
allow to be derived of two types (a.k.a. as multiple inheritance).

I think that we should continue using DO_UPCAST() until there are some
design that allows you to change that.  This particular case "requires"
that VirtioDevice is the 1st field of the struct.  If you change the
code enough to make container_of() work, doing the
s/DO_UPCAST/container_of/ is going to be the less of your problems.

Later, Juan.