Re: [Qemu-devel] [PATCH v2 3/3] pc-dimm: Add Error argument to pc_existing_dimms_capacity

[Igor Mammedov]

On Tue, 27 Jan 2015 09:35:02 +0530
Bharata B Rao <address@hidden> wrote:

> Now that pc_existing_dimms_capacity() is an API, include Error pointer
> as an argument and modify the caller appropriately.
> 
> Suggested-by: Igor Mammedov <address@hidden>
> Signed-off-by: Bharata B Rao <address@hidden>
> ---
>  hw/i386/pc.c             |  4 ++--
>  hw/mem/pc-dimm.c         | 32 ++++++++++++++++++++++----------
>  include/hw/mem/pc-dimm.h |  2 +-
>  3 files changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 2ec45a4..c7af6aa 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1584,8 +1584,8 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
>          goto out;
>      }
>  
> -    if (pc_existing_dimms_capacity(OBJECT(machine), 
> &existing_dimms_capacity)) {
> -        error_setg(&local_err, "failed to get total size of existing DIMMs");
> +    existing_dimms_capacity = pc_existing_dimms_capacity(&local_err);
> +    if (local_err) {
>          goto out;
>      }
>  
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index f02ce6e..18cdc54 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -22,32 +22,44 @@
>  #include "qemu/config-file.h"
>  #include "qapi/visitor.h"
>  #include "qemu/range.h"
> -#include "qapi/qmp/qerror.h"
>  
> -int pc_existing_dimms_capacity(Object *obj, void *opaque)
> +typedef struct pc_dimms_capacity {
> +     uint64_t size;
> +     Error    **errp;
> +} pc_dimms_capacity;
> +
> +static int pc_existing_dimms_capacity_internal(Object *obj, void *opaque)
>  {
> -    Error *local_err = NULL;
> -    uint64_t *size = opaque;
> +    pc_dimms_capacity *cap = opaque;
> +    uint64_t *size = &cap->size;
>  
>      if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
>          DeviceState *dev = DEVICE(obj);
>  
>          if (dev->realized) {
>              (*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
> -                &local_err);
> +                cap->errp);
>          }
>  
> -        if (local_err) {
> -            qerror_report_err(local_err);
> -            error_free(local_err);
> +        if (cap->errp && *cap->errp) {
>              return 1;
>          }
>      }
> -
> -    object_child_foreach(obj, pc_existing_dimms_capacity, opaque);
> +    object_child_foreach(obj, pc_existing_dimms_capacity_internal, opaque);
>      return 0;
>  }
>  
> +uint64_t pc_existing_dimms_capacity(Error **errp)
> +{
> +    pc_dimms_capacity cap;
> +
> +    cap.size = 0;
> +    cap.errp = errp;
> +
> +    pc_existing_dimms_capacity_internal(qdev_get_machine(), &cap);
> +    return cap.size;
> +}
> +
>  int qmp_pc_dimm_device_list(Object *obj, void *opaque)
>  {
>      MemoryDeviceInfoList ***prev = opaque;
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index bbfa53f..f7b80b4 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -78,5 +78,5 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
>  int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp);
>  
>  int qmp_pc_dimm_device_list(Object *obj, void *opaque);
> -int pc_existing_dimms_capacity(Object *obj, void *opaque);
> +uint64_t pc_existing_dimms_capacity(Error **errp);
>  #endif

Reviewed-by: Igor Mammedov <address@hidden>