On Thu, Mar 18, 2010 at 08:36:26AM +0100, Juan Quintela wrote:
> "Michael S. Tsirkin" <m...@redhat.com> 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.
I don't think qdev cares about how vdev field is within device
structure, it is not virtio specific.
> Look at the rest of hw/*.
I think you mean that a similar assumption is made by lots of
other code. Does not mean it's always a good idea and that
we need to force this assumption everywhere.
> 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".
container_of does it and I think it's sane.
People like comparing it to inheritance but for me it's just
using a field in a structure. multiple fields in a structure are 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.
I just sent a patch that removes this assumption.
> 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.
I do not think it is essential at all. We just add an offset.
Yes we make such assumptions a lot but it's a bug, not a feature.
> >> - 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(-)
> >>
> >>
