diff mbox series

[for-3.2,v3,10/14] qdev-props: call object_apply_global_props()

Message ID 20181107123652.23417-11-marcandre.lureau@redhat.com
State New
Headers show
Series Generalize machine compatibility properties | expand

Commit Message

Marc-André Lureau Nov. 7, 2018, 12:36 p.m. UTC 
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(-)

Comments

Igor Mammedov Nov. 26, 2018, 1:20 p.m. UTC | #1 
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

>  }
>  
>  /* --- 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);
> +            }
>          }
>      }
>  }
Marc-André Lureau Nov. 26, 2018, 8:02 p.m. UTC | #2 
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.

> >  }
> >
> >  /* --- 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);
> > +            }
> >          }
> >      }
> >  }
>
>
Igor Mammedov Nov. 27, 2018, 2:12 p.m. UTC | #3 
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);
> > > +            }
> > >          }
> > >      }
> > >  }  
> >
> >  
> 
>
diff mbox series

Patch

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);
 }
 
 /* --- 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);
+            }
         }
     }
 }