[Qemu-devel] [PATCH v7 25/28] qdev-props: remove errp from GlobalProperty

Marc-André Lureau posted 28 patches 7 years, 1 month ago
[Qemu-devel] [PATCH v7 25/28] qdev-props: remove errp from GlobalProperty
Posted by Marc-André Lureau 7 years, 1 month ago
All qdev_prop_register_global() set &error_fatal for errp, except
'-rtc driftfix=slew', which arguably should also use &error_fatal, as
otherwise failing to apply the property would only report a warning.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/qdev-core.h    | 6 ------
 hw/core/qdev-properties.c | 4 ++--
 qom/cpu.c                 | 1 -
 target/i386/cpu.c         | 1 -
 target/sparc/cpu.c        | 1 -
 vl.c                      | 1 -
 6 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 86b05baeeb..b6ce69c4ac 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -250,18 +250,12 @@ struct PropertyInfo {
 /**
  * GlobalProperty:
  * @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. For
- *        hotplugged devices, errp is always ignored and warnings are
- *        printed instead.
  */
 typedef struct GlobalProperty {
     const char *driver;
     const char *property;
     const char *value;
     bool used;
-    Error **errp;
 } GlobalProperty;
 
 static inline void
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 3467e0485c..a2d25d3d37 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1238,8 +1238,8 @@ void qdev_prop_set_globals(DeviceState *dev)
         if (err != NULL) {
             error_prepend(&err, "can't apply global %s.%s=%s: ",
                           prop->driver, prop->property, prop->value);
-            if (!dev->hotplugged && prop->errp) {
-                error_propagate(prop->errp, err);
+            if (!dev->hotplugged) {
+                error_propagate(&error_fatal, err);
             } else {
                 warn_report_err(err);
             }
diff --git a/qom/cpu.c b/qom/cpu.c
index 9ad1372d57..5442a7323b 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -312,7 +312,6 @@ static void cpu_common_parse_features(const char *typename, char *features,
             prop->driver = typename;
             prop->property = g_strdup(featurestr);
             prop->value = g_strdup(val);
-            prop->errp = &error_fatal;
             qdev_prop_register_global(prop);
         } else {
             error_setg(errp, "Expected key=value format, found %s.",
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 677a3bd5fb..fa37203d89 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3568,7 +3568,6 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
         prop->driver = typename;
         prop->property = g_strdup(name);
         prop->value = g_strdup(val);
-        prop->errp = &error_fatal;
         qdev_prop_register_global(prop);
     }
 
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 0f090ece54..4a4445bdf5 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -111,7 +111,6 @@ cpu_add_feat_as_prop(const char *typename, const char *name, const char *val)
     prop->driver = typename;
     prop->property = g_strdup(name);
     prop->value = g_strdup(val);
-    prop->errp = &error_fatal;
     qdev_prop_register_global(prop);
 }
 
diff --git a/vl.c b/vl.c
index 7a99a93908..72a0968fda 100644
--- a/vl.c
+++ b/vl.c
@@ -2952,7 +2952,6 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
     g->driver   = qemu_opt_get(opts, "driver");
     g->property = qemu_opt_get(opts, "property");
     g->value    = qemu_opt_get(opts, "value");
-    g->errp = &error_fatal;
     qdev_prop_register_global(g);
     return 0;
 }
-- 
2.20.1.2.gb21ebb671b


Re: [Qemu-devel] [PATCH v7 25/28] qdev-props: remove errp from GlobalProperty
Posted by Cornelia Huck 7 years, 1 month ago
On Fri, 21 Dec 2018 13:04:07 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:

> All qdev_prop_register_global() set &error_fatal for errp, except
> '-rtc driftfix=slew', which arguably should also use &error_fatal, as
> otherwise failing to apply the property would only report a warning.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/hw/qdev-core.h    | 6 ------
>  hw/core/qdev-properties.c | 4 ++--
>  qom/cpu.c                 | 1 -
>  target/i386/cpu.c         | 1 -
>  target/sparc/cpu.c        | 1 -
>  vl.c                      | 1 -
>  6 files changed, 2 insertions(+), 12 deletions(-)
> 

> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 3467e0485c..a2d25d3d37 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1238,8 +1238,8 @@ void qdev_prop_set_globals(DeviceState *dev)
>          if (err != NULL) {
>              error_prepend(&err, "can't apply global %s.%s=%s: ",
>                            prop->driver, prop->property, prop->value);
> -            if (!dev->hotplugged && prop->errp) {
> -                error_propagate(prop->errp, err);
> +            if (!dev->hotplugged) {
> +                error_propagate(&error_fatal, err);

Might want to add a comment to this function that any error is fatal
for non-hotplugged devices.

>              } else {
>                  warn_report_err(err);
>              }

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [Qemu-devel] [PATCH v7 25/28] qdev-props: remove errp from GlobalProperty
Posted by Marc-André Lureau 7 years, 1 month ago
Hi

On Fri, Dec 21, 2018 at 7:34 PM Cornelia Huck <cohuck@redhat.com> wrote:
>
> On Fri, 21 Dec 2018 13:04:07 +0400
> Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
>
> > All qdev_prop_register_global() set &error_fatal for errp, except
> > '-rtc driftfix=slew', which arguably should also use &error_fatal, as
> > otherwise failing to apply the property would only report a warning.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  include/hw/qdev-core.h    | 6 ------
> >  hw/core/qdev-properties.c | 4 ++--
> >  qom/cpu.c                 | 1 -
> >  target/i386/cpu.c         | 1 -
> >  target/sparc/cpu.c        | 1 -
> >  vl.c                      | 1 -
> >  6 files changed, 2 insertions(+), 12 deletions(-)
> >
>
> > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > index 3467e0485c..a2d25d3d37 100644
> > --- a/hw/core/qdev-properties.c
> > +++ b/hw/core/qdev-properties.c
> > @@ -1238,8 +1238,8 @@ void qdev_prop_set_globals(DeviceState *dev)
> >          if (err != NULL) {
> >              error_prepend(&err, "can't apply global %s.%s=%s: ",
> >                            prop->driver, prop->property, prop->value);
> > -            if (!dev->hotplugged && prop->errp) {
> > -                error_propagate(prop->errp, err);
> > +            if (!dev->hotplugged) {
> > +                error_propagate(&error_fatal, err);
>
> Might want to add a comment to this function that any error is fatal
> for non-hotplugged devices.
>

What about adding the following comment to GlobalProperty?

 * An error is fatal for non-hotplugged devices, when the global is applied.

> >              } else {
> >                  warn_report_err(err);
> >              }
>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>

thanks

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v7 25/28] qdev-props: remove errp from GlobalProperty
Posted by Cornelia Huck 7 years, 1 month ago
On Fri, 4 Jan 2019 14:57:26 +0400
Marc-André Lureau <marcandre.lureau@gmail.com> wrote:

> Hi
> 
> On Fri, Dec 21, 2018 at 7:34 PM Cornelia Huck <cohuck@redhat.com> wrote:
> >
> > On Fri, 21 Dec 2018 13:04:07 +0400
> > Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> >  
> > > All qdev_prop_register_global() set &error_fatal for errp, except
> > > '-rtc driftfix=slew', which arguably should also use &error_fatal, as
> > > otherwise failing to apply the property would only report a warning.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > >  include/hw/qdev-core.h    | 6 ------
> > >  hw/core/qdev-properties.c | 4 ++--
> > >  qom/cpu.c                 | 1 -
> > >  target/i386/cpu.c         | 1 -
> > >  target/sparc/cpu.c        | 1 -
> > >  vl.c                      | 1 -
> > >  6 files changed, 2 insertions(+), 12 deletions(-)
> > >  
> >  
> > > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > > index 3467e0485c..a2d25d3d37 100644
> > > --- a/hw/core/qdev-properties.c
> > > +++ b/hw/core/qdev-properties.c
> > > @@ -1238,8 +1238,8 @@ void qdev_prop_set_globals(DeviceState *dev)
> > >          if (err != NULL) {
> > >              error_prepend(&err, "can't apply global %s.%s=%s: ",
> > >                            prop->driver, prop->property, prop->value);
> > > -            if (!dev->hotplugged && prop->errp) {
> > > -                error_propagate(prop->errp, err);
> > > +            if (!dev->hotplugged) {
> > > +                error_propagate(&error_fatal, err);  
> >
> > Might want to add a comment to this function that any error is fatal
> > for non-hotplugged devices.
> >  
> 
> What about adding the following comment to GlobalProperty?
> 
>  * An error is fatal for non-hotplugged devices, when the global is applied.

Sounds good.

> 
> > >              } else {
> > >                  warn_report_err(err);
> > >              }  
> >
> > Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> >  
> 
> thanks
>