[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.11] pc: fix crash on attempted cpu unplug
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH for-2.11] pc: fix crash on attempted cpu unplug |
Date: |
Mon, 20 Nov 2017 15:59:51 +0100 |
On Mon, 20 Nov 2017 12:44:54 -0200
Eduardo Habkost <address@hidden> wrote:
> On Mon, Nov 20, 2017 at 03:34:13PM +0100, Igor Mammedov wrote:
> > when qemu is started with '-no-acpi' CLI option, an attempt
> > to unplug a CPU using device_del results in null pointer
> > dereference at:
> >
> > #0 object_get_class
> > #1 pc_machine_device_unplug_request_cb
> > #2 qmp_marshal_device_del
> >
> > which is caused by pcms->acpi_dev == NULL due to ACPI support
> > being disabled.
> >
> > Considering that ACPI support is necessary for unplug to work,
> > check that it's enabled and fail unplug request gracefully
> > if no acpi device were found.
> >
> > Signed-off-by: Igor Mammedov <address@hidden>
>
> Reviewed-by: Eduardo Habkost <address@hidden>
>
> But I have one small suggestion:
>
> > ---
> > hw/i386/pc.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index c3afe5b..d80cec3 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1842,6 +1842,11 @@ static void pc_cpu_unplug_request_cb(HotplugHandler
> > *hotplug_dev,
> > X86CPU *cpu = X86_CPU(dev);
> > PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> >
> > + if (!pcms->acpi_dev) {
> > + error_setg(&local_err, "not supported without acpi");
>
> I suggest "CPU hot unplug not supported without ACPI" instead.
I've thought about it but considering error is issued in context of
device_del command, I've opted for more concise variant.
Would you still like me to respin patch with your suggestion?
>
> > + goto out;
> > + }
> > +
> > pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx);
> > assert(idx != -1);
> > if (idx == 0) {
> > --
> > 2.7.4
> >
>