Re: [Qemu-devel] [PATCH v0 2/2] pc-dimm: Make pc_existing_dimms_capacity global

[Bharata B Rao]

On Tue, Jan 20, 2015 at 11:18:37AM +0100, Igor Mammedov wrote:
> On Mon, 12 Jan 2015 09:32:34 +0530
> Bharata B Rao <address@hidden> wrote:
> 
> > Move pc_existing_dimms_capacity() to pc-dimm.c since it would be needed
> > by PowerPC memory hotplug code too.
> > 
> > Signed-off-by: Bharata B Rao <address@hidden>
> > ---
> >  hw/i386/pc.c             | 24 ------------------------
> >  hw/mem/pc-dimm.c         | 25 +++++++++++++++++++++++++
> >  include/hw/mem/pc-dimm.h |  1 +
> >  3 files changed, 26 insertions(+), 24 deletions(-)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 125cf0a..2ec45a4 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1552,30 +1552,6 @@ void qemu_register_pc_machine(QEMUMachine *m)
> >      g_free(name);
> >  }
> >  
> > -static int pc_existing_dimms_capacity(Object *obj, void *opaque)
> > -{
> > -    Error *local_err = NULL;
> > -    uint64_t *size = opaque;
> > -
> > -    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);
> > -        }
> > -
> > -        if (local_err) {
> > -            qerror_report_err(local_err);
> > -            error_free(local_err);
> > -            return 1;
> > -        }
> > -    }
> > -
> > -    object_child_foreach(obj, pc_existing_dimms_capacity, opaque);
> > -    return 0;
> > -}
> > -
> >  static void pc_dimm_plug(HotplugHandler *hotplug_dev,
> >                           DeviceState *dev, Error **errp)
> >  {
> > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> > index d431834..f02ce6e 100644
> > --- a/hw/mem/pc-dimm.c
> > +++ b/hw/mem/pc-dimm.c
> > @@ -22,6 +22,31 @@
> >  #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)
> > +{
> since you are making it API, could you pass Error **errp argument
> and deal with error in caller?
> Along with it you can make function return void.

I don't think that can be done because pc_existing_dimms_capacity() calls
itself recursively via object_child_foreach() and hence its signature
if fixed.

> 
> > +    Error *local_err = NULL;
> > +    uint64_t *size = opaque;
> > +
> > +    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);
> > +        }
> > +
> > +        if (local_err) {
> > +            qerror_report_err(local_err);
> > +            error_free(local_err);
> > +            return 1;
> > +        }
> > +    }
> > +
> > +    object_child_foreach(obj, pc_existing_dimms_capacity, opaque);
> > +    return 0;
> > +}