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

2012-05-30 Thread 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 Mammedovimamm...@redhat.com
---
  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



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

2012-05-29 Thread Igor Mammedov
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 imamm...@redhat.com
---
 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 {
-- 
1.7.7.6