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. > Also, spapr_cpu_core_realize_child() seem to only have spapr to pass it down > to spapr_cpu_init()... ie, not even sure spapr_cpu_core_realize_child() > actually needs a spapr variable. > > > object_property_set_bool(child, true, "realized", &local_err); > > if (local_err) { > > goto error; > > diff --git a/scripts/device-crash-test b/scripts/device-crash-test > > index e77b693..8eb2d02 100755 > > --- a/scripts/device-crash-test > > +++ b/scripts/device-crash-test > > @@ -200,6 +200,7 @@ ERROR_WHITELIST = [ > > {'log':r"Multiple VT220 operator consoles are not supported"}, > > {'log':r"core 0 already populated"}, > > {'log':r"could not find stage1 bootloader"}, > > + {'log':r"spapr-cpu-core needs a pseries machine"}, > > > > # other exitcode=1 failures not listed above will just generate INFO > > messages: > > {'exitcode':1, 'loglevel':logging.INFO}, > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature