On 07/20/2017 01:11 AM, Bharata B Rao wrote:
Commit 0cffce56 (hw/ppc/spapr.c: adding pending_dimm_unplugs to
sPAPRMachineState) introduced a new way to track pending LMBs of DIMM
device that is marked for removal. Since this commit we can hit the
assert in spapr_pending_dimm_unplugs_add() in the following situation:
- DIMM device removal fails as the guest doesn't allow the removal.
- Subsequent attempt to remove the same DIMM would hit the assert
as the corresponding sPAPRDIMMState is still part of the
pending_dimm_unplugs list.
Fix this by removing the assert and conditionally adding the
sPAPRDIMMState to pending_dimm_unplugs list only when it is not
already present.
Fixes: 0cffce56ae3501c5783d779f97993ce478acf856
Signed-off-by: Bharata B Rao <[email protected]>
---
Changes in v1:
- Added comment (David Gibson)
- Ensured we free sPAPRDIMMState when corresonding entry already
exists (Daniel Henrique Barboza)
Daniel had shown another alternative, we can switch over to that
if preferred.
Reviewed-by: Daniel Barboza <[email protected]>
hw/ppc/spapr.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 1cb09e7..c6091e2 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2853,8 +2853,17 @@ static sPAPRDIMMState
*spapr_pending_dimm_unplugs_find(sPAPRMachineState *s,
static void spapr_pending_dimm_unplugs_add(sPAPRMachineState *spapr,
sPAPRDIMMState *dimm_state)
{
- g_assert(!spapr_pending_dimm_unplugs_find(spapr, dimm_state->dimm));
- QTAILQ_INSERT_HEAD(&spapr->pending_dimm_unplugs, dimm_state, next);
+ /*
+ * If this request is for a DIMM whose removal had failed earlier
+ * (due to guest's refusal to remove the LMBs), we would have this
+ * dimm_state already in the pending_dimm_unplugs list. In that
+ * case don't add again.
+ */
+ if (!spapr_pending_dimm_unplugs_find(spapr, dimm_state->dimm)) {
+ QTAILQ_INSERT_HEAD(&spapr->pending_dimm_unplugs, dimm_state, next);
+ } else {
+ g_free(dimm_state);
+ }
}
static void spapr_pending_dimm_unplugs_remove(sPAPRMachineState *spapr,
