* [Qemu-devel] [PATCH 01/22] target-i386: return Error from cpu_x86_find_by_name()
2012-09-07 20:54 [Qemu-devel] [PATCH 00/22 v2] target-i386: convert CPU features into properties Igor Mammedov
@ 2012-09-07 20:54 ` Igor Mammedov
2012-09-11 19:41 ` Don Slutz
2012-09-13 14:38 ` Eduardo Habkost
0 siblings, 2 replies; 7+ messages in thread
From: Igor Mammedov @ 2012-09-07 20:54 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, aliguori, stefanha, jan.kiszka, mdroth, don.slutz,
blauwirbel, avi, pbonzini, lersek, afaerber, ehabkost
it will allow to use property setters there later.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
--
v2:
style change, add braces (reqested by Blue Swirl)
---
target-i386/cpu.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ac12139..a89bdc4 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1086,7 +1086,8 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
cpu->env.tsc_khz = value / 1000;
}
-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;
@@ -1241,6 +1242,11 @@ 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=xyz)\n", featurestr);
goto error;
}
+
+ if (error_is_set(errp)) {
+ goto error;
+ }
+
featurestr = strtok(NULL, ",");
}
x86_cpu_def->features |= plus_features;
@@ -1264,6 +1270,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
error:
g_free(s);
+ if (!error_is_set(errp)) {
+ error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
+ }
return -1;
}
@@ -1350,8 +1359,10 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
memset(def, 0, sizeof(*def));
- 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 = def->vendor1;
env->cpuid_vendor2 = def->vendor2;
@@ -1401,6 +1412,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
env->cpuid_svm_features &= 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;
--
1.7.11.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 01/22] target-i386: return Error from cpu_x86_find_by_name()
2012-09-07 20:54 ` [Qemu-devel] [PATCH 01/22] target-i386: return Error from cpu_x86_find_by_name() Igor Mammedov
@ 2012-09-11 19:41 ` Don Slutz
2012-09-13 14:38 ` Eduardo Habkost
1 sibling, 0 replies; 7+ messages in thread
From: Don Slutz @ 2012-09-11 19:41 UTC (permalink / raw)
To: Igor Mammedov
Cc: peter.maydell, aliguori, stefanha, jan.kiszka, mdroth, qemu-devel,
blauwirbel, don.slutz, avi, pbonzini, lersek, afaerber, ehabkost
On 09/07/12 16:54, Igor Mammedov wrote:
> it will allow to use property setters there later.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> --
> v2:
> style change, add braces (reqested by Blue Swirl)
> ---
> target-i386/cpu.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index ac12139..a89bdc4 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1086,7 +1086,8 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
> cpu->env.tsc_khz = value / 1000;
> }
>
> -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;
> @@ -1241,6 +1242,11 @@ 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=xyz)\n", featurestr);
> goto error;
> }
> +
> + if (error_is_set(errp)) {
> + goto error;
> + }
> +
> featurestr = strtok(NULL, ",");
> }
> x86_cpu_def->features |= plus_features;
> @@ -1264,6 +1270,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
>
> error:
> g_free(s);
> + if (!error_is_set(errp)) {
> + error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
> + }
> return -1;
> }
>
> @@ -1350,8 +1359,10 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
>
> memset(def, 0, sizeof(*def));
>
> - 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 = def->vendor1;
> env->cpuid_vendor2 = def->vendor2;
> @@ -1401,6 +1412,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
> env->cpuid_svm_features &= 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;
Reviewed-by: Don Slutz <Don@CloudSwitch.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 01/22] target-i386: return Error from cpu_x86_find_by_name()
2012-09-07 20:54 ` [Qemu-devel] [PATCH 01/22] target-i386: return Error from cpu_x86_find_by_name() Igor Mammedov
2012-09-11 19:41 ` Don Slutz
@ 2012-09-13 14:38 ` Eduardo Habkost
2012-09-13 15:49 ` Igor Mammedov
1 sibling, 1 reply; 7+ messages in thread
From: Eduardo Habkost @ 2012-09-13 14:38 UTC (permalink / raw)
To: Igor Mammedov
Cc: peter.maydell, aliguori, stefanha, jan.kiszka, mdroth, qemu-devel,
blauwirbel, don.slutz, avi, pbonzini, lersek, afaerber
On Fri, Sep 07, 2012 at 10:54:50PM +0200, Igor Mammedov wrote:
> it will allow to use property setters there later.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> --
> v2:
> style change, add braces (reqested by Blue Swirl)
> ---
> target-i386/cpu.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index ac12139..a89bdc4 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1086,7 +1086,8 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
> cpu->env.tsc_khz = value / 1000;
> }
>
> -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;
> @@ -1241,6 +1242,11 @@ 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=xyz)\n", featurestr);
> goto error;
> }
> +
> + if (error_is_set(errp)) {
> + goto error;
> + }
> +
Why add this check, if you never use errp on any code above that line
(and eventually all the code above gets removed by patch 19)?
But is harmless (as error is never set anyway), and this is code that
simply gets removed later in this series. So:
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> featurestr = strtok(NULL, ",");
> }
> x86_cpu_def->features |= plus_features;
> @@ -1264,6 +1270,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
>
> error:
> g_free(s);
> + if (!error_is_set(errp)) {
> + error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
> + }
> return -1;
> }
>
> @@ -1350,8 +1359,10 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
>
> memset(def, 0, sizeof(*def));
>
> - 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 = def->vendor1;
> env->cpuid_vendor2 = def->vendor2;
> @@ -1401,6 +1412,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
> env->cpuid_svm_features &= 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;
> --
> 1.7.11.4
>
>
--
Eduardo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 01/22] target-i386: return Error from cpu_x86_find_by_name()
2012-09-13 14:38 ` Eduardo Habkost
@ 2012-09-13 15:49 ` Igor Mammedov
0 siblings, 0 replies; 7+ messages in thread
From: Igor Mammedov @ 2012-09-13 15:49 UTC (permalink / raw)
To: Eduardo Habkost
Cc: peter.maydell, aliguori, stefanha, jan.kiszka, mdroth, qemu-devel,
blauwirbel, don.slutz, avi, pbonzini, lersek, afaerber
On Thu, 13 Sep 2012 11:38:08 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Fri, Sep 07, 2012 at 10:54:50PM +0200, Igor Mammedov wrote:
> > it will allow to use property setters there later.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > --
> > v2:
> > style change, add braces (reqested by Blue Swirl)
> > ---
> > target-i386/cpu.c | 19 ++++++++++++++++---
> > 1 file changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index ac12139..a89bdc4 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1086,7 +1086,8 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
> > cpu->env.tsc_khz = value / 1000;
> > }
> >
> > -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;
> > @@ -1241,6 +1242,11 @@ 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=xyz)\n", featurestr);
> > goto error;
> > }
> > +
> > + if (error_is_set(errp)) {
> > + goto error;
> > + }
> > +
>
> Why add this check, if you never use errp on any code above that line
> (and eventually all the code above gets removed by patch 19)?
It looks like remnants from initial version where each converted feature had
setter in this loop.
I'll remove it for next respin.
>
> But is harmless (as error is never set anyway), and this is code that
> simply gets removed later in this series. So:
>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>
>
> > featurestr = strtok(NULL, ",");
> > }
> > x86_cpu_def->features |= plus_features;
> > @@ -1264,6 +1270,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
> >
> > error:
> > g_free(s);
> > + if (!error_is_set(errp)) {
> > + error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
> > + }
> > return -1;
> > }
> >
> > @@ -1350,8 +1359,10 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
> >
> > memset(def, 0, sizeof(*def));
> >
> > - 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 = def->vendor1;
> > env->cpuid_vendor2 = def->vendor2;
> > @@ -1401,6 +1412,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
> > env->cpuid_svm_features &= 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;
> > --
> > 1.7.11.4
> >
> >
>
> --
> Eduardo
--
Regards,
Igor
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 01/22] target-i386: return Error from cpu_x86_find_by_name()
[not found] <1348688952-5748-1-git-send-email-imammedo@redhat.com>
@ 2012-09-26 19:48 ` Igor Mammedov
2012-09-26 20:23 ` Igor Mammedov
0 siblings, 1 reply; 7+ messages in thread
From: Igor Mammedov @ 2012-09-26 19:48 UTC (permalink / raw)
To: qemu-devel
Cc: aliguori, ehabkost, jan.kiszka, Don, mtosatti, mdroth, blauwirbel,
pbonzini, lersek, afaerber, stefanha
it will allow to use property setters there later.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Don Slutz <Don@CloudSwitch.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
--
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(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c4e6163..e1e2346 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1086,7 +1086,8 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
cpu->env.tsc_khz = value / 1000;
}
-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;
@@ -1241,6 +1242,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=xyz)\n", featurestr);
goto error;
}
+
featurestr = strtok(NULL, ",");
}
x86_cpu_def->features |= plus_features;
@@ -1264,6 +1266,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
error:
g_free(s);
+ if (!error_is_set(errp)) {
+ error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
+ }
return -1;
}
@@ -1350,8 +1355,10 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
memset(def, 0, sizeof(*def));
- 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 = def->vendor1;
env->cpuid_vendor2 = def->vendor2;
@@ -1401,6 +1408,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
env->cpuid_svm_features &= 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;
--
1.7.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 01/22] target-i386: return Error from cpu_x86_find_by_name()
2012-09-26 19:48 ` [Qemu-devel] [PATCH 01/22] target-i386: return Error from cpu_x86_find_by_name() Igor Mammedov
@ 2012-09-26 20:23 ` Igor Mammedov
0 siblings, 0 replies; 7+ messages in thread
From: Igor Mammedov @ 2012-09-26 20:23 UTC (permalink / raw)
To: qemu-devel
Looks like mail server suffered failure. I'll try to repost later.
---
----- The following addresses had permanent fatal errors -----
<qemu-devel@nongnu.org>
----- Transcript of session follows -----
550 5.1.2 <qemu-devel@nongnu.org>... Host unknown (Name server: eggs.gnu.org.:
host not found)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 01/22] target-i386: return Error from cpu_x86_find_by_name()
2012-09-26 20:32 [Qemu-devel] [PATCH 00/22 v3] target-i386: convert CPU features into properties Igor Mammedov
@ 2012-09-26 20:32 ` Igor Mammedov
0 siblings, 0 replies; 7+ messages in thread
From: Igor Mammedov @ 2012-09-26 20:32 UTC (permalink / raw)
To: qemu-devel
Cc: aliguori, ehabkost, jan.kiszka, Don, mtosatti, mdroth, blauwirbel,
pbonzini, lersek, afaerber, stefanha
it will allow to use property setters there later.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Don Slutz <Don@CloudSwitch.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
--
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(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c4e6163..e1e2346 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1086,7 +1086,8 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
cpu->env.tsc_khz = value / 1000;
}
-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;
@@ -1241,6 +1242,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=xyz)\n", featurestr);
goto error;
}
+
featurestr = strtok(NULL, ",");
}
x86_cpu_def->features |= plus_features;
@@ -1264,6 +1266,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
error:
g_free(s);
+ if (!error_is_set(errp)) {
+ error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
+ }
return -1;
}
@@ -1350,8 +1355,10 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
memset(def, 0, sizeof(*def));
- 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 = def->vendor1;
env->cpuid_vendor2 = def->vendor2;
@@ -1401,6 +1408,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
env->cpuid_svm_features &= 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;
--
1.7.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-09-26 20:33 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1348688952-5748-1-git-send-email-imammedo@redhat.com>
2012-09-26 19:48 ` [Qemu-devel] [PATCH 01/22] target-i386: return Error from cpu_x86_find_by_name() Igor Mammedov
2012-09-26 20:23 ` Igor Mammedov
2012-09-26 20:32 [Qemu-devel] [PATCH 00/22 v3] target-i386: convert CPU features into properties Igor Mammedov
2012-09-26 20:32 ` [Qemu-devel] [PATCH 01/22] target-i386: return Error from cpu_x86_find_by_name() Igor Mammedov
-- strict thread matches above, loose matches on Subject: below --
2012-09-07 20:54 [Qemu-devel] [PATCH 00/22 v2] target-i386: convert CPU features into properties Igor Mammedov
2012-09-07 20:54 ` [Qemu-devel] [PATCH 01/22] target-i386: return Error from cpu_x86_find_by_name() Igor Mammedov
2012-09-11 19:41 ` Don Slutz
2012-09-13 14:38 ` Eduardo Habkost
2012-09-13 15:49 ` Igor Mammedov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).