On 08/24/2018 05:09 PM, Peter Maydell wrote: > On 21 August 2018 at 05:33, David Gibson <da...@gibson.dropbear.id.au> wrote: >> From: Cédric Le Goater <c...@kaod.org> >> >> It should save us some CPU cycles as these routines perform a lot of >> checks. >> >> Signed-off-by: Cédric Le Goater <c...@kaod.org> >> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> >> --- >> hw/ppc/spapr_pci.c | 11 ++++++----- >> 1 file changed, 6 insertions(+), 5 deletions(-) > > Hi; Coverity points out in CID 1395183 that there's a bug in > this part of this patch: > >> @@ -1558,6 +1559,7 @@ static void spapr_phb_realize(DeviceState *dev, Error >> **errp) >> sPAPRMachineState *spapr = >> (sPAPRMachineState *) object_dynamic_cast(qdev_get_machine(), >> TYPE_SPAPR_MACHINE); >> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); > > This has moved the call to SPAPR_MACHINE_GET_CLASS() above > the check for "is spapr NULL", which is wrong, because it > will unconditionally dereference the pointer you pass to it.
I see. This is a simple fix but the root reason for this check is commit f7d6bfcdc0fe ("spapr_pci: fail gracefully with non-pseries machine types"). Is there a way to specify which device type can or can not be plugged on a machine ? I suppose we cannot use : machine_class_allow_dynamic_sysbus_dev() for cold plugged devices. Or can we ? That would be better. Thanks, C.