[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/1] target-i386: Clean up misuse of qdev_init()
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH 1/1] target-i386: Clean up misuse of qdev_init() in realize method |
Date: |
Thu, 5 Feb 2015 13:51:14 +0100 |
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>