Re: [Qemu-arm] [Qemu-devel] [PATCH for-3.2 v3 10/14] qdev-props: call object_apply_global_props()

[Igor Mammedov <imammedo@redhat.com>]

On Tue, 27 Nov 2018 00:02:35 +0400
Marc-André Lureau <marcandre.lureau@gmail.com> wrote:

> Hi
> On Mon, Nov 26, 2018 at 5:27 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Wed,  7 Nov 2018 16:36:48 +0400
> > Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> >  
> > > It's now possible to use the common function.
> > >
> > > Teach object_apply_global_props() to warn if Error argument is NULL.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  hw/core/qdev-properties.c | 24 ++----------------------
> > >  qom/object.c              |  6 +++++-
> > >  2 files changed, 7 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > > index 8728cbab9f..239535a4cb 100644
> > > --- a/hw/core/qdev-properties.c
> > > +++ b/hw/core/qdev-properties.c
> > > @@ -1223,28 +1223,8 @@ int qdev_prop_check_globals(void)
> > >
> > >  void qdev_prop_set_globals(DeviceState *dev)
> > >  {
> > > -    int i;
> > > -
> > > -    for (i = 0; i < global_props()->len; i++) {
> > > -        GlobalProperty *prop;
> > > -        Error *err = NULL;
> > > -
> > > -        prop = g_array_index(global_props(), GlobalProperty *, i);
> > > -        if (object_dynamic_cast(OBJECT(dev), prop->driver) == NULL) {
> > > -            continue;
> > > -        }
> > > -        prop->used = true;
> > > -        object_property_parse(OBJECT(dev), prop->value, prop->property, &err);
> > > -        if (err != NULL) {
> > > -            error_prepend(&err, "can't apply global %s.%s=%s: ",
> > > -                          prop->driver, prop->property, prop->value);
> > > -            if (!dev->hotplugged) {
> > > -                error_propagate(&error_fatal, err);
> > > -            } else {
> > > -                warn_report_err(err);
> > > -            }
> > > -        }
> > > -    }
> > > +    object_apply_global_props(OBJECT(dev), global_props(),
> > > +                              dev->hotplugged ? NULL : &error_fatal);  
> > arguably, it's up to caller to decide it warn or not.
> > I'd leave it warning code out of object_apply_global_props() and let caller do the job  
> 
> The problem is that there may be multiple errors, and the remaining
> globals should be applied.
>
> I'll add a comment.
ok

 
> > >  }
> > >
> > >  /* --- 64bit unsigned int 'size' type --- */
> > > diff --git a/qom/object.c b/qom/object.c
> > > index 9acdf9e16d..b1a7f70550 100644
> > > --- a/qom/object.c
> > > +++ b/qom/object.c
> > > @@ -392,7 +392,11 @@ void object_apply_global_props(Object *obj, GArray *props, Error **errp)
> > >          if (err != NULL) {
> > >              error_prepend(&err, "can't apply global %s.%s=%s: ",
> > >                            p->driver, p->property, p->value);
> > > -            error_propagate(errp, err);
> > > +            if (errp) {
> > > +                error_propagate(errp, err);
> > > +            } else {
> > > +                warn_report_err(err);
> > > +            }
> > >          }
> > >      }
> > >  }  
> >
> >  
> 
>