[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v0 00/21]: Monitor: improve handlers error handl
Re: [Qemu-devel] [PATCH v0 00/21]: Monitor: improve handlers error handling
Thu, 11 Feb 2010 10:12:29 -0600
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:18.104.22.168) Gecko/20091209 Fedora/3.0-4.fc12 Lightning/1.0pre Thunderbird/3.0
On 02/11/2010 09:27 AM, Markus Armbruster wrote:
Anthony Liguori<address@hidden> writes:
On 02/10/2010 07:49 PM, Luiz Capitulino wrote:
When I started converting handlers to the QObject style, I thought that
returning an error code wouldn't be needed. That is, we have an error object
already, so if the handler returns the error object it has failed, otherwise
it has succeeded.
This was also very convenient, because handlers have never returned an error
code, and thus it would be easier to add a call to qemu_error_new() in the
right place instead of returning error codes.
Turns out we need both. Actually, I should not have abused the error object
this way because (as Markus says) this is too fragile and we can end up
reporting bogus errors to clients (among other problems).
The good news is that it's easy to fix.
All we have to do is to change cmd_new() (the handler callback) to return an
error code and convert existing QObject handlers to it. This series does that
and most of the patches are really straightforward conversions.
Additionally, Markus has designed an excellent debug mechanism for QMP, which
is implemented by the last patches in this series, and will allow us to catch
important QObject conversion and error handling bugs in handlers.
Instead of returning -1, would it make more sense to return an error
object? If fact, why not drop ret_data as a passed in parameter, and
just always return either the result or an error object.
Tempting, isn't it?
The practical trouble with this idea is that you have to adjust a lot of
code to return error objects all the way up into the handler. With the
current design, you emit error objects "sideways",
But you still have to propagate an error return somewhere in order to
short circuit the execution of the handler.
So you end up doing:
err = func1();
if (err == -1)
err = func2();
if (err == -1)
Would it be just as easy to say:
obj = func1();
obj = func2();
Ultimately, this you the ability to decide in func3 whether you want to
continue propagating the error or whether you want to take corrective
The current qerror stuff is really only useful within a single
function. Once you start building infrastructure around qerror, it
becomes very difficult to deal with.
into the monitor
state. This lets us keep the current mechanisms to report success /
failure (return>= 0 / -1; non-NULL / NULL, non-zero / zero, ...).
- [Qemu-devel] [PATCH 18/21] Monitor: Drop the print disabling mechanism, (continued)
- [Qemu-devel] [PATCH 18/21] Monitor: Drop the print disabling mechanism, Luiz Capitulino, 2010/02/10
- [Qemu-devel] [PATCH 19/21] Monitor: Audit handler return, Luiz Capitulino, 2010/02/10
- [Qemu-devel] [PATCH 20/21] Monitor: Debug stray prints the right way, Luiz Capitulino, 2010/02/10
- [Qemu-devel] [PATCH 21/21] Monitor: Report more than one error in handlers, Luiz Capitulino, 2010/02/10
- Re: [Qemu-devel] [PATCH v0 00/21]: Monitor: improve handlers error handling, Markus Armbruster, 2010/02/11
- Re: [Qemu-devel] [PATCH v0 00/21]: Monitor: improve handlers error handling, Anthony Liguori, 2010/02/11