From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:50006)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <imammedo@redhat.com>) id 1UPVsG-0008Qf-Uz
	for qemu-devel@nongnu.org; Tue, 09 Apr 2013 06:34:02 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <imammedo@redhat.com>) id 1UPVsF-0001fF-Rq
	for qemu-devel@nongnu.org; Tue, 09 Apr 2013 06:34:00 -0400
Received: from mx1.redhat.com ([209.132.183.28]:27056)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <imammedo@redhat.com>) id 1UPVsF-0001f7-JW
	for qemu-devel@nongnu.org; Tue, 09 Apr 2013 06:33:59 -0400
Date: Tue, 9 Apr 2013 12:33:47 +0200
From: Igor Mammedov <imammedo@redhat.com>
Message-ID: <20130409123347.33fb0a1b@nial.usersys.redhat.com>
In-Reply-To: <5163EDBD.7070907@redhat.com>
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>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 03/22] target-i386: split out CPU creation
 and features parsing into cpu_x86_create()
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: aliguori@us.ibm.com, Eduardo Habkost <ehabkost@redhat.com>, claudio.fontana@huawei.com, qemu-devel@nongnu.org, aderumier@odiso.com, lcapitulino@redhat.com, jfrei@linux.vnet.ibm.com, yang.z.zhang@intel.com, afaerber@suse.de, lig.fnst@cn.fujitsu.com, rth@twiddle.net

On Tue, 09 Apr 2013 12:30:21 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> 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.

> 
> Paolo
> 
> >> >          goto out;
> >> >      }
> >> >  
> >> > -    cpu_x86_parse_featurestr(cpu, features, &error);
> >> > -    if (error) {
> >> > +    cpu_x86_parse_featurestr(cpu, features, errp);
> >> > +    if (error_is_set(errp)) {
> >> >          goto out;
> >> >      }
> >> >  
> >> > -    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 = 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.
>