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

[Gerd Hoffmann]

  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.

cheers,
  Gerd