From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42549) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDfCQ-000435-RL for qemu-devel@nongnu.org; Tue, 20 Jan 2015 15:14:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YDfCN-0008UW-KW for qemu-devel@nongnu.org; Tue, 20 Jan 2015 15:14:54 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47131) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDfCN-0008UP-CE for qemu-devel@nongnu.org; Tue, 20 Jan 2015 15:14:51 -0500 From: Markus Armbruster References: <1421744647-26844-1-git-send-email-armbru@redhat.com> <54BE8A24.10906@redhat.com> Date: Tue, 20 Jan 2015 21:14:47 +0100 In-Reply-To: <54BE8A24.10906@redhat.com> (Eric Blake's message of "Tue, 20 Jan 2015 10:02:28 -0700") Message-ID: <87fvb5ryyw.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH] qdev: Don't exit when running into bad -global List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: imammedo@redhat.com, ehabkost@redhat.com, qemu-devel@nongnu.org, afaerber@suse.de, mst@redhat.com Eric Blake 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 >> --- >> 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 Thanks!