From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59582) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCzWJ-0001Ne-3O for qemu-devel@nongnu.org; Mon, 10 Feb 2014 17:40:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WCzWB-0001EO-Pm for qemu-devel@nongnu.org; Mon, 10 Feb 2014 17:40:07 -0500 Received: from cantor2.suse.de ([195.135.220.15]:50975 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCzWB-0001Cr-JP for qemu-devel@nongnu.org; Mon, 10 Feb 2014 17:39:59 -0500 Message-ID: <52F95537.40007@suse.de> Date: Mon, 10 Feb 2014 23:39:51 +0100 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1391111339-6958-1-git-send-email-ehabkost@redhat.com> <1391111339-6958-8-git-send-email-ehabkost@redhat.com> <20140131181338.GB28427@otherpad.lan.raisama.net> <52F81C09.6070504@suse.de> <20140210081958.GN24353@otherpad.lan.raisama.net> <20140210082617.GO24353@otherpad.lan.raisama.net> <20140210102130.GP24353@otherpad.lan.raisama.net> In-Reply-To: <20140210102130.GP24353@otherpad.lan.raisama.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [qom-cpu PATCH 7/7 v9] target-i386: CPU model subclasses List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: kvm@vger.kernel.org, libvir-list@redhat.com, qemu-devel@nongnu.org, Igor Mammedov , Paolo Bonzini , Jiri Denemark Am 10.02.2014 11:21, schrieb Eduardo Habkost: > +static const TypeInfo x86_cpu_host_type_info =3D { > + .name =3D CPU_CLASS_NAME("host"), > + .parent =3D TYPE_X86_CPU, > + .instance_size =3D sizeof(X86CPU), > + .instance_init =3D x86_cpu_instance_init_host, > + .abstract =3D false, > + .class_size =3D sizeof(X86CPUClass), > + .class_init =3D x86_cpu_class_init_host, > +}; This looks broken: .class_data is not set but the loading of the cpudef happens in the TYPE_X86_CPU initfn. My preferred solution would be to move the cpudef-loading from TYPE_X86_CPU's instance_init to a separate one specified for the models only, allowing non-cpudef-based models. Not finished investigating yet. For now I've prepended a patch implementing my generalized CPUClass::class_by_name instead of a custom x86_cpu_class_by_name(). Other style nits that I'm working on cleaning up are declarations in the middle of blocks, keeping _class_init naming convention (pretty sure my patches always had the most-specific-to-least-specific naming), strictly distinguishing between type and class, adding to my gtk-style documentation rather than new custom comments, placing struct documentation in the header and keeping the diff nicely readable AFAP. I'd further like to keep some other conventions from previous CPU subclasses, like pulling the model for loop out of the model registration function. My patches had always tried to turn what is now x86_cpu_load_def() into an instance_init function rather than calling it from one - did you have reasons not to? Did you consider converting the host model in a first step to make the patch smaller? I'd rather finish my investigations and discuss my v10 patches but Paolo is already asking whether Eduardo should send a PULL, so here's my textual reply informing of some thoughts and WIP. ;) Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg