| Message ID | 1442605096-14103-5-git-send-email-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Sep 19, 2015 at 1:08 AM, Eduardo Habkost <ehabkost@redhat.com> wrote: > QOM instance_init functions are not supposed to have any side-effects, > as new objects may be created at any moment for querying property > information (see qmp_device_list_properties()). > > Calling cpu_exec_init() also affects QEMU's ability to handle errors > during CPU creation, as some actions done by cpu_exec_init() can't be > reverted. > > Move cpu_exec_init() call to realize so a simple object_new() won't > trigger it, and so that it is called after some basic validation of CPU > parameters. Since you are moving cpu_exec_init() to realize, does it make sense to define unrealize and call cpu_exec_exit() from it ? Regards, Bharata.
On Mon, Sep 21, 2015 at 11:11:47AM +0530, Bharata B Rao wrote: > On Sat, Sep 19, 2015 at 1:08 AM, Eduardo Habkost <ehabkost@redhat.com> wrote: > > QOM instance_init functions are not supposed to have any side-effects, > > as new objects may be created at any moment for querying property > > information (see qmp_device_list_properties()). > > > > Calling cpu_exec_init() also affects QEMU's ability to handle errors > > during CPU creation, as some actions done by cpu_exec_init() can't be > > reverted. > > > > Move cpu_exec_init() call to realize so a simple object_new() won't > > trigger it, and so that it is called after some basic validation of CPU > > parameters. > > Since you are moving cpu_exec_init() to realize, does it make sense to > define unrealize and call cpu_exec_exit() from it ? It does make sense. But it needs to be done more carefully because currently cpu_exec_exit() is likely to make QEMU crash, and calling it from unrealize would make the crash triggerable using a QMP qom-set command.
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 2e5a303..19d1525 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2832,6 +2832,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) goto out; } + cpu_exec_init(cs, &error_abort); + if (tcg_enabled()) { tcg_x86_init(); } @@ -3027,7 +3029,6 @@ static void x86_cpu_initfn(Object *obj) FeatureWord w; cs->env_ptr = env; - cpu_exec_init(cs, &error_abort); object_property_add(obj, "family", "int", x86_cpuid_version_get_family,
QOM instance_init functions are not supposed to have any side-effects, as new objects may be created at any moment for querying property information (see qmp_device_list_properties()). Calling cpu_exec_init() also affects QEMU's ability to handle errors during CPU creation, as some actions done by cpu_exec_init() can't be reverted. Move cpu_exec_init() call to realize so a simple object_new() won't trigger it, and so that it is called after some basic validation of CPU parameters. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v1 -> v2: * Call cpu_exec_init() after basic parameter validation --- target-i386/cpu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)