spapr_cpu_core: add missing rollback on realization path

The spapr_realize_vcpu() function doesn't rollback in case of error.
This isn't a problem with coldplugged CPUs because the machine won't
start and QEMU will exit. Hotplug is a different story though: the
CPU thread is started under object_property_set_bool() and it assumes
it can access the CPU object.

If icp_create() fails, we return an error without unregistering the
reset handler for this CPU, and we let the underlying QEMU thread for
this CPU alive. Since spapr_cpu_core_realize() doesn't care to unrealize
already realized CPUs either, but happily frees all of them anyway, the
CPU thread crashes instantly:

(qemu) device_add host-spapr-cpu-core,core-id=1,id=gku
GKU: failing icp_create (cpu 0x11497fd0)
                             ^^^^^^^^^^
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffee3feaa0 (LWP 24725)]
0x00000000104c8374 in object_dynamic_cast_assert (obj=0x11497fd0,
                                                  ^^^^^^^^^^^^^^
                                             pointer to the CPU object
623         trace_object_dynamic_cast_assert(obj ? obj->class->type->name
(gdb) p obj->class->type
$1 = (Type) 0x0
(gdb) p * obj
$2 = {class = 0x10ea9c10, free = 0x11244620,
                                 ^^^^^^^^^^
                              should be g_free
(gdb) p g_free
$3 = {<text variable, no debug info>} 0x7ffff282bef0 <g_free>

obj is a dangling pointer to the CPU that was just destroyed in
spapr_cpu_core_realize().

This patch adds proper rollback to both spapr_realize_vcpu() and
spapr_cpu_core_realize().

Signed-off-by: Greg Kurz <groug@kaod.org>
[dwg: Fixed a conflict due to a change in my tree]
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This commit is contained in:
Greg Kurz 2018-06-14 23:50:42 +02:00 committed by David Gibson
parent 27607c1cdc
commit 9986ddec4c

View File

@ -145,11 +145,14 @@ static void spapr_realize_vcpu(PowerPCCPU *cpu, sPAPRMachineState *spapr,
cpu->intc = icp_create(OBJECT(cpu), spapr->icp_type, XICS_FABRIC(spapr), cpu->intc = icp_create(OBJECT(cpu), spapr->icp_type, XICS_FABRIC(spapr),
&local_err); &local_err);
if (local_err) { if (local_err) {
goto error; goto error_unregister;
} }
return; return;
error_unregister:
qemu_unregister_reset(spapr_cpu_reset, cpu);
cpu_remove_sync(CPU(cpu));
error: error:
error_propagate(errp, local_err); error_propagate(errp, local_err);
} }
@ -208,11 +211,15 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
for (j = 0; j < cc->nr_threads; j++) { for (j = 0; j < cc->nr_threads; j++) {
spapr_realize_vcpu(sc->threads[j], spapr, &local_err); spapr_realize_vcpu(sc->threads[j], spapr, &local_err);
if (local_err) { if (local_err) {
goto err; goto err_unrealize;
} }
} }
return; return;
err_unrealize:
while (--j >= 0) {
spapr_unrealize_vcpu(sc->threads[j]);
}
err: err:
while (--i >= 0) { while (--i >= 0) {
obj = OBJECT(sc->threads[i]); obj = OBJECT(sc->threads[i]);