From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MszFD-0005H2-SD for qemu-devel@nongnu.org; Wed, 30 Sep 2009 09:29:23 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MszF9-0005CA-0n for qemu-devel@nongnu.org; Wed, 30 Sep 2009 09:29:23 -0400 Received: from [199.232.76.173] (port=45777 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MszF8-0005Bt-O8 for qemu-devel@nongnu.org; Wed, 30 Sep 2009 09:29:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:21383) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MszF8-0002PG-5l for qemu-devel@nongnu.org; Wed, 30 Sep 2009 09:29:18 -0400 Received: from int-mx03.intmail.prod.int.phx2.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n8UDTHll027014 for ; Wed, 30 Sep 2009 09:29:17 -0400 Subject: Re: [Qemu-devel] [PATCH 2/3] Don't exit() in config_error() From: Mark McLoughlin In-Reply-To: <87vdj0lg4v.fsf@pike.pond.sub.org> References: <1254306462.3105.20.camel@blaa> <87vdj0lg4v.fsf@pike.pond.sub.org> Content-Type: text/plain Date: Wed, 30 Sep 2009 14:27:47 +0100 Message-Id: <1254317267.3105.62.camel@blaa> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Reply-To: Mark McLoughlin List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org On Wed, 2009-09-30 at 15:24 +0200, Markus Armbruster wrote: > Mark McLoughlin writes: > > > On Mon, 2009-09-28 at 21:11 +0200, Markus Armbruster wrote: > >> Propagating errors up the call chain is tedious. In startup code, we > >> can take a shortcut: terminate the program. This is wrong elsewhere, > >> the monitor in particular. > >> > >> config_error() tries to cater for both customers: it terminates the > >> program unless its mon parameter tells it it's working for the > >> monitor. > >> > >> Its users need to return status anyway (unless passing a null mon > >> argument, which none do), which their users need to check. So this > >> automatic exit buys us exactly nothing useful. Only the dangerous > >> delusion that we can get away without returning status. Some of its > >> users fell for that. Their callers continue executing after failure > >> when working for the monitor. > >> > >> This bites monitor command host_net_add in two places: > >> > >> * net_slirp_init() continues after slirp_hostfwd(), slirp_guestfwd(), > >> or slirp_smb() failed, and may end up reporting success. This > >> happens for "host_net_add user guestfwd=foo": it complains about the > >> invalid guest forwarding rule, then happily creates the user network > >> without guest forwarding. > >> > >> * net_client_init() can't detect slirp_guestfwd() failure, and gets > >> fooled by net_slirp_init() lying about success. Suppresses its > >> "Could not initialize device" message. > >> > >> Add the missing error reporting, make sure errors are checked, and > >> drop the exit() from config_error(). > >> > >> Signed-off-by: Markus Armbruster > >> --- > >> net.c | 83 ++++++++++++++++++++++++++++++++++++---------------------------- > >> net.h | 4 +- > >> vl.c | 9 ++++-- > >> 3 files changed, 55 insertions(+), 41 deletions(-) > >> > > ... > >> diff --git a/vl.c b/vl.c > >> index 7bfd415..96e4312 100644 > >> --- a/vl.c > >> +++ b/vl.c > > ... > >> @@ -5766,7 +5768,8 @@ int main(int argc, char **argv, char **envp) > >> > >> /* init USB devices */ > >> if (usb_enabled) { > >> - foreach_device_config(DEV_USB, usb_parse); > >> + if (foreach_device_config(DEV_USB, usb_parse) < 0) > >> + exit(1); > >> } > >> > >> /* init generic devices */ > > > > This hunk appears to be unrelated > > net_client_init() returns failure through usb_device_add(), usb_parse(), > foreach_device_config() to main(). Without this hunk, we ignore the > error and continue. > > Before patch: net_client_init() or one of its callees terminates the > program on failure, by calling config_error(). > > Makes sense? Yes, it does - I shouldn't have doubted you :-) Re-instated it again in my tree Thanks, Mark.