Markus Armbruster <arm...@redhat.com> writes: > Luiz Capitulino <lcapitul...@redhat.com> 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.
