From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NsAQk-0007Ll-3J for qemu-devel@nongnu.org; Thu, 18 Mar 2010 03:46:11 -0400 Received: from [199.232.76.173] (port=51653 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NsAQh-0007LX-ON for qemu-devel@nongnu.org; Thu, 18 Mar 2010 03:46:07 -0400 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NsAQg-0005Ql-6D for qemu-devel@nongnu.org; Thu, 18 Mar 2010 03:46:07 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32280) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NsAQf-0005Qh-Nz for qemu-devel@nongnu.org; Thu, 18 Mar 2010 03:46:06 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o2I7k45c027398 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 18 Mar 2010 03:46:05 -0400 Date: Thu, 18 Mar 2010 09:42:39 +0200 From: "Michael S. Tsirkin" Message-ID: <20100318074239.GA23474@redhat.com> References: <20100318064015.GA16973@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org On Thu, Mar 18, 2010 at 08:36:26AM +0100, Juan Quintela wrote: > "Michael S. Tsirkin" 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(-) > >> > >>