Re: [Qemu-devel] [PATCH] qdev: Don't exit when running into bad -global

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1726 bytes --]

On 01/20/2015 02:04 AM, 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.
> 

> 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.

I agree that outputting a warning up front then ignoring the bogus
value, is as good as we can do given we are under "can't fail"
constraints, and much better than confusingly failing down the road.

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/core/qdev-properties.c    | 21 +++++++++------------
>  hw/core/qdev.c               |  8 +-------
>  include/hw/qdev-properties.h |  4 +---
>  3 files changed, 11 insertions(+), 22 deletions(-)


>  
> -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)

Is the indentation off here?

But that's minor, so:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]