From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=54471 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q05aW-0004sw-Q6 for qemu-devel@nongnu.org; Thu, 17 Mar 2011 01:17:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q05aV-0004sx-1f for qemu-devel@nongnu.org; Thu, 17 Mar 2011 01:17:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:7854) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q05aU-0004sa-LR for qemu-devel@nongnu.org; Thu, 17 Mar 2011 01:17:31 -0400 Date: Thu, 17 Mar 2011 07:17:18 +0200 From: "Michael S. Tsirkin" Subject: Re: [Qemu-devel] Re: [PATCH 07/26] pci/p2pbr: generic pci p2p bridge Message-ID: <20110317051718.GE32049@redhat.com> References: <392c8eaaeb8f0729ab39b261d0dda21208c05ea6.1300266238.git.yamahata@valinux.co.jp> <20110316213441.GA32049@redhat.com> <20110317020851.GG16841@valinux.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110317020851.GG16841@valinux.co.jp> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaku Yamahata Cc: qemu-devel@nongnu.org On Thu, Mar 17, 2011 at 11:08:51AM +0900, Isaku Yamahata wrote: > On Wed, Mar 16, 2011 at 11:34:42PM +0200, Michael S. Tsirkin wrote: > > On Wed, Mar 16, 2011 at 06:29:18PM +0900, Isaku Yamahata wrote: > > > Create generic pci p2p bridge device which can be customized > > > via properties like vendor id/device id and so on. > > > With this, we can avoid to create many pci p2p bridge which only > > > differs in those ids. > > > > > > Cc: Michael S. Tsirkin > > > Signed-off-by: Isaku Yamahata > > > > So we added 213 lines and we saved all of 20 in other places? > > Maybe I miss the point ... > > What are missing is, > - The patch eliminates logic duplication rather than simple > line insertion/deletion. > - It also simplifies q35 code which is 26/26. Its code saving isn't counted. > - The lines of newly added copyright notice are counted. > - If line saving is so important, the numbers of lines can be > reduced dramatically by accepting 14 arguments functions instead > of using struct. struct initialization bloated line insertion. > But I think it doesn't increase code complexity. > But doesn't seem to reduce it either. Line count is just an attempt to see how well the abstraction works. All this one does is move from pci_config_set_vendor_id(dev, XXX) to .vendor_id = XXX. This just does not seem to be a big win. qdev properties are also user-visible, aren't they? Adding properties that, if changed, will confuse the guest doesn't seem to be a good idea either. > Anyway this patch isn't very critical. I think the available choice is > > - this patch > - modify the patch to use 14 arguments function. > Thus we can save much more lines. > - Add one more p2p bridge code which q35 uses, accepting same code which > differs only in IDs. > - any other ideas? > > Which option do you prefer? Add one more bridge for q35. > > > > > > > --- > > > Makefile.objs | 2 +- > > > hw/pci_p2pbr.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > hw/pci_p2pbr.h | 61 +++++++++++++++++++++++ > > > 3 files changed, 213 insertions(+), 1 deletions(-) > > > create mode 100644 hw/pci_p2pbr.c > > > create mode 100644 hw/pci_p2pbr.h > > > > -- > yamahata