From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52222) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPZ86-0008Rh-2S for qemu-devel@nongnu.org; Tue, 09 Apr 2013 10:02:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UPZ83-0005ra-EV for qemu-devel@nongnu.org; Tue, 09 Apr 2013 10:02:34 -0400 Received: from cantor2.suse.de ([195.135.220.15]:57774 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPZ83-0005rG-5l for qemu-devel@nongnu.org; Tue, 09 Apr 2013 10:02:31 -0400 Message-ID: <51641F75.3040709@suse.de> Date: Tue, 09 Apr 2013 16:02:29 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1365172636-28628-1-git-send-email-imammedo@redhat.com> <1365172636-28628-4-git-send-email-imammedo@redhat.com> <20130408183046.GB11124@otherpad.lan.raisama.net> <5163EDBD.7070907@redhat.com> <20130409123347.33fb0a1b@nial.usersys.redhat.com> In-Reply-To: <20130409123347.33fb0a1b@nial.usersys.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 03/22] target-i386: split out CPU creation and features parsing into cpu_x86_create() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: aliguori@us.ibm.com, Eduardo Habkost , claudio.fontana@huawei.com, qemu-devel@nongnu.org, aderumier@odiso.com, lcapitulino@redhat.com, jfrei@linux.vnet.ibm.com, yang.z.zhang@intel.com, Paolo Bonzini , lig.fnst@cn.fujitsu.com, rth@twiddle.net Am 09.04.2013 12:33, schrieb Igor Mammedov: > On Tue, 09 Apr 2013 12:30:21 +0200 > Paolo Bonzini wrote: >=20 >> Il 08/04/2013 20:30, Eduardo Habkost ha scritto: >>>>> - cpu_x86_register(cpu, name, &error); >>>>> - if (error) { >>>>> + cpu_x86_register(cpu, name, errp); >>>>> + if (error_is_set(errp)) { >>> So the function now does error checking properly if and only if errp = is >>> not NULL. Do we really want to do that? >> >> No, using error_propagate is the correct idiom indeed. > Ok, I'll use error_propagate() in next version. I remember accepting some CPU refactoring patch but asking you to fix up the same issue as follow-up - I don't remember having received a single fix-up patch, could you please check on whether that is hidden in some series or still missing? Thanks! Andreas >=20 >> >> Paolo >> >>>>> goto out; >>>>> } >>>>> =20 >>>>> - cpu_x86_parse_featurestr(cpu, features, &error); >>>>> - if (error) { >>>>> + cpu_x86_parse_featurestr(cpu, features, errp); >>>>> + if (error_is_set(errp)) { >>>>> goto out; >>>>> } >>>>> =20 >>>>> - object_property_set_bool(OBJECT(cpu), true, "realized", &error= ); >>>>> +out: >>>>> + g_strfreev(model_pieces); >>> Any specific reason you didn't choose to keep 'Error *error =3D NULL' >>> inside cpu_x86_create() as well, and use error_propagate() here? I >>> believe it would make the patch simpler and easier to review, and at = the >>> same time make cpu_x86_init() check for errors properly even if errp = is >>> NULL. This is the opposite of what you did on x86_cpu_realizefn() at >>> patch 01/22. >> >=20 --=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