On Fri, 8 Jun 2018 14:48:13 +0200 David Hildenbrand <da...@redhat.com> wrote:
> Let's finish cleaning up the hotplug handler. This check can be > performed in the pre_plug code as the very first thing. > > Signed-off-by: David Hildenbrand <da...@redhat.com> > --- > hw/ppc/spapr.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 1f577b274b..9b8b4068b1 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -3184,12 +3184,18 @@ out: > static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState > *dev, > Error **errp) > { > + const sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(hotplug_dev); Even if const is valid here, it doesn't seem to be widely used in similar situations: $ git grep 'Class.* = .*GET_CLASS' | grep const | wc -l 14 $ git grep 'Class.* = .*GET_CLASS' | grep -v const | wc -l 642 > PCDIMMDevice *dimm = PC_DIMM(dev); > PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); ... like here for example. This inconsistency is a bit weird. The patch is good anyway so: Reviewed-by: Greg Kurz <gr...@kaod.org> > MemoryRegion *mr; > uint64_t size; > char *mem_dev; > > + if (!smc->dr_lmb_enabled) { > + error_setg(errp, "Memory hotplug not supported for this machine"); > + return; > + } > + > mr = ddc->get_memory_region(dimm, errp); > if (!mr) { > return; > @@ -3571,14 +3577,7 @@ out: > static void spapr_machine_device_plug(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp) > { > - MachineState *ms = MACHINE(hotplug_dev); > - sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(ms); > - > if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > - if (!smc->dr_lmb_enabled) { > - error_setg(errp, "Memory hotplug not supported for this > machine"); > - return; > - } > spapr_memory_plug(hotplug_dev, dev, errp); > } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) { > spapr_core_plug(hotplug_dev, dev, errp);