From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NdP5K-0005g3-F4 for qemu-devel@nongnu.org; Fri, 05 Feb 2010 09:23:02 -0500 Received: from [199.232.76.173] (port=60487 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NdP5J-0005fp-Ue for qemu-devel@nongnu.org; Fri, 05 Feb 2010 09:23:02 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NdP5G-0005oI-7o for qemu-devel@nongnu.org; Fri, 05 Feb 2010 09:23:01 -0500 Received: from mx20.gnu.org ([199.232.41.8]:25681) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1NdP5F-0005aZ-Ok for qemu-devel@nongnu.org; Fri, 05 Feb 2010 09:22:57 -0500 Received: from mx1.redhat.com ([209.132.183.28]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NdP3i-0003a7-CR for qemu-devel@nongnu.org; Fri, 05 Feb 2010 09:21:22 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o15ELF9Y027496 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 5 Feb 2010 09:21:15 -0500 From: Markus Armbruster Subject: Re: [Qemu-devel] [PATCH 3/4] QError: Don't abort on multiple faults References: <1265314396-6583-1-git-send-email-lcapitulino@redhat.com> <1265314396-6583-4-git-send-email-lcapitulino@redhat.com> Date: Fri, 05 Feb 2010 15:21:13 +0100 In-Reply-To: (Markus Armbruster's message of "Fri, 05 Feb 2010 10:15:28 +0100") Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: qemu-devel@nongnu.org Markus Armbruster writes: > Luiz Capitulino writes: > >> Ideally, Monitor code should report an error only once and >> return the error information up the call chain. >> >> To assure that this happens as expected and that no error is >> lost, we have an assert() in qemu_error_internal(). >> >> However, we still have not fully converted handlers using >> monitor_printf() to report errors. As there can be multiple >> monitor_printf() calls on an error, the assertion is easily >> triggered when debugging is enabled; and we will get a memory >> leak if it's not. >> >> The solution to this problem is to allow multiple faults by only >> reporting the first one, and to release the additional error objects. > > I want this badly. > > [...] Let me elaborate a bit. While this patch is a much wanted improvement, what I *really* want is something else. Right now, we have 41 uses of qemu_error_new(). We still have >300 uses of monitor_printf(), many of them errors. Plus some 100 uses of qemu_error(), which boils down to monitor_printf() when running within a monitor. Not to mention >1000 uses of stderr. To convert a monitor handler to QError, we have to make it report exactly one error on every unsuccessful path, with qemu_error_new(). That's not too hard. Then we have to ensure it does not call monitor_printf() directly (not hard either) or indirectly (ouch). I say "ouch", because those prints can hide behind long call chains, in code shared with other users. Cleaning up all those stray prints will take time. Without this patch, a stray print is fatal, unless it happens to be the only one *and* there is no real error. With this patch, we survive, but the UndefinedError triggered by the stray print displaces any later real error. What I really want is that stray prints do not mess with my real errors.