All users are passing &error_abort already. Document the fact
that spapr_drc_attach() should only be passed a free DRC, which
is supposedly the case if appropriate checking is done earlier.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 include/hw/ppc/spapr_drc.h | 8 +++++++-
 hw/ppc/spapr.c             | 6 +++---
 hw/ppc/spapr_drc.c         | 8 ++------
 hw/ppc/spapr_nvdimm.c      | 2 +-
 hw/ppc/spapr_pci.c         | 2 +-
 5 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 165b281496bb..def3593adc8b 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -235,7 +235,13 @@ SpaprDrc *spapr_drc_by_index(uint32_t index);
 SpaprDrc *spapr_drc_by_id(const char *type, uint32_t id);
 int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t drc_type_mask);
 
-bool spapr_drc_attach(SpaprDrc *drc, DeviceState *d, Error **errp);
+/*
+ * These functions respectively abort if called with a device already
+ * attached or no device attached. In the case of spapr_drc_attach(),
+ * this means that the attachability of the DRC *must* be checked
+ * beforehand (eg. check drc->dev at pre-plug).
+ */
+void spapr_drc_attach(SpaprDrc *drc, DeviceState *d);
 void spapr_drc_detach(SpaprDrc *drc);
 
 /* Returns true if a hot plug/unplug request is pending */
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5e32d1d396b4..e0047f41073e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3399,7 +3399,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
          * that doesn't overlap with any existing mapping at pre-plug. The
          * corresponding LMB DRCs are thus assumed to be all attachable.
          */
-        spapr_drc_attach(drc, dev, &error_abort);
+        spapr_drc_attach(drc, dev);
         if (!hotplugged) {
             spapr_drc_reset(drc);
         }
@@ -3767,7 +3767,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev)
          * core being plugged into a free slot. Nothing should already
          * be attached to the corresponding DRC.
          */
-        spapr_drc_attach(drc, dev, &error_abort);
+        spapr_drc_attach(drc, dev);
 
         if (hotplugged) {
             /*
@@ -3920,7 +3920,7 @@ static void spapr_phb_plug(HotplugHandler *hotplug_dev, DeviceState *dev)
     assert(drc);
 
     /* spapr_phb_pre_plug() already checked the DRC is attachable */
-    spapr_drc_attach(drc, dev, &error_abort);
+    spapr_drc_attach(drc, dev);
 
     if (hotplugged) {
         spapr_hotplug_req_add_by_index(drc);
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 77718cde1ff2..f991cf89a08a 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -369,14 +369,11 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
     } while (fdt_depth != 0);
 }
 
-bool spapr_drc_attach(SpaprDrc *drc, DeviceState *d, Error **errp)
+void spapr_drc_attach(SpaprDrc *drc, DeviceState *d)
 {
     trace_spapr_drc_attach(spapr_drc_index(drc));
 
-    if (drc->dev) {
-        error_setg(errp, "an attached device is still awaiting release");
-        return false;
-    }
+    g_assert(!drc->dev);
     g_assert((drc->state == SPAPR_DRC_STATE_LOGICAL_UNUSABLE)
              || (drc->state == SPAPR_DRC_STATE_PHYSICAL_POWERON));
 
@@ -386,7 +383,6 @@ bool spapr_drc_attach(SpaprDrc *drc, DeviceState *d, Error **errp)
                              object_get_typename(OBJECT(drc->dev)),
                              (Object **)(&drc->dev),
                              NULL, 0);
-    return true;
 }
 
 static void spapr_drc_release(SpaprDrc *drc)
diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index 2f1c196e1b76..73ee006541a6 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -101,7 +101,7 @@ void spapr_add_nvdimm(DeviceState *dev, uint64_t slot)
      * pc_dimm_get_free_slot() provided a free slot at pre-plug. The
      * corresponding DRC is thus assumed to be attachable.
      */
-    spapr_drc_attach(drc, dev, &error_abort);
+    spapr_drc_attach(drc, dev);
 
     if (hotplugged) {
         spapr_hotplug_req_add_by_index(drc);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 2829f298d9c1..e946bd5055cc 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1601,7 +1601,7 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
     }
 
     /* spapr_pci_pre_plug() already checked the DRC is attachable */
-    spapr_drc_attach(drc, DEVICE(pdev), &error_abort);
+    spapr_drc_attach(drc, DEVICE(pdev));
 
     /* If this is function 0, signal hotplug for all the device functions.
      * Otherwise defer sending the hotplug event.
-- 
2.26.2

On Sat, Nov 21, 2020 at 12:42:08AM +0100, Greg Kurz wrote:
> All users are passing &error_abort already. Document the fact
> that spapr_drc_attach() should only be passed a free DRC, which
> is supposedly the case if appropriate checking is done earlier.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-6.0, thanks.

> ---
>  include/hw/ppc/spapr_drc.h | 8 +++++++-
>  hw/ppc/spapr.c             | 6 +++---
>  hw/ppc/spapr_drc.c         | 8 ++------
>  hw/ppc/spapr_nvdimm.c      | 2 +-
>  hw/ppc/spapr_pci.c         | 2 +-
>  5 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index 165b281496bb..def3593adc8b 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -235,7 +235,13 @@ SpaprDrc *spapr_drc_by_index(uint32_t index);
>  SpaprDrc *spapr_drc_by_id(const char *type, uint32_t id);
>  int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t drc_type_mask);
>  
> -bool spapr_drc_attach(SpaprDrc *drc, DeviceState *d, Error **errp);
> +/*
> + * These functions respectively abort if called with a device already
> + * attached or no device attached. In the case of spapr_drc_attach(),
> + * this means that the attachability of the DRC *must* be checked
> + * beforehand (eg. check drc->dev at pre-plug).
> + */
> +void spapr_drc_attach(SpaprDrc *drc, DeviceState *d);
>  void spapr_drc_detach(SpaprDrc *drc);
>  
>  /* Returns true if a hot plug/unplug request is pending */
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5e32d1d396b4..e0047f41073e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3399,7 +3399,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
>           * that doesn't overlap with any existing mapping at pre-plug. The
>           * corresponding LMB DRCs are thus assumed to be all attachable.
>           */
> -        spapr_drc_attach(drc, dev, &error_abort);
> +        spapr_drc_attach(drc, dev);
>          if (!hotplugged) {
>              spapr_drc_reset(drc);
>          }
> @@ -3767,7 +3767,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev)
>           * core being plugged into a free slot. Nothing should already
>           * be attached to the corresponding DRC.
>           */
> -        spapr_drc_attach(drc, dev, &error_abort);
> +        spapr_drc_attach(drc, dev);
>  
>          if (hotplugged) {
>              /*
> @@ -3920,7 +3920,7 @@ static void spapr_phb_plug(HotplugHandler *hotplug_dev, DeviceState *dev)
>      assert(drc);
>  
>      /* spapr_phb_pre_plug() already checked the DRC is attachable */
> -    spapr_drc_attach(drc, dev, &error_abort);
> +    spapr_drc_attach(drc, dev);
>  
>      if (hotplugged) {
>          spapr_hotplug_req_add_by_index(drc);
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 77718cde1ff2..f991cf89a08a 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -369,14 +369,11 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
>      } while (fdt_depth != 0);
>  }
>  
> -bool spapr_drc_attach(SpaprDrc *drc, DeviceState *d, Error **errp)
> +void spapr_drc_attach(SpaprDrc *drc, DeviceState *d)
>  {
>      trace_spapr_drc_attach(spapr_drc_index(drc));
>  
> -    if (drc->dev) {
> -        error_setg(errp, "an attached device is still awaiting release");
> -        return false;
> -    }
> +    g_assert(!drc->dev);
>      g_assert((drc->state == SPAPR_DRC_STATE_LOGICAL_UNUSABLE)
>               || (drc->state == SPAPR_DRC_STATE_PHYSICAL_POWERON));
>  
> @@ -386,7 +383,6 @@ bool spapr_drc_attach(SpaprDrc *drc, DeviceState *d, Error **errp)
>                               object_get_typename(OBJECT(drc->dev)),
>                               (Object **)(&drc->dev),
>                               NULL, 0);
> -    return true;
>  }
>  
>  static void spapr_drc_release(SpaprDrc *drc)
> diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
> index 2f1c196e1b76..73ee006541a6 100644
> --- a/hw/ppc/spapr_nvdimm.c
> +++ b/hw/ppc/spapr_nvdimm.c
> @@ -101,7 +101,7 @@ void spapr_add_nvdimm(DeviceState *dev, uint64_t slot)
>       * pc_dimm_get_free_slot() provided a free slot at pre-plug. The
>       * corresponding DRC is thus assumed to be attachable.
>       */
> -    spapr_drc_attach(drc, dev, &error_abort);
> +    spapr_drc_attach(drc, dev);
>  
>      if (hotplugged) {
>          spapr_hotplug_req_add_by_index(drc);
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 2829f298d9c1..e946bd5055cc 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1601,7 +1601,7 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
>      }
>  
>      /* spapr_pci_pre_plug() already checked the DRC is attachable */
> -    spapr_drc_attach(drc, DEVICE(pdev), &error_abort);
> +    spapr_drc_attach(drc, DEVICE(pdev));
>  
>      /* If this is function 0, signal hotplug for all the device functions.
>       * Otherwise defer sending the hotplug event.

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