Re: [Qemu-devel] [PATCH qom-cpu-next v5] target-i386: Split command line parsing out of cpu_x86_register()
Am 15.02.2013 14:06, schrieb Igor Mammedov: From: Andreas Färber afaer...@suse.de In order to instantiate a CPU subtype we will need to know which type, so move the cpu_model splitting into cpu_x86_init(). Parameters need to be set on the X86CPU instance, so move cpu_x86_parse_featurestr() into cpu_x86_init() as well. This leaves cpu_x86_register() operating on the model name only. Signed-off-by: Andreas Färber afaer...@suse.de Signed-off-by: Igor Mammedov imamm...@redhat.com --- v5: * get error to report from cpu_x86_register() v4: * consolidate resource cleanup in when leaving cpu_x86_init(), to avoid clean code duplication. * remove unnecessary error message from hw/pc.c This version still has the flaw of printing an x86-specific error message in the model-not-found NULL return case, leading to duplicate error messages for qemu-i386 / qemu-x86_64. But I think the progress towards x86 hotplug outweighs that nit, and adding #ifdef TARGET_I386 to linux-user and bsd-user seemed unnecessarily ugly to me. Fixing this (or q35?) can be done as follow-up. Thanks, applied to qom-cpu: https://github.com/afaerber/qemu-cpu/commits/qom-cpu Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH qom-cpu-next v5] target-i386: Split command line parsing out of cpu_x86_register()
From: Andreas Färber afaer...@suse.de In order to instantiate a CPU subtype we will need to know which type, so move the cpu_model splitting into cpu_x86_init(). Parameters need to be set on the X86CPU instance, so move cpu_x86_parse_featurestr() into cpu_x86_init() as well. This leaves cpu_x86_register() operating on the model name only. Signed-off-by: Andreas Färber afaer...@suse.de Signed-off-by: Igor Mammedov imamm...@redhat.com --- v5: * get error to report from cpu_x86_register() v4: * consolidate resource cleanup in when leaving cpu_x86_init(), to avoid clean code duplication. * remove unnecessary error message from hw/pc.c --- hw/pc.c |1 - target-i386/cpu.c | 80 ++-- 2 files changed, 40 insertions(+), 41 deletions(-) diff --git a/hw/pc.c b/hw/pc.c index 53cc173..07caba7 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -876,7 +876,6 @@ void pc_cpus_init(const char *cpu_model) for (i = 0; i smp_cpus; i++) { if (!cpu_x86_init(cpu_model)) { -fprintf(stderr, Unable to find x86 CPU definition\n); exit(1); } } diff --git a/target-i386/cpu.c b/target-i386/cpu.c index dcbed0d..dadb0f0 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1516,27 +1516,16 @@ static void filter_features_for_kvm(X86CPU *cpu) } #endif -static int cpu_x86_register(X86CPU *cpu, const char *cpu_model) +static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp) { CPUX86State *env = cpu-env; x86_def_t def1, *def = def1; -Error *error = NULL; -char *name, *features; -gchar **model_pieces; memset(def, 0, sizeof(*def)); -model_pieces = g_strsplit(cpu_model, ,, 2); -if (!model_pieces[0]) { -error_setg(error, Invalid/empty CPU model name); -goto out; -} -name = model_pieces[0]; -features = model_pieces[1]; - if (cpu_x86_find_by_name(def, name) 0) { -error_setg(error, Unable to find CPU definition: %s, name); -goto out; +error_setg(errp, Unable to find CPU definition: %s, name); +return; } if (kvm_enabled()) { @@ -1544,58 +1533,69 @@ static int cpu_x86_register(X86CPU *cpu, const char *cpu_model) } def-ext_features |= CPUID_EXT_HYPERVISOR; -object_property_set_str(OBJECT(cpu), def-vendor, vendor, error); -object_property_set_int(OBJECT(cpu), def-level, level, error); -object_property_set_int(OBJECT(cpu), def-family, family, error); -object_property_set_int(OBJECT(cpu), def-model, model, error); -object_property_set_int(OBJECT(cpu), def-stepping, stepping, error); +object_property_set_str(OBJECT(cpu), def-vendor, vendor, errp); +object_property_set_int(OBJECT(cpu), def-level, level, errp); +object_property_set_int(OBJECT(cpu), def-family, family, errp); +object_property_set_int(OBJECT(cpu), def-model, model, errp); +object_property_set_int(OBJECT(cpu), def-stepping, stepping, errp); env-cpuid_features = def-features; env-cpuid_ext_features = def-ext_features; env-cpuid_ext2_features = def-ext2_features; env-cpuid_ext3_features = def-ext3_features; -object_property_set_int(OBJECT(cpu), def-xlevel, xlevel, error); +object_property_set_int(OBJECT(cpu), def-xlevel, xlevel, errp); env-cpuid_kvm_features = def-kvm_features; env-cpuid_svm_features = def-svm_features; env-cpuid_ext4_features = def-ext4_features; env-cpuid_7_0_ebx_features = def-cpuid_7_0_ebx_features; env-cpuid_xlevel2 = def-xlevel2; -object_property_set_str(OBJECT(cpu), def-model_id, model-id, error); -if (error) { -goto out; -} - -cpu_x86_parse_featurestr(cpu, features, error); -out: -g_strfreev(model_pieces); -if (error) { -fprintf(stderr, %s\n, error_get_pretty(error)); -error_free(error); -return -1; -} -return 0; +object_property_set_str(OBJECT(cpu), def-model_id, model-id, errp); } X86CPU *cpu_x86_init(const char *cpu_model) { -X86CPU *cpu; +X86CPU *cpu = NULL; CPUX86State *env; +gchar **model_pieces; +char *name, *features; Error *error = NULL; +model_pieces = g_strsplit(cpu_model, ,, 2); +if (!model_pieces[0]) { +error_setg(error, Invalid/empty CPU model name); +goto out; +} +name = model_pieces[0]; +features = model_pieces[1]; + cpu = X86_CPU(object_new(TYPE_X86_CPU)); env = cpu-env; env-cpu_model_str = cpu_model; -if (cpu_x86_register(cpu, cpu_model) 0) { -object_unref(OBJECT(cpu)); -return NULL; +cpu_x86_register(cpu, name, error); +if (error) { +goto out; +} + +cpu_x86_parse_featurestr(cpu, features, error); +if (error) { +goto out; } object_property_set_bool(OBJECT(cpu), true, realized, error); if (error) { +goto out; +} + +out: +g_strfreev(model_pieces);
Re: [Qemu-devel] [PATCH qom-cpu-next v5] target-i386: Split command line parsing out of cpu_x86_register()
On Fri, Feb 15, 2013 at 02:06:56PM +0100, Igor Mammedov wrote: From: Andreas Färber afaer...@suse.de In order to instantiate a CPU subtype we will need to know which type, so move the cpu_model splitting into cpu_x86_init(). Parameters need to be set on the X86CPU instance, so move cpu_x86_parse_featurestr() into cpu_x86_init() as well. This leaves cpu_x86_register() operating on the model name only. Signed-off-by: Andreas Färber afaer...@suse.de Signed-off-by: Igor Mammedov imamm...@redhat.com Reviewed-by: Eduardo Habkost ehabk...@redhat.com --- v5: * get error to report from cpu_x86_register() v4: * consolidate resource cleanup in when leaving cpu_x86_init(), to avoid clean code duplication. * remove unnecessary error message from hw/pc.c --- hw/pc.c |1 - target-i386/cpu.c | 80 ++-- 2 files changed, 40 insertions(+), 41 deletions(-) diff --git a/hw/pc.c b/hw/pc.c index 53cc173..07caba7 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -876,7 +876,6 @@ void pc_cpus_init(const char *cpu_model) for (i = 0; i smp_cpus; i++) { if (!cpu_x86_init(cpu_model)) { -fprintf(stderr, Unable to find x86 CPU definition\n); exit(1); } } diff --git a/target-i386/cpu.c b/target-i386/cpu.c index dcbed0d..dadb0f0 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1516,27 +1516,16 @@ static void filter_features_for_kvm(X86CPU *cpu) } #endif -static int cpu_x86_register(X86CPU *cpu, const char *cpu_model) +static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp) { CPUX86State *env = cpu-env; x86_def_t def1, *def = def1; -Error *error = NULL; -char *name, *features; -gchar **model_pieces; memset(def, 0, sizeof(*def)); -model_pieces = g_strsplit(cpu_model, ,, 2); -if (!model_pieces[0]) { -error_setg(error, Invalid/empty CPU model name); -goto out; -} -name = model_pieces[0]; -features = model_pieces[1]; - if (cpu_x86_find_by_name(def, name) 0) { -error_setg(error, Unable to find CPU definition: %s, name); -goto out; +error_setg(errp, Unable to find CPU definition: %s, name); +return; } if (kvm_enabled()) { @@ -1544,58 +1533,69 @@ static int cpu_x86_register(X86CPU *cpu, const char *cpu_model) } def-ext_features |= CPUID_EXT_HYPERVISOR; -object_property_set_str(OBJECT(cpu), def-vendor, vendor, error); -object_property_set_int(OBJECT(cpu), def-level, level, error); -object_property_set_int(OBJECT(cpu), def-family, family, error); -object_property_set_int(OBJECT(cpu), def-model, model, error); -object_property_set_int(OBJECT(cpu), def-stepping, stepping, error); +object_property_set_str(OBJECT(cpu), def-vendor, vendor, errp); +object_property_set_int(OBJECT(cpu), def-level, level, errp); +object_property_set_int(OBJECT(cpu), def-family, family, errp); +object_property_set_int(OBJECT(cpu), def-model, model, errp); +object_property_set_int(OBJECT(cpu), def-stepping, stepping, errp); env-cpuid_features = def-features; env-cpuid_ext_features = def-ext_features; env-cpuid_ext2_features = def-ext2_features; env-cpuid_ext3_features = def-ext3_features; -object_property_set_int(OBJECT(cpu), def-xlevel, xlevel, error); +object_property_set_int(OBJECT(cpu), def-xlevel, xlevel, errp); env-cpuid_kvm_features = def-kvm_features; env-cpuid_svm_features = def-svm_features; env-cpuid_ext4_features = def-ext4_features; env-cpuid_7_0_ebx_features = def-cpuid_7_0_ebx_features; env-cpuid_xlevel2 = def-xlevel2; -object_property_set_str(OBJECT(cpu), def-model_id, model-id, error); -if (error) { -goto out; -} - -cpu_x86_parse_featurestr(cpu, features, error); -out: -g_strfreev(model_pieces); -if (error) { -fprintf(stderr, %s\n, error_get_pretty(error)); -error_free(error); -return -1; -} -return 0; +object_property_set_str(OBJECT(cpu), def-model_id, model-id, errp); } X86CPU *cpu_x86_init(const char *cpu_model) { -X86CPU *cpu; +X86CPU *cpu = NULL; CPUX86State *env; +gchar **model_pieces; +char *name, *features; Error *error = NULL; +model_pieces = g_strsplit(cpu_model, ,, 2); +if (!model_pieces[0]) { +error_setg(error, Invalid/empty CPU model name); +goto out; +} +name = model_pieces[0]; +features = model_pieces[1]; + cpu = X86_CPU(object_new(TYPE_X86_CPU)); env = cpu-env; env-cpu_model_str = cpu_model; -if (cpu_x86_register(cpu, cpu_model) 0) { -object_unref(OBJECT(cpu)); -return NULL; +cpu_x86_register(cpu, name, error); +if (error) { +goto out;