From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45578) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFdfc-0002ye-Fs for qemu-devel@nongnu.org; Wed, 22 Jun 2016 04:38:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bFdfb-0007Dg-Cd for qemu-devel@nongnu.org; Wed, 22 Jun 2016 04:38:00 -0400 Date: Wed, 22 Jun 2016 16:37:47 +0800 From: Fam Zheng Message-ID: <20160622083747.GA17040@ad.usersys.redhat.com> References: <1466500894-9710-1-git-send-email-kwolf@redhat.com> <3f872c08-06d8-d755-9369-02ecd0d6d000@redhat.com> <20160621105620.GD4520@noname.redhat.com> <20160621113146.GE4520@noname.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160621113146.GE4520@noname.redhat.com> Subject: Re: [Qemu-devel] [PATCH 00/17] block: Convert common I/O path to BdrvChild List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Paolo Bonzini , qemu-block@nongnu.org, jcody@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com On Tue, 06/21 13:31, Kevin Wolf wrote: > Am 21.06.2016 um 13:01 hat Paolo Bonzini geschrieben: > > On 21/06/2016 12:56, Kevin Wolf wrote: > > > Am 21.06.2016 um 11:47 hat Paolo Bonzini geschrieben: > > >> I still fail to understand what is the rationale for this change. The > > >> API is weird; you read from a disk, not from an edge, and in fact the > > >> first thing all the APIs do is dereference the BdrvChild... > > >> > > >> The assertions are nice, but I'm not sure it's a good idea to design a > > >> whole API around them. > > > > > > Do you see a problem with such an API, though? If there is no reason not > > > to have the advantages, as small as they may seem, why not take them? > > > > I don't see a reason not to take them; I don't see any red flags, but > > there are some yellow flags (the kinda weird API) that I don't > > understand and I hope you can explain. > > > > Thinking more about it, it's perfectly possible that this is just a > > combination of block/io.c's growth by accretion and the well-known fact > > "naming pseudo-OOP member functions in C sucks". I think this interface makes a lot of sense to me - I have been looking forward to this model since three or four years ago, just without knowing ahead that the object will be called BdrvChild, and that they'll occupy the first parameter of bdrv_* API. I'm glad we are here today. > > > > In other words, if you sell me this as "let's add some member functions > > to BdrvChild and use them", I can buy it. Perhaps the only thing to do > > then is to rename functions and design a consistent naming. And so I agree on renaming functions. :) Fam > > Hm, I never thought about it this way, but I think it actually makes > sense. > > As we want to represent a graph where both nodes and edges can have > attributes and methods, OOP-wise both of them are objects, namely BDS > and BdrvChild. > > So we have some BDS A that has a Child B, and Child B in turn has a > BDS C. What we used to do is that A asks B for the node it points to > (C), and then directly calls a method of C. After the conversion, A > calls a method of B, which in turn forwards the request by calling a > method of C, which is much more straightforward and ideally even allows > the node that B points to to remain private (we're not quite there, > though).