[Qemu-devel] [PATCH 04/15] spapr_pci: add proper rollback on PHB realize error path

[Greg Kurz]

The current realize code assumes the PHB is coldplugged, ie, QEMU will
terminate if an error is detected, and does not bother to free anything
it has already allocated.

In order to support PHB hotplug, let's first ensure spapr_phb_realize()
doesn't leak anything in case of error.

Signed-off-by: Greg Kurz <address@hidden>
---
 hw/ppc/spapr_pci.c |   40 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index e59adbe706bb..46d7062dd143 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1570,6 +1570,7 @@ static void spapr_phb_realize(DeviceState *dev, Error 
**errp)
     sPAPRTCETable *tcet;
     const unsigned windows_supported =
         sphb->ddw_enabled ? SPAPR_PCI_DMA_MAX_WINDOWS : 1;
+    Object *drcs[PCI_SLOT_MAX * 8];
 
     if (!spapr) {
         error_setg(errp, TYPE_SPAPR_PCI_HOST_BRIDGE " needs a pseries 
machine");
@@ -1733,7 +1734,10 @@ static void spapr_phb_realize(DeviceState *dev, Error 
**errp)
         spapr_irq_claim(spapr, irq, true, &local_err);
         if (local_err) {
             error_propagate_prepend(errp, local_err, "can't allocate LSIs: ");
-            return;
+            while (--i >= 0) {
+                spapr_irq_free(spapr, sphb->lsi_table[i].irq, 1);
+            }
+            goto fail_del_msiwindow;
         }
 
         sphb->lsi_table[i].irq = irq;
@@ -1741,9 +1745,10 @@ static void spapr_phb_realize(DeviceState *dev, Error 
**errp)
 
     /* allocate connectors for child PCI devices */
     if (sphb->dr_enabled) {
-        for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
-            spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
-                                   (sphb->index << 16) | i);
+        for (i = 0; i < ARRAY_SIZE(drcs); i++) {
+            drcs[i] =
+                OBJECT(spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
+                                              (sphb->index << 16) | i));
         }
     }
 
@@ -1753,13 +1758,38 @@ static void spapr_phb_realize(DeviceState *dev, Error 
**errp)
         if (!tcet) {
             error_setg(errp, "Creating window#%d failed for %s",
                        i, sphb->dtbusname);
-            return;
+            while (--i >= 0) {
+                tcet = spapr_tce_find_by_liobn(sphb->dma_liobn[i]);
+                assert(tcet);
+                memory_region_del_subregion(&sphb->iommu_root,
+                                            spapr_tce_get_iommu(tcet));
+                object_unparent(OBJECT(tcet));
+            }
+            goto fail_free_drcs;
         }
         memory_region_add_subregion(&sphb->iommu_root, 0,
                                     spapr_tce_get_iommu(tcet));
     }
 
     sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
+    return;
+
+fail_free_drcs:
+    if (sphb->dr_enabled) {
+        for (i = 0; i < ARRAY_SIZE(drcs); i++) {
+            object_unparent(drcs[i]);
+        }
+    }
+fail_del_msiwindow:
+    memory_region_del_subregion(&sphb->iommu_root, &sphb->msiwindow);
+    address_space_destroy(&sphb->iommu_as);
+    qbus_set_hotplug_handler(BUS(phb->bus), NULL, &error_abort);
+    pci_unregister_root_bus(phb->bus);
+    memory_region_del_subregion(get_system_memory(), &sphb->iowindow);
+    if (sphb->mem64_win_pciaddr != (hwaddr)-1) {
+        memory_region_del_subregion(get_system_memory(), &sphb->mem64window);
+    }
+    memory_region_del_subregion(get_system_memory(), &sphb->mem32window);
 }
 
 static int spapr_phb_children_reset(Object *child, void *opaque)