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 <m...@redhat.com> > > > Signed-off-by: Isaku Yamahata <yamah...@valinux.co.jp> > > > > 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
