[PATCH] spapr: Assert DIMM unplug state in spapr_memory_unplug()

[PATCH] spapr: Assert DIMM unplug state in spapr_memory_unplug()

[Greg Kurz]

spapr_memory_unplug() is the last step of the hot unplug sequence.
It is indirectly called by:

 spapr_lmb_release()
  hotplug_handler_unplug()

and spapr_lmb_release() already buys us that DIMM unplug state is
present : it gets restored with spapr_recover_pending_dimm_state()
if missing.

g_assert() that spapr_pending_dimm_unplugs_find() cannot return NULL
in spapr_memory_unplug() to make this clear and silence Coverity.

Fixes: Coverity CID 1450767
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d56418ca2942..73a06df3b1b1 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3660,6 +3660,9 @@ static void spapr_memory_unplug(HotplugHandler *hotplug_dev, DeviceState *dev)
     SpaprMachineState *spapr = SPAPR_MACHINE(hotplug_dev);
     SpaprDimmState *ds = spapr_pending_dimm_unplugs_find(spapr, PC_DIMM(dev));
 
+    /* We really shouldn't get this far without anything to unplug */
+    g_assert(ds);
+
     pc_dimm_unplug(PC_DIMM(dev), MACHINE(hotplug_dev));
     qdev_unrealize(dev);
     spapr_pending_dimm_unplugs_remove(spapr, ds);

Re: [PATCH] spapr: Assert DIMM unplug state in spapr_memory_unplug()

[Daniel Henrique Barboza]

On 3/13/21 4:23 AM, Greg Kurz wrote:
> spapr_memory_unplug() is the last step of the hot unplug sequence.
> It is indirectly called by:
> 
>   spapr_lmb_release()
>    hotplug_handler_unplug()
> 
> and spapr_lmb_release() already buys us that DIMM unplug state is
> present : it gets restored with spapr_recover_pending_dimm_state()
> if missing.
> 
> g_assert() that spapr_pending_dimm_unplugs_find() cannot return NULL
> in spapr_memory_unplug() to make this clear and silence Coverity.
> 
> Fixes: Coverity CID 1450767
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   hw/ppc/spapr.c |    3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d56418ca2942..73a06df3b1b1 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3660,6 +3660,9 @@ static void spapr_memory_unplug(HotplugHandler *hotplug_dev, DeviceState *dev)
>       SpaprMachineState *spapr = SPAPR_MACHINE(hotplug_dev);
>       SpaprDimmState *ds = spapr_pending_dimm_unplugs_find(spapr, PC_DIMM(dev));
>   
> +    /* We really shouldn't get this far without anything to unplug */
> +    g_assert(ds);
> +
>       pc_dimm_unplug(PC_DIMM(dev), MACHINE(hotplug_dev));
>       qdev_unrealize(dev);
>       spapr_pending_dimm_unplugs_remove(spapr, ds);
> 
> 

Re: [PATCH] spapr: Assert DIMM unplug state in spapr_memory_unplug()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1558 bytes --]

On Sat, Mar 13, 2021 at 08:23:31AM +0100, Greg Kurz wrote:
> spapr_memory_unplug() is the last step of the hot unplug sequence.
> It is indirectly called by:
> 
>  spapr_lmb_release()
>   hotplug_handler_unplug()
> 
> and spapr_lmb_release() already buys us that DIMM unplug state is
> present : it gets restored with spapr_recover_pending_dimm_state()
> if missing.
> 
> g_assert() that spapr_pending_dimm_unplugs_find() cannot return NULL
> in spapr_memory_unplug() to make this clear and silence Coverity.
> 
> Fixes: Coverity CID 1450767
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-6.0, thanks.

> ---
>  hw/ppc/spapr.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d56418ca2942..73a06df3b1b1 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3660,6 +3660,9 @@ static void spapr_memory_unplug(HotplugHandler *hotplug_dev, DeviceState *dev)
>      SpaprMachineState *spapr = SPAPR_MACHINE(hotplug_dev);
>      SpaprDimmState *ds = spapr_pending_dimm_unplugs_find(spapr, PC_DIMM(dev));
>  
> +    /* We really shouldn't get this far without anything to unplug */
> +    g_assert(ds);
> +
>      pc_dimm_unplug(PC_DIMM(dev), MACHINE(hotplug_dev));
>      qdev_unrealize(dev);
>      spapr_pending_dimm_unplugs_remove(spapr, ds);
> 
> 

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]