On Tue, 28 Nov 2017 12:28:43 +0100
Paolo Bonzini <pbonz...@redhat.com> wrote:
> On 20/11/2017 18:05, Eduardo Habkost wrote:
> > On Mon, Nov 20, 2017 at 03:59:51PM +0100, Igor Mammedov wrote:
> >> On Mon, 20 Nov 2017 12:44:54 -0200
> >> Eduardo Habkost <ehabk...@redhat.com> 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 <imamm...@redhat.com>
> >>>
> >>> Reviewed-by: Eduardo Habkost <ehabk...@redhat.com>
> >>>
> >>> 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?
> >
> > I'd prefer to, so the error message would make sense even if out
> > of context. But you still have my Reviewed-by in case Michael
> > wants to apply this version.
>
> I made the change and queued the patch.
I've thought Michael (CCed) queued v2
but he hasn't sent a pull req yet.
> Paolo
