| Message ID | 20210114180628.1675603-7-danielhb413@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | pseries: avoid unplug the last online CPU core + assorted fixes | expand |
On Thu, Jan 14, 2021 at 03:06:27PM -0300, Daniel Henrique Barboza wrote: > Next patch is going to add more conditions to allow a CPU core > hotunplug. Let's put it into a separated function to avoid crowding > the body of spapr_core_unplug_request(). > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> LGTM, except for naming. I'd expect a function named *_possible() to return a boolean, where "true" means it *is* possible, rather than a 0-or-negative-error value. So you could either change the type and sense of the function, or change the name to, say spapr_core_unplug_check(), which I would expect to return an error code. > --- > hw/ppc/spapr.c | 27 ++++++++++++++++++++------- > 1 file changed, 20 insertions(+), 7 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 2c403b574e..a2f01c21aa 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -3706,22 +3706,35 @@ static void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *dev) > qdev_unrealize(dev); > } > > -static > -void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev, > - Error **errp) > +static int spapr_core_unplug_possible(HotplugHandler *hotplug_dev, CPUCore *cc, > + Error **errp) > { > - SpaprMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev)); > int index; > - SpaprDrc *drc; > - CPUCore *cc = CPU_CORE(dev); > > if (!spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index)) { > error_setg(errp, "Unable to find CPU core with core-id: %d", > cc->core_id); > - return; > + return -1; > } > + > if (index == 0) { > error_setg(errp, "Boot CPU core may not be unplugged"); > + return -1; > + } > + > + return 0; > +} > + > +static > +void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev, > + Error **errp) > +{ > + ERRP_GUARD(); > + SpaprMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev)); > + SpaprDrc *drc; > + CPUCore *cc = CPU_CORE(dev); > + > + if (spapr_core_unplug_possible(hotplug_dev, cc, errp) < 0) { > return; > } >
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 2c403b574e..a2f01c21aa 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3706,22 +3706,35 @@ static void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *dev) qdev_unrealize(dev); } -static -void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev, - Error **errp) +static int spapr_core_unplug_possible(HotplugHandler *hotplug_dev, CPUCore *cc, + Error **errp) { - SpaprMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev)); int index; - SpaprDrc *drc; - CPUCore *cc = CPU_CORE(dev); if (!spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index)) { error_setg(errp, "Unable to find CPU core with core-id: %d", cc->core_id); - return; + return -1; } + if (index == 0) { error_setg(errp, "Boot CPU core may not be unplugged"); + return -1; + } + + return 0; +} + +static +void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev, + Error **errp) +{ + ERRP_GUARD(); + SpaprMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev)); + SpaprDrc *drc; + CPUCore *cc = CPU_CORE(dev); + + if (spapr_core_unplug_possible(hotplug_dev, cc, errp) < 0) { return; }
Next patch is going to add more conditions to allow a CPU core hotunplug. Let's put it into a separated function to avoid crowding the body of spapr_core_unplug_request(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/ppc/spapr.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-)