diff mbox

[v2,04/10] qdev: GlobalProperty.errp field

Message ID 1466437983-27133-5-git-send-email-ehabkost@redhat.com
State New
Headers show

Commit Message

Eduardo Habkost June 20, 2016, 3:52 p.m. UTC
The new field will allow error handling to be configured by
qdev_prop_register_global() callers: &error_fatal and
&error_abort can be used to make QEMU exit or abort if any errors
are reported when applying the properties.

While doing it, change the error message from "global %s.%s=%s
ignored" to "can't apply global %s.%s=%s".

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* Reword doc comments
  * Suggested-by: Markus Armbruster <armbru@redhat.com>
* Squashed with patch "Use error_prepend() for errors applying
  globals"
  * Suggested-by: Markus Armbruster <armbru@redhat.com>
* Moved to the end of struct to not break test-qdev-global-props
---
 hw/core/qdev-properties.c | 11 ++++++++---
 include/hw/qdev-core.h    |  4 ++++
 2 files changed, 12 insertions(+), 3 deletions(-)

Comments

Igor Mammedov June 21, 2016, 8:11 a.m. UTC | #1
On Mon, 20 Jun 2016 12:52:57 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> The new field will allow error handling to be configured by
> qdev_prop_register_global() callers: &error_fatal and
> &error_abort can be used to make QEMU exit or abort if any errors
> are reported when applying the properties.
> 
> While doing it, change the error message from "global %s.%s=%s
> ignored" to "can't apply global %s.%s=%s".
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 -> v2:
> * Reword doc comments
>   * Suggested-by: Markus Armbruster <armbru@redhat.com>
> * Squashed with patch "Use error_prepend() for errors applying
>   globals"
>   * Suggested-by: Markus Armbruster <armbru@redhat.com>
> * Moved to the end of struct to not break test-qdev-global-props
> ---
>  hw/core/qdev-properties.c | 11 ++++++++---
>  include/hw/qdev-core.h    |  4 ++++
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 64e17aa..0fe7214 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1078,9 +1078,14 @@ static void
> qdev_prop_set_globals_for_type(DeviceState *dev, prop->used = true;
>          object_property_parse(OBJECT(dev), prop->value,
> prop->property, &err); if (err != NULL) {
> -            assert(prop->user_provided);
> -            error_reportf_err(err, "Warning: global %s.%s=%s
> ignored: ",
> -                              prop->driver, prop->property,
> prop->value);
> +            error_prepend(&err, "can't apply global %s.%s=%s: ",
> +                          prop->driver, prop->property, prop->value);
> +            if (prop->errp) {
> +                error_propagate(prop->errp, err);
> +            } else {
> +                assert(prop->user_provided);
> +                error_reportf_err(err, "Warning: ");
this will never print warning as assert will trigger first, but in
8/10 assert is removed so

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> +            }
>          }
>      }
>  }
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 24aa0a7..1d1f861 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -259,6 +259,9 @@ struct PropertyInfo {
>   * @user_provided: Set to true if property comes from user-provided
> config
>   * (command-line or config file).
>   * @used: Set to true if property was used when initializing a
> device.
> + * @errp: Error destination, used like first argument of error_setg()
> + *        in case property setting fails later. If @errp is NULL, we
> + *        print warnings instead of ignoring errors silently.
>   */
>  typedef struct GlobalProperty {
>      const char *driver;
> @@ -266,6 +269,7 @@ typedef struct GlobalProperty {
>      const char *value;
>      bool user_provided;
>      bool used;
> +    Error **errp;
>  } GlobalProperty;
>  
>  /*** Board API.  This should go away once we have a machine config
> file.  ***/
Eduardo Habkost June 21, 2016, 1:32 p.m. UTC | #2
On Tue, Jun 21, 2016 at 10:11:28AM +0200, Igor Mammedov wrote:
> On Mon, 20 Jun 2016 12:52:57 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > The new field will allow error handling to be configured by
> > qdev_prop_register_global() callers: &error_fatal and
> > &error_abort can be used to make QEMU exit or abort if any errors
> > are reported when applying the properties.
> > 
> > While doing it, change the error message from "global %s.%s=%s
> > ignored" to "can't apply global %s.%s=%s".
> > 
> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Changes v1 -> v2:
> > * Reword doc comments
> >   * Suggested-by: Markus Armbruster <armbru@redhat.com>
> > * Squashed with patch "Use error_prepend() for errors applying
> >   globals"
> >   * Suggested-by: Markus Armbruster <armbru@redhat.com>
> > * Moved to the end of struct to not break test-qdev-global-props
> > ---
> >  hw/core/qdev-properties.c | 11 ++++++++---
> >  include/hw/qdev-core.h    |  4 ++++
> >  2 files changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > index 64e17aa..0fe7214 100644
> > --- a/hw/core/qdev-properties.c
> > +++ b/hw/core/qdev-properties.c
> > @@ -1078,9 +1078,14 @@ static void
> > qdev_prop_set_globals_for_type(DeviceState *dev, prop->used = true;
> >          object_property_parse(OBJECT(dev), prop->value,
> > prop->property, &err); if (err != NULL) {
> > -            assert(prop->user_provided);
> > -            error_reportf_err(err, "Warning: global %s.%s=%s
> > ignored: ",
> > -                              prop->driver, prop->property,
> > prop->value);
> > +            error_prepend(&err, "can't apply global %s.%s=%s: ",
> > +                          prop->driver, prop->property, prop->value);
> > +            if (prop->errp) {
> > +                error_propagate(prop->errp, err);
> > +            } else {
> > +                assert(prop->user_provided);
> > +                error_reportf_err(err, "Warning: ");
> this will never print warning as assert will trigger first, but in
> 8/10 assert is removed so
> 

Assert won't trigger and warning will be printed if
(prop->user_provided && !prop->errp).

> Reviewed-by: Igor Mammedov <imammedo@redhat.com>

Thanks!
Markus Armbruster June 23, 2016, 7:54 a.m. UTC | #3
Eduardo Habkost <ehabkost@redhat.com> writes:

> The new field will allow error handling to be configured by
> qdev_prop_register_global() callers: &error_fatal and
> &error_abort can be used to make QEMU exit or abort if any errors
> are reported when applying the properties.
>
> While doing it, change the error message from "global %s.%s=%s
> ignored" to "can't apply global %s.%s=%s".
>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>
diff mbox

Patch

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 64e17aa..0fe7214 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1078,9 +1078,14 @@  static void qdev_prop_set_globals_for_type(DeviceState *dev,
         prop->used = true;
         object_property_parse(OBJECT(dev), prop->value, prop->property, &err);
         if (err != NULL) {
-            assert(prop->user_provided);
-            error_reportf_err(err, "Warning: global %s.%s=%s ignored: ",
-                              prop->driver, prop->property, prop->value);
+            error_prepend(&err, "can't apply global %s.%s=%s: ",
+                          prop->driver, prop->property, prop->value);
+            if (prop->errp) {
+                error_propagate(prop->errp, err);
+            } else {
+                assert(prop->user_provided);
+                error_reportf_err(err, "Warning: ");
+            }
         }
     }
 }
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 24aa0a7..1d1f861 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -259,6 +259,9 @@  struct PropertyInfo {
  * @user_provided: Set to true if property comes from user-provided config
  * (command-line or config file).
  * @used: Set to true if property was used when initializing a device.
+ * @errp: Error destination, used like first argument of error_setg()
+ *        in case property setting fails later. If @errp is NULL, we
+ *        print warnings instead of ignoring errors silently.
  */
 typedef struct GlobalProperty {
     const char *driver;
@@ -266,6 +269,7 @@  typedef struct GlobalProperty {
     const char *value;
     bool user_provided;
     bool used;
+    Error **errp;
 } GlobalProperty;
 
 /*** Board API.  This should go away once we have a machine config file.  ***/