From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39460) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QlKr9-0008Es-1a for qemu-devel@nongnu.org; Mon, 25 Jul 2011 09:06:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QlKr7-0005vt-FT for qemu-devel@nongnu.org; Mon, 25 Jul 2011 09:05:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34636) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QlKr7-0005vn-6y for qemu-devel@nongnu.org; Mon, 25 Jul 2011 09:05:57 -0400 Message-ID: <4E2D6AE3.10608@redhat.com> Date: Mon, 25 Jul 2011 15:08:51 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1311558293-5855-1-git-send-email-aliguori@us.ibm.com> <4E2D51A1.2080301@redhat.com> <4E2D657C.8050701@codemonkey.ws> In-Reply-To: <4E2D657C.8050701@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Anthony Liguori , qemu-devel@nongnu.org Am 25.07.2011 14:45, schrieb Anthony Liguori: > On 07/25/2011 06:21 AM, Kevin Wolf wrote: >> Am 25.07.2011 03:44, schrieb Anthony Liguori: >>> Hi, >>> >>> This series is the rough beginnings of the QEMU Object Model. This is basically >>> qdev generalized on steroids. >>> >>> This series includes the core infrastructure, a strawman Device type, and >>> the beginnings of the conversion of CharDriverState. This is in a rougher >>> state than I would like it to be but I wanted to get the concepts on the list >>> as soon as possible. >>> >>> My plan is to drop the Device parts from future versions of the series and just >>> focus on backends to start with. >>> >>> Please note that this series has an awful lot of ramifications. Most of our >>> current command line options would become deprecated, the build system will >>> change significantly, and a lot of our QMP functions will become deprecated. >>> >>> It seems like a lot of change, but hopefully this series illustrates how we >>> can do it very incrementally with value being added at each stage of the >>> conversion. >> >> I haven't looked in much detail at it yet, but it has still the same >> problem I was talking about last week: Patches 17-21 don't actually >> convert existing code, but they add new code. > > Actually, it's mostly existing code. In terms of incremental > conversion, the most straight forward way is to adds the new version > side-by-side with the old version and then remove the old version. > > Converting in device in place requires some gymnastics. If you think > it's absolutely critical, I could try to do it but I'm not sure I agree > it's the thing to do. Okay, if it isn't possible with reasonable effort, I guess we'll have to bite the bullet and give it a very careful manual review immediately before the merge. By the way, I see that you create new directories. You probably have an idea of what the directory structure will look like after the whole conversion is completed. Can you share this idea with us? >> This means that we can't >> review only the changes, but have to review the whole code. It also >> makes conflicts with patches modifying the old version hard to even notice. >> >> >> On another note, I'm not so sure if your renaming is really helpful. It >> doesn't matter that much with qemu-char because someone thought having >> the function pointers in CharDriverState was a good idea, but if you're >> consistent, the rename would go like this in the block layer: >> >> BlockDriverState -> BlockDriver >> BlockDriver -> BlockDriverClass > > I think we do need to introduce consistent naming conventions. > > If those conventions are FooState and Foo, that could be okay, but the > code base today is absolutely not consistent on it. > > I think Foo and FooClass is better because Foo is the most common usage > of the type and it's less characters to type. Maybe. But then, CharDriver is a really bad names for an instance. There is only one driver, which made it a good name for the class until now. Maybe CharBackend and CharBackendClass (or CharBackendDriver) would be a more sensible replacement that follows your pattern. Kevin