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

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

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

Yes.  Dear maintainer, can you fix this up on commit, or would you
prefer me to respin?

> But that's minor, so:
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!