[Qemu-devel] Re: [PATCH 0/9] Virtio cleanups

[Juan Quintela]

Gerd Hoffmann <address@hidden> wrote:
>   Hi,
>
>>> I think we should move away from struct layout assumptions that
>>> DO_UPCAST enforces, and to use container_of where possible.
>>> I'll post a series shortly that do this for virtio.
>>
>> Not in this case.  qdev needs it to be in that order, and that will not
>> change without changing everything again.   Look at the rest of hw/*.
>>
>> The only "sane" way of doing OOP on C is to use the super-class memmbers as 
>> the
>> 1st member of the sub-classes.  That will not change anytime soon.  And
>> trying to "emulate" multiple inheritance in C is completely "insane".
>>
>> The improtant bit is the patch is:
>>
>> +    VirtIODevice *vdev = virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
>> +                                            sizeof(struct 
>> virtio_blk_config),
>> +                                            sizeof(VirtIOBlock));
>> +    s = DO_UPCAST(VirtIOBlock, vdev, vdev);
>>
>> You can't have a virtio_common_init() that initializes the shared bits
>> ad init them in the middle of nowhere.  It _needs_ to be at the
>> beginning of the shared struct.
>
> No.  qdev requires it for devices because it does the allocation for
> you.  qdev does *not* require it for busses for example.  You can
> easily embed the bus struct into the device state of the parent
> device:
>
> struct scsi_hba {
>    PCIDevice pcidev;
>    SCSIBus bus;
>    /* more device state here */
> };
>
> Moving virtio into the same direction is a good thing IMHO.  We can
> have something like this then:
>
> struct VirtioBlockPCI {
>    VirtIOPCIProxy proxy;
>    VirtIOBlock block;
> };
>
> The messy qdev property handing for virtio (copying stuff between vdev
> and proxy) can go away then.  And I think it also simplifies things
> alot for vmstate.

This one is different.  It has some advantage, and you don't have to do
the malloc() independently.  For vmstate will not help much (in this
case), but obviosly will not make things more difficult either.

Will think a bit about this, obviously it is not easy.  I tried at
some point doing something like:

struct VirtIOPCIProxy {
  (all VirtioPCIProxy)
  VirtIOBlock vdev;
}

And using sizeof tricks to get the right addresses.  I failed.  Perhaps
using all the Virtio*PCI types will fix it.  Would take a look at that.

But I still think that this is independent that VirtIO being 1st (or
not) memer of VirtIOBlock.

Later, Juan.