| Message ID | 20201030202131.796967-1-ehabkost@redhat.com |
|---|---|
| Headers | show |
| Series | qdev/qom: Remove explicit type names from error_setg() calls | expand |
On Fri, 30 Oct 2020 16:21:21 -0400 Eduardo Habkost <ehabkost@redhat.com> wrote: > object_property_parse() will add a > "Property '<TYPE>.<PROP>' can't take value '<VALUE>'" > prefix automatically for us. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> > --- > Cc: Eduardo Habkost <ehabkost@redhat.com> > Cc: Igor Mammedov <imammedo@redhat.com> > Cc: qemu-devel@nongnu.org > --- > backends/hostmem-memfd.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c > index e5626d4330..05cf743fe8 100644 > --- a/backends/hostmem-memfd.c > +++ b/backends/hostmem-memfd.c > @@ -87,8 +87,7 @@ memfd_backend_set_hugetlbsize(Object *obj, Visitor *v, const char *name, > return; > } > if (!value) { > - error_setg(errp, "Property '%s.%s' doesn't take value '%" PRIu64 "'", > - object_get_typename(obj), name, value); > + error_setg(errp, "hugetlbsize can't be zero"); > return; > } > m->hugetlbsize = value;
On Fri, 30 Oct 2020 16:21:27 -0400 Eduardo Habkost <ehabkost@redhat.com> wrote: > object_property_parse() will add a > "Property '<TYPE>.<PROP>' can't take value '<VALUE>'" > prefix automatically for us. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> > --- > Cc: Xiao Guangrong <xiaoguangrong.eric@gmail.com> > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Cc: Igor Mammedov <imammedo@redhat.com> > Cc: qemu-devel@nongnu.org > --- > hw/mem/nvdimm.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c > index e1574bc07c..b9a99d58ed 100644 > --- a/hw/mem/nvdimm.c > +++ b/hw/mem/nvdimm.c > @@ -56,8 +56,7 @@ static void nvdimm_set_label_size(Object *obj, Visitor *v, const char *name, > return; > } > if (value < MIN_NAMESPACE_LABEL_SIZE) { > - error_setg(errp, "Property '%s.%s' (0x%" PRIx64 ") is required" > - " at least 0x%lx", object_get_typename(obj), name, value, > + error_setg(errp, "label size should be at least 0x%lx", > MIN_NAMESPACE_LABEL_SIZE); > return; > } > @@ -89,8 +88,7 @@ static void nvdimm_set_uuid(Object *obj, Visitor *v, const char *name, > } > > if (qemu_uuid_parse(value, &nvdimm->uuid) != 0) { > - error_setg(errp, "Property '%s.%s' has invalid value", > - object_get_typename(obj), name); > + error_setg(errp, "invalid UUID"); > } > > g_free(value);
On Fri, 30 Oct 2020 16:21:12 -0400 Eduardo Habkost <ehabkost@redhat.com> wrote: > Make object_property_parse() automatically add a error message > prefix mentioning the QOM type and property name when > encountering errors. > > As we have a large number of functions that add their own > "Property '...'" to the error messages, add a temporary check for > existing prefixes before prepending our own. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> > --- > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: "Daniel P. Berrangé" <berrange@redhat.com> > Cc: Eduardo Habkost <ehabkost@redhat.com> > Cc: qemu-devel@nongnu.org > --- > qom/object.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/qom/object.c b/qom/object.c > index 20726e4584..6fb1657724 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -1635,9 +1635,20 @@ int object_property_get_enum(Object *obj, const char *name, > bool object_property_parse(Object *obj, const char *name, > const char *string, Error **errp) > { > + ERRP_GUARD(); > Visitor *v = string_input_visitor_new(string); > bool ok = object_property_set(obj, name, v, errp); > > + if (!ok) { > + /* > + * Temporary check for existing prefix, until all error reporting > + * functions remove their own prefix. > + */ > + if (!g_str_has_prefix(error_get_pretty(*errp), "Property '")) { > + error_prepend(errp, "Property '%s.%s' can't take value '%s': ", > + object_get_typename(obj), name, string); > + } > + } > visit_free(v); > return ok; > }
On Fri, 30 Oct 2020 16:21:25 -0400 Eduardo Habkost <ehabkost@redhat.com> wrote: > object_property_parse() will add a > "Property '<TYPE>.<PROP>' can't take value '<VALUE>'" > prefix automatically for us. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> > --- > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Richard Henderson <rth@twiddle.net> > Cc: Eduardo Habkost <ehabkost@redhat.com> > Cc: qemu-devel@nongnu.org > --- > target/i386/cpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 0d8606958e..2c00f94308 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -4529,7 +4529,7 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value, > int i; > > if (strlen(value) != CPUID_VENDOR_SZ) { > - error_setg(errp, QERR_PROPERTY_VALUE_BAD, "", "vendor", value); > + error_setg(errp, "invalid vendor ID"); > return; > } >
Based-on: 20201029220246.472693-1-ehabkost@redhat.com Git branch: https://gitlab.com/ehabkost/qemu/-/commits/work/prop-error-reporting One of the obstacles when refactoring the QOM property parsing code are the references to the object itself in the error code, to generate "Property '<TYPE>.<PROP>' can't take value ..." error messages. This makes it hard to isolate the string parsing code into functions that are independent from QOM. The simple solution for that would be to just remove the prefix and have less descriptive error messages. A slightly better solution is to make the code that parses -device and -object add a "Property '<TYPE>.<PROP>' can't take value '<VALUE>': " prefix automatically when encountering an error when setting a property. This is the approach implemented in this series. Eduardo Habkost (20): qom: Add prefix to error message inside object_property_parse() qdev: Stop using error_set_from_qdev_prop_error() for netdev property qdev: Stop using error_set_from_qdev_prop_error() for audiodev property qdev: Stop using error_set_from_qdev_prop_error() for mac property qdev: Stop using error_set_from_qdev_prop_error() for devfn property qdev: Stop using error_set_from_qdev_prop_error() for PCI host device property qdev: Stop using error_set_from_qdev_prop_error() for css devno property qdev: Delete unused error_set_from_qdev_prop_error() function cryptodev: Remove unnecessary prefix from error message memfd: Remove unnecessary prefix from error message tpm_util: Remove unnecessary prefix from error message qdev: drive: Remove unnecessary prefix from error message qdev: chardev: Remove unnecessary prefix from error message i386: Remove unnecessary prefix from error message qerror: Delete unused QERR_PROPERTY_VALUE_BAD macro nvdimm: Remove unnecessary prefix from error message colo-compare: Remove unnecessary prefix from error message filter-dump: Remove unnecessary prefix from error message filter-buffer: Remove unnecessary prefix from error message qom: Remove error prefix check at object_property_parse() include/hw/qdev-properties.h | 2 -- include/qapi/qmp/qerror.h | 3 --- backends/cryptodev.c | 3 +-- backends/hostmem-memfd.c | 3 +-- backends/tpm/tpm_util.c | 3 +-- hw/core/qdev-properties-system.c | 28 +++++++++++----------------- hw/core/qdev-properties.c | 22 ---------------------- hw/mem/nvdimm.c | 6 ++---- hw/s390x/css.c | 2 +- net/colo-compare.c | 9 +++------ net/dump.c | 3 +-- net/filter-buffer.c | 3 +-- qom/object.c | 5 +++++ target/i386/cpu.c | 2 +- 14 files changed, 28 insertions(+), 66 deletions(-) -- 2.28.0