From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NsLX6-0002j2-R3 for qemu-devel@nongnu.org; Thu, 18 Mar 2010 15:37:28 -0400 Received: from [199.232.76.173] (port=42944 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NsLX6-0002iL-75 for qemu-devel@nongnu.org; Thu, 18 Mar 2010 15:37:28 -0400 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NsLX4-0000TF-MG for qemu-devel@nongnu.org; Thu, 18 Mar 2010 15:37:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51650) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NsLX4-0000TB-An for qemu-devel@nongnu.org; Thu, 18 Mar 2010 15:37:26 -0400 Received: from int-mx03.intmail.prod.int.phx2.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o2IJbPCi012686 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 18 Mar 2010 15:37:25 -0400 From: Juan Quintela In-Reply-To: <20100318172156.GB27805@redhat.com> (Michael S. Tsirkin's message of "Thu, 18 Mar 2010 19:21:56 +0200") References: <20100318064015.GA16973@redhat.com> <4BA24A26.5030602@redhat.com> <4BA2569A.3040600@redhat.com> <20100318172156.GB27805@redhat.com> Date: Thu, 18 Mar 2010 20:37:23 +0100 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: "Michael S. Tsirkin" Cc: Gerd Hoffmann , qemu-devel@nongnu.org "Michael S. Tsirkin" wrote: > On Thu, Mar 18, 2010 at 06:08:06PM +0100, Juan Quintela wrote: >> Gerd Hoffmann wrote: > > The whole '1st place' hack is really not necessary, if we know the > offset we can use container_of. container_of() don't cut mustard. The whole point is that virtio-pci code don't care at all about what virtio specific device it has. You should have to use an offset/pointer (take your poison). > >> > Just changing it for the snake of change isn't a good reason >> > either. But if it helps cleaning the code we can change it without >> > running into trouble. You can't cast VirtIODevice to VirtIOBlock any >> > more, but you don't really want to anyway for type checking reasons. >> >> As I have show, from the time that virtio-pci only uses the virtio.c >> functions (and needs VirtIODevice fields), I don't see how to embed it >> and share the current functions in virtio-pci.c. > > We can just keep the pointer from VirtiPCIProxy to virtio device. Then there is not much reason of using it embeded in the 1st place. If we are talking about hackish approachs, a pointer to the middle of the own struct has lots of points of "hackiness" >> Notice that what we really want is create the devices in >> >> .qdev.name = "virtio-balloon-pci", >> .qdev.size = sizeof(VirtIOPCIProxy), >> >> as >> >> .qdev.name = "virtio-balloon-pci", >> .qdev.size = sizeof(VirtIOPCIProxy) + sizeof(VirtIOBlock) - sizeof(VirtIODevice), >> >> And having a single definition of: >> >> struct VirtioPCIProxy { >> >> struct VirtIODevice vdev; >> } >> >> Yes, it is ugly/subtle as hell, but no other simple way. You allocate >> the struct on each virtio-*.c file, where you know the size, and no >> embed. Or you embed the struct, and replicate lots of code to have it >> type safe. > > We don't need to drop the vdev pointer if it's helpful. > If we have it around there will be close to no code > changes. And no advantage for the change either :( Reducing one qemu_mallocz() and nothing else. >> Any of the two options are worse than current way IMHO. Having to >> export VirtIOBlock only to know its size looks wrong in addition. >> >> Depending on size of the last element is ... too hackish? > > Yes. Then I think that changing to not be the 1st element brings us ... nothing? Or worse, we have to export the VirtIO* structs that are private now. I think that it is better just my cleanup, the other change just complicate things. Later, Juan.