Re: [Qemu-devel] [PATCH qom-next 09/12] target-i386: make cpu a child of /machine right after it's created

[Igor Mammedov]

Andreas,

Would it be better to squash this patch into
  pc: Add CPU as /machine/cpu[n]
or just leave it as is?

Is there any reason why it shouldn't be done in cpu's initfn?
My concern here is that if it's done outside *_cpu_initfn then
we will have provide 'device_add' with an option
to specify parent for new cpu and Anthony wasn't happy about similar
idea (http://lists.gnu.org/archive/html/qemu-devel/2012-05/msg03481.html).
Maybe we should put cpus under /peripherial instead of /machine after all?

On 05/30/2012 12:10 AM, Igor Mammedov wrote:
> Make cpu child of /machine before any properties are set.
> It is reqired before apic creation is moved to cpu_model property
> setter because setting link to cpu in apic requires cpu to have
> canonical path.
>
> Signed-off-by: Igor Mammedov<address@hidden>
> ---
>   hw/pc.c              |   11 -----------
>   target-i386/helper.c |   15 +++++++++++++++
>   2 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 4a687d6..e38bf24 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -943,8 +943,6 @@ static X86CPU *pc_new_cpu(const char *cpu_model)
>   {
>       X86CPU *cpu;
>       CPUX86State *env;
> -    char *name;
> -    Error *error = NULL;
>
>       cpu = cpu_x86_init(cpu_model);
>       if (cpu == NULL) {
> @@ -952,15 +950,6 @@ static X86CPU *pc_new_cpu(const char *cpu_model)
>       }
>       env =&cpu->env;
>
> -    name = g_strdup_printf("cpu[%d]", env->cpu_index);
> -    object_property_add_child(OBJECT(qdev_get_machine()), name,
> -                              OBJECT(cpu),&error);
> -    g_free(name);
> -    if (error_is_set(&error)) {
> -        qerror_report_err(error);
> -        exit(1);
> -    }
> -
>       if ((env->cpuid_features&  CPUID_APIC) || smp_cpus>  1) {
>           env->apic_state = apic_init(env, env->cpuid_apic_id);
>       }
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 748eee8..8444f2c 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -21,6 +21,7 @@
>   #include "kvm.h"
>   #ifndef CONFIG_USER_ONLY
>   #include "sysemu.h"
> +#include "hw/qdev.h"
>   #include "monitor.h"
>   #endif
>
> @@ -1153,9 +1154,23 @@ X86CPU *cpu_x86_init(const char *cpu_model)
>   {
>       X86CPU *cpu;
>       Error *errp = NULL;
> +#ifndef CONFIG_USER_ONLY
> +    char *name;
> +#endif
>
>       cpu = X86_CPU(object_new(TYPE_X86_CPU));
>
> +#ifndef CONFIG_USER_ONLY
> +    name = g_strdup_printf("cpu[%d]", cpu->env.cpu_index);
> +    object_property_add_child(OBJECT(qdev_get_machine()), name,
> +                              OBJECT(cpu),&errp);
> +    g_free(name);
> +    if (error_is_set(&errp)) {
> +        qerror_report_err(errp);
> +        exit(1);
> +    }
> +#endif
> +
>       if (cpu_model) {
>           object_property_set_str(OBJECT(cpu), cpu_model, "cpu-model",&errp);
>       } else {

--
-----
 Igor