| Message ID | 1466022773-8965-6-git-send-email-ehabkost@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > hw/core/qdev-properties.c | 8 ++++++-- > include/hw/qdev-core.h | 4 ++++ > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c > index cd19603..0fe7214 100644 > --- a/hw/core/qdev-properties.c > +++ b/hw/core/qdev-properties.c > @@ -1078,10 +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_prepend(&err, "can't apply global %s.%s=%s: ", > prop->driver, prop->property, prop->value); > - error_reportf_err(err, "Warning: "); > + if (prop->errp) { > + error_propagate(prop->errp, err); > + } else { > + assert(prop->user_provided); > + error_reportf_err(err, "Warning: "); > + } > } > } > } Now I understand. Suggest to squash the previous patch into this one. > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 24aa0a7..16e7cc0 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -256,6 +256,9 @@ struct PropertyInfo { > > /** > * GlobalProperty: > + * @errp: If set, error_propagate() will be called on errors when applying > + * the property. &error_abort or &error_fatal may be used to make > + * errors automatically abort or exit QEMU. "If set" is awkward, because it's not what we usually mean when we talk about an error being set. Suggest "If non-null". But what makes null special isn't that errors won't be propagated. In fact, the code behaves as if they were (propagating to null frees the error, which is exactly what the code does), *except* for one thing that isn't mentioned here, but should be: we print a warning. What about: * @errp: Error destination, used like a first argument of error_setg(), * except with null @errp, we print warnings instead of ignoring errors * silently. > * @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. > @@ -264,6 +267,7 @@ typedef struct GlobalProperty { > const char *driver; > const char *property; > const char *value; > + Error **errp; > bool user_provided; > bool used; > } GlobalProperty;
On Mon, Jun 20, 2016 at 10:14:55AM +0200, Markus Armbruster wrote: > Eduardo Habkost <ehabkost@redhat.com> writes: [...] > > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > > index 24aa0a7..16e7cc0 100644 > > --- a/include/hw/qdev-core.h > > +++ b/include/hw/qdev-core.h > > @@ -256,6 +256,9 @@ struct PropertyInfo { > > > > /** > > * GlobalProperty: > > + * @errp: If set, error_propagate() will be called on errors when applying > > + * the property. &error_abort or &error_fatal may be used to make > > + * errors automatically abort or exit QEMU. > > "If set" is awkward, because it's not what we usually mean when we talk > about an error being set. Suggest "If non-null". Agreed. > > But what makes null special isn't that errors won't be propagated. In > fact, the code behaves as if they were (propagating to null frees the > error, which is exactly what the code does), *except* for one thing that > isn't mentioned here, but should be: we print a warning. > > What about: > > * @errp: Error destination, used like a first argument of error_setg(), > * except with null @errp, we print warnings instead of ignoring errors > * silently. Perfect. I will use it. Thanks!
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index cd19603..0fe7214 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -1078,10 +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_prepend(&err, "can't apply global %s.%s=%s: ", prop->driver, prop->property, prop->value); - error_reportf_err(err, "Warning: "); + 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..16e7cc0 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -256,6 +256,9 @@ struct PropertyInfo { /** * GlobalProperty: + * @errp: If set, error_propagate() will be called on errors when applying + * the property. &error_abort or &error_fatal may be used to make + * errors automatically abort or exit QEMU. * @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. @@ -264,6 +267,7 @@ typedef struct GlobalProperty { const char *driver; const char *property; const char *value; + Error **errp; bool user_provided; bool used; } GlobalProperty;
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. Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/core/qdev-properties.c | 8 ++++++-- include/hw/qdev-core.h | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)