On Tue, Jan 20, 2015 at 10:04:07AM +0100, Markus Armbruster wrote:
> -global lets you set a nice booby-trap for yourself:
>
> $ qemu-system-x86_64 -nodefaults -S -display none -usb -monitor stdio
> -global usb-mouse.usb_version=l
> QEMU 2.1.94 monitor - type 'help' for more information
> (qemu) device_add usb-mouse
> Parameter 'usb_version' expects an int64 value or range
> $ echo $?
> 1
>
> Not nice. Until commit 3196270 we even abort()ed.
>
> The same error triggers if you manage to screw up a machine type's
> compat_props. To demonstrate, change HW_COMPAT_2_1's entry to
>
> .driver = "usb-mouse",\
> .property = "usb_version",\
> .value = "1", \
>
> Then run
>
> $ qemu-system-x86_64 -usb -M pc-i440fx-2.1 -device usb-mouse
> upstream-qemu: -device usb-mouse: Parameter 'usb_version' expects an
> int64 value or range
> $ echo $?
> 1
>
> One of our creatively cruel error messages.
>
> Since this is actually a coding error, we *should* abort() here.
> Replace the error by an assertion failure in this case.
>
> But turn the fatal error into a mere warning when the faulty
> GlobalProperty comes from the user. Looks like this:
>
> $ qemu-system-x86_64 -nodefaults -S -display none -usb -monitor stdio
> -global usb-mouse.usb_version=l
> QEMU 2.1.94 monitor - type 'help' for more information
> (qemu) device_add usb-mouse
> Warning: global usb-mouse.usb_version=l ignored (Parameter 'usb_version'
> expects an int64 value or range)
> (qemu)
>
> This is consistent with how we handle similarly unusable -global in
> qdev_prop_check_globals().
>
> You could argue that the error should make device_add fail. Would be
> harder, because we're running within TypeInfo's instance_post_init()
> method device_post_init(), which can't fail.
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
Applied, thanks!
> ---
> hw/core/qdev-properties.c | 21 +++++++++------------
> hw/core/qdev.c | 8 +-------
> include/hw/qdev-properties.h | 4 +---
> 3 files changed, 11 insertions(+), 22 deletions(-)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 2e47f70..5a4e4d5 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -990,8 +990,8 @@ int qdev_prop_check_globals(void)
> return ret;
> }
>
> -void qdev_prop_set_globals_for_type(DeviceState *dev, const char *typename,
> - Error **errp)
> +static void qdev_prop_set_globals_for_type(DeviceState *dev,
> + const char *typename)
> {
> GlobalProperty *prop;
>
> @@ -1004,25 +1004,22 @@ void qdev_prop_set_globals_for_type(DeviceState *dev,
> const char *typename,
> prop->used = true;
> object_property_parse(OBJECT(dev), prop->value, prop->property,
> &err);
> if (err != NULL) {
> - error_propagate(errp, err);
> + assert(prop->user_provided);
> + error_report("Warning: global %s.%s=%s ignored (%s)",
> + prop->driver, prop->property, prop->value,
> + error_get_pretty(err));
> + error_free(err);
> return;
> }
> }
> }
>
> -void qdev_prop_set_globals(DeviceState *dev, Error **errp)
> +void qdev_prop_set_globals(DeviceState *dev)
> {
> ObjectClass *class = object_get_class(OBJECT(dev));
>
> do {
> - Error *err = NULL;
> -
> - qdev_prop_set_globals_for_type(dev, object_class_get_name(class),
> - &err);
> - if (err != NULL) {
> - error_propagate(errp, err);
> - return;
> - }
> + qdev_prop_set_globals_for_type(dev, object_class_get_name(class));
> class = object_class_get_parent(class);
> } while (class);
> }
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 901f289..827c084 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -1126,13 +1126,7 @@ static void device_initfn(Object *obj)
>
> static void device_post_init(Object *obj)
> {
> - Error *err = NULL;
> - qdev_prop_set_globals(DEVICE(obj), &err);
> - if (err) {
> - qerror_report_err(err);
> - error_free(err);
> - exit(EXIT_FAILURE);
> - }
> + qdev_prop_set_globals(DEVICE(obj));
> }
>
> /* Unlink device from bus and free the structure. */
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index 070006c..57ee363 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -180,9 +180,7 @@ void qdev_prop_set_ptr(DeviceState *dev, const char
> *name, void *value);
> void qdev_prop_register_global(GlobalProperty *prop);
> void qdev_prop_register_global_list(GlobalProperty *props);
> int qdev_prop_check_globals(void);
> -void qdev_prop_set_globals(DeviceState *dev, Error **errp);
> -void qdev_prop_set_globals_for_type(DeviceState *dev, const char *typename,
> - Error **errp);
> +void qdev_prop_set_globals(DeviceState *dev);
> void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
> Property *prop, const char *value);
>
> --
> 1.9.3
