From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55226) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a6zHp-0003ar-Au for qemu-devel@nongnu.org; Thu, 10 Dec 2015 06:21:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a6zHk-0004pX-4Q for qemu-devel@nongnu.org; Thu, 10 Dec 2015 06:21:25 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40384) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a6zHj-0004pR-Ti for qemu-devel@nongnu.org; Thu, 10 Dec 2015 06:21:20 -0500 Date: Thu, 10 Dec 2015 11:21:15 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20151210112114.GD2570@work-vm> References: <87a8pl9hmt.fsf@blackfin.pond.sub.org> <20151208141938.GB2593@work-vm> <87io480y0n.fsf@blackfin.pond.sub.org> <566827FC.4080701@redhat.com> <87a8piimvi.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87a8piimvi.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] Error handling in realize() methods List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Paolo Bonzini , Peter Crosthwaite , qemu-devel@nongnu.org, Andreas =?iso-8859-1?Q?F=E4rber?= , Peter Maydell * Markus Armbruster (armbru@redhat.com) wrote: > Paolo Bonzini writes: > > > On 09/12/2015 10:30, Markus Armbruster wrote: > >> My current working assumption is that passing &error_fatal to > >> memory_region_init_ram() & friends is okay even in realize() methods and > >> their supporting code, except when the allocation can be large. > > > > I suspect a lot of memory_region_init_ram()s could be considered > > potentially large (at least in the 16-64 megabytes range). Propagation > > of memory_region_init_ram() failures is easy enough, thanks to Error**, > > that we should just do it. > > Propagating an out-of-memory error right in realize() is easy. What's > not so easy is making realize() fail cleanly (all side effects undone; > we get that wrong in many places), and finding and propagating > out-of-memory errors hiding deeper in the call tree. > > However, genuinely "large" allocations should be relatively few, and > handling them gracefully in hot-pluggable devices is probably feasible. > > I doubt ensuring *all* allocations on behalf of a hot-pluggable device > are handled gracefully is a good use of our reseources, or even > feasible. > > Likewise, graceful error handling for devices that cannot be hot-plugged > feels like a waste of resources we can ill afford. I think we should > simply document their non-gracefulness by either setting hotpluggable = > false or cannot_instantiate_with_device_add_yet = true with a suitable > comment. > > > Even if we don't, we should use &error_abort, not &error_fatal > > (programmer error---due to laziness---rather than user error). > > &error_fatal should really be restricted to code that is running very > > close to main(). > > "Very close to main" is a question of dynamic context. > > Consider a device that can only be created during machine initialization > (cannot_instantiate_with_device_add_yet = true or hotpluggable = false). > &error_fatal is perfectly adequate there. &error_abort would be > misleading, because when it fails, it's almost certainly because the > user tried to create too big a machine. > > Now consider a hot-pluggable device. Its recognized "large" allocations > all fail gracefully. What about its other allocations? Two kinds: the > ones visible in the device model code, and the ones hiding elsewhere, > which include "a few" of the 2300+ uses of GLib memory allocation. The > latter exit(). Why should the former abort()? > > Now use that hot-pluggable device during machine initialization. > abort() is again misleading. > > Let's avoid a fruitless debate on when to exit() and when to abort() on > out-of-memory, and just stick to exit(). We don't need a core dump to > tell a developer to fix his lazy error handling. The tricky bit is when a user says 'it crashed with out of memory' - and we just did an exit, how do we find out which bit we should improve the error handling on? I guess the use of abort() could tell us that - however it's a really big assumption that in an OOM case we'd be able to dump the information. Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK