Re: [Qemu-devel] [PATCH 1/1] target-i386: Clean up misuse of qdev_init() in realize method

[Igor Mammedov]

On Thu,  5 Feb 2015 10:29:15 +0100
Markus Armbruster <address@hidden> wrote:

> x86_cpu_apic_realize() calls qdev_init() to realize the APIC.
> qdev_init()'s error handling has unwanted side effects: it unparents
> the device, and it calls qerror_report_err().
> 
> qerror_report_err() is always inappropriate in realize methods,
> because it doesn't return the Error object.  It either reports the
> error to stderr or the human monitor, or it stores it in the QMP
> monitor, where it makes the QMP command fail even though the realize
> method succeeded.
> 
> Fortunately, qdev_init() can't actually fail here, because realize
> can't fail for any of the three possible APIC device models.
> 
> Clean up by cutting out the qdev_init() middle-man: set property
> "realized" directly.
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  target-i386/cpu.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 3a9b32e..2a82407 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2752,12 +2752,8 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error 
> **errp)
>      if (cpu->apic_state == NULL) {
>          return;
>      }
> -
> -    if (qdev_init(cpu->apic_state)) {
> -        error_setg(errp, "APIC device '%s' could not be initialized",
> -                   object_get_typename(OBJECT(cpu->apic_state)));
> -        return;
> -    }
> +    object_property_set_bool(OBJECT(cpu->apic_state), true, "realized",
> +                             errp);
>  }
>  #else
>  static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)

Reviewed-by: Igor Mammedov <address@hidden>