From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35687) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aY6DK-00060E-Ol for qemu-devel@nongnu.org; Tue, 23 Feb 2016 01:12:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aY6DF-0004TF-Ms for qemu-devel@nongnu.org; Tue, 23 Feb 2016 01:12:50 -0500 Received: from e23smtp07.au.ibm.com ([202.81.31.140]:43474) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aY6DF-0004T3-3u for qemu-devel@nongnu.org; Tue, 23 Feb 2016 01:12:45 -0500 Received: from localhost by e23smtp07.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 23 Feb 2016 16:12:38 +1000 Received: from d23relay06.au.ibm.com (d23relay06.au.ibm.com [9.185.63.219]) by d23dlp02.au.ibm.com (Postfix) with ESMTP id 453882BB0059 for ; Tue, 23 Feb 2016 17:12:32 +1100 (EST) Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay06.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u1N6CJwv5636536 for ; Tue, 23 Feb 2016 17:12:32 +1100 Received: from d23av01.au.ibm.com (localhost [127.0.0.1]) by d23av01.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u1N6BteC030542 for ; Tue, 23 Feb 2016 17:11:55 +1100 Date: Tue, 23 Feb 2016 11:41:34 +0530 From: Bharata B Rao Message-ID: <20160223061113.GA10940@in.ibm.com> References: <1456117285-22273-1-git-send-email-bharata@linux.vnet.ibm.com> <56CB2A09.9040102@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <56CB2A09.9040102@suse.de> Subject: Re: [Qemu-devel] [RFC PATCH v0 0/8] cpu-package hotplug Reply-To: bharata@linux.vnet.ibm.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: ehabkost@redhat.com, aik@ozlabs.ru, armbru@redhat.com, agraf@suse.de, qemu-devel@nongnu.org, pbonzini@redhat.com, imammedo@redhat.com, david@gibson.dropbear.id.au On Mon, Feb 22, 2016 at 04:32:25PM +0100, Andreas Färber wrote: > Hi Bharata, > > Am 22.02.2016 um 06:01 schrieb Bharata B Rao: > > This is an attempt to implement David Gibson's RFC that was posted at > > https://lists.gnu.org/archive/html/qemu-ppc/2016-02/msg00000.html > > I am not sure if I have followed all the aspects of the RFC fully, but we > > can make changes going forward. > > I am not familiar with David's RFC beyond what was portrayed on the KVM > call - this is not what we discussed on the call and I don't like it. > > Further, your commits are pretty cryptic to me. Please improve your > commit messages. > > For example, you add a cpu_type field and you assign it the value > TYPE_POWERPC_CPU. That's not the user-chosen CPU type then, it's a base > CPU type that cannot be instantiated. Either name it cpu_base_type or > fill it in with proper values in one patch - that patch on its own does > not create value and does not explain your claim: > "Storing CPU typename in MachineState lets us to create CPU threads > for all architectures in uniform manner from arch-neutral code." > I'm pretty sure that CPU threads cannot be created from that type, as it > would run into an assertion. Yes, that should have been called cpu_base_type. > > Next, you make a functionally correct refactoring of cpu_generic_init(), > but I don't understand why you duplicate that code. cpu_foo_init() still > expects things to be realized, so instead of realizing once in a central > place you do it in nine different places. Had you touched all helper > functions we might be able to move that to three places, once for > softmmu, once or twice for linux-user and once for bsd-user. But I > rather get the feeling that you misunderstand those legacy helper > functions, they're for -cpu handling and not to my knowledge used for > cpu-add at all. You should not be using them and then won't need to > touch them in this way. By using them in your supposedly QOM code you > are hiding an object_new() call inside deep layers of helper functions > instead of using QOM native functions such as object_initialize(), > object_new() and object_property_set*(). The intention was to re-use this generic sounding (cpu_generic_init) routine with a bit of modification. But since it is a legacy helper as you mention, we should be fine with using object_new() and other QOM native functions directly. Regards, Bharata.