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

[Juan Quintela]

"Michael S. Tsirkin" <address@hidden> wrote:
> On Tue, Mar 16, 2010 at 07:51:16PM +0100, Juan Quintela wrote:
>> Hi
>> 
>> This series introduces several virtio cleanups:
>> - add comment to pci (mst)
>> - tell virtio about DO_UPCAST
>
> 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.  This "assumption" is used for all the
code left and right.  It is an essentioal part of the qdev design, not
something that can be changed easily.

>> - use QLIST instead of one open list
>> - virtio-pci/msix: remove duplicated test
>> 
>> Please review and apply.
>> 
>> This is split for a series previously sent.  Will send the vmstate
>> conversions as a different series on top of this one.
>> 
>> Later, Juan.
>> 
>> Juan Quintela (8):
>>   virtio: Teach virtio-balloon about DO_UPCAST
>>   virtio: Teach virtio-blk about DO_UPCAST
>>   virtio: Teach virtio-net about DO_UPCAST
>>   virtio: Use DO_UPCAST instead of a cast
>>   virtio-pci: Remove duplicate test
>>   QLIST: Introduce QLIST_COPY_HEAD
>>   virtio-blk: change rq type to VirtIOBlockReq
>>   virtio-blk: use QLIST for the list of requests
>> 
>> Michael S. Tsirkin (1):
>>   qemu/pci: document msix_entries_nr field
>> 
>>  hw/msix.c           |    8 -------
>>  hw/pci.h            |    4 ++-
>>  hw/virtio-balloon.c |   15 ++++---------
>>  hw/virtio-blk.c     |   54 
>> ++++++++++++++++++++++++--------------------------
>>  hw/virtio-net.c     |   29 +++++++++++----------------
>>  hw/virtio-pci.c     |    7 +++--
>>  qemu-queue.h        |    4 +++
>>  7 files changed, 54 insertions(+), 67 deletions(-)
>> 
>>