On 14.08.2017 05:24, David Gibson wrote:
> On Sat, Aug 12, 2017 at 12:16:51PM +0200, Greg Kurz wrote:
>> On Sat, 12 Aug 2017 10:38:10 +0200
>> Thomas Huth <th...@redhat.com> wrote:
>>
>>> QEMU currently crashes when the user tries to add a spapr-cpu-core
>>> on a non-pseries machine:
>>>
>>> $ qemu-system-ppc64 -S -machine ppce500,accel=tcg \
>>> -device POWER5+_v2.1-spapr-cpu-core
>>> hw/ppc/spapr_cpu_core.c:178:spapr_cpu_core_realize_child:
>>> Object 0x55cee1f55160 is not an instance of type spapr-machine
>>> Aborted (core dumped)
>>>
>>> So let's add a proper check for the correct machine time with
>>> a more friendly error message here.
>>>
>>> Reported-by: Eduardo Habkost <ehabk...@redhat.com>
>>> Signed-off-by: Thomas Huth <th...@redhat.com>
>>> ---
>>> hw/ppc/spapr_cpu_core.c | 9 ++++++++-
>>> scripts/device-crash-test | 1 +
>>> 2 files changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
>>> index ea278ce..0f3d653 100644
>>> --- a/hw/ppc/spapr_cpu_core.c
>>> +++ b/hw/ppc/spapr_cpu_core.c
>>> @@ -175,11 +175,18 @@ static void spapr_cpu_core_unrealizefn(DeviceState
>>> *dev, Error **errp)
>>> static void spapr_cpu_core_realize_child(Object *child, Error **errp)
>>> {
>>> Error *local_err = NULL;
>>> - sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>>> + sPAPRMachineState *spapr;
>>> CPUState *cs = CPU(child);
>>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>>> Object *obj;
>>>
>>> + spapr = (sPAPRMachineState *)object_dynamic_cast(qdev_get_machine(),
>>> + TYPE_SPAPR_MACHINE);
>>> + if (!spapr) {
>>> + error_setg(errp, "spapr-cpu-core needs a pseries machine");
>>> + return;
>>> + }
>>> +
>>
>> This is the realize function for individual threads. Maybe this sanity check
>> should be performed earlier at the core level in spapr_cpu_core_realize() ?
>
> Ah, yes, that would be a better way of doing it.
>
> I'm also not clear if you're proposing this for 2.10 or 2.11.I was not sure, either ;-) Anyway, it's not a bug that blocks normal usage of QEMU, and apparently there's a better but more extensive way to fix this, so let's postpone this to 2.11. Thomas
signature.asc
Description: OpenPGP digital signature
