From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34161) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLzHA-0001wZ-QC for qemu-devel@nongnu.org; Wed, 10 Oct 2012 12:36:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TLzH2-0003kV-Ha for qemu-devel@nongnu.org; Wed, 10 Oct 2012 12:36:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46925) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLzH2-0003kC-8r for qemu-devel@nongnu.org; Wed, 10 Oct 2012 12:36:44 -0400 Date: Wed, 10 Oct 2012 13:37:21 -0300 From: Luiz Capitulino Message-ID: <20121010133721.7b461ff2@doriath.home> In-Reply-To: <20121010163810.14917897@nial.usersys.redhat.com> References: <1349192235-31895-1-git-send-email-imammedo@redhat.com> <1349192235-31895-2-git-send-email-imammedo@redhat.com> <50758096.6000805@suse.de> <20121010163810.14917897@nial.usersys.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 01/23] target-i386: return Error from cpu_x86_find_by_name() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: aliguori@us.ibm.com, hpa@linux.intel.com, ehabkost@redhat.com, jan.kiszka@siemens.com, Don@cloudswitch.com, mtosatti@redhat.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, blauwirbel@gmail.com, avi@redhat.com, pbonzini@redhat.com, akong@redhat.com, lersek@redhat.com, Andreas =?ISO-8859-1?B?RuRyYmVy?= , stefanha@linux.vnet.ibm.com On Wed, 10 Oct 2012 16:38:10 +0200 Igor Mammedov wrote: > On Wed, 10 Oct 2012 16:05:10 +0200 > Andreas F=E4rber wrote: >=20 > > Am 02.10.2012 17:36, schrieb Igor Mammedov: > > > it will allow to use property setters there later. > > >=20 > > > Signed-off-by: Igor Mammedov > > > Reviewed-by: Don Slutz > > > Reviewed-by: Eduardo Habkost > > > -- > > > v2: > > > - style change, add braces (reqested by Blue Swirl) > > > - removed unused error_is_set(errp) in properties set loop > > > --- > > > target-i386/cpu.c | 15 ++++++++++++--- > > > 1 files changed, 12 insertions(+), 3 deletions(-) > > >=20 > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > > index bb1e44e..e1ffa40 100644 > > > --- a/target-i386/cpu.c > > > +++ b/target-i386/cpu.c > > > @@ -1097,7 +1097,8 @@ static void x86_cpuid_set_tsc_freq(Object *obj, > > > Visitor *v, void *opaque, cpu->env.tsc_khz =3D value / 1000; > > > } > > > =20 > > > -static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char > > > *cpu_model) +static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t > > > *x86_cpu_def, > > > + const char *cpu_model, Error **errp) > > > { > > > unsigned int i; > > > x86_def_t *def; > >=20 > > > @@ -1254,6 +1255,7 @@ static int cpu_x86_find_by_name(x86_def_t > > > *x86_cpu_def, const char *cpu_model) fprintf(stderr, "feature string = `%s' > > > not in format (+feature|-feature|feature=3Dxyz)\n", featurestr); goto= error; > > > } > > > + > > > featurestr =3D strtok(NULL, ","); > > > } > > > x86_cpu_def->features |=3D plus_features; > >=20 > > Disconnected whitespace change. > >=20 > > > @@ -1282,6 +1284,9 @@ static int cpu_x86_find_by_name(x86_def_t > > > *x86_cpu_def, const char *cpu_model)=20 > > > error: > > > g_free(s); > > > + if (!error_is_set(errp)) { > > > + error_set(errp, QERR_INVALID_PARAMETER_COMBINATION); > >=20 > > Note to self: error_set() checks for errp being NULL, so the use of > > !error_is_set() logic seems fine. > >=20 > > QERR_ alarm - acceptable use? Or better use error_setg() or something? > >=20 > > Otherwise looks okay to me, I could drop the whitespace hunk myself. > I'll fix it for the next re-spin of this series and do whatever is decided > with QERR_... It's simpler than you can imagine, you can just do: error_setg(errp, "invalid parameter combination: %s", params); >=20 > Thanks! >=20 > >=20 > > Andreas > >=20 > > > + } > > > return -1; > > > } > > > =20 > > > @@ -1368,8 +1373,10 @@ int cpu_x86_register(X86CPU *cpu, const char > > > *cpu_model)=20 > > > memset(def, 0, sizeof(*def)); > > > =20 > > > - if (cpu_x86_find_by_name(def, cpu_model) < 0) > > > - return -1; > > > + if (cpu_x86_find_by_name(cpu, def, cpu_model, &error) < 0) { > > > + goto out; > > > + } > > > + > > > if (def->vendor1) { > > > env->cpuid_vendor1 =3D def->vendor1; > > > env->cpuid_vendor2 =3D def->vendor2; > > > @@ -1419,6 +1426,8 @@ int cpu_x86_register(X86CPU *cpu, const char > > > *cpu_model) env->cpuid_svm_features &=3D TCG_SVM_FEATURES; > > > } > > > object_property_set_str(OBJECT(cpu), def->model_id, "model-id", > > > &error); + > > > +out: > > > if (error_is_set(&error)) { > > > error_free(error); > > > return -1; > > >=20 > >=20 > >=20 >=20