[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 14:00:12 -0200
On Thu, 11 Feb 2010 16:27:00 +0100
Markus Armbruster <address@hidden> wrote:
> Anthony Liguori <address@hidden> writes:
> > On 02/10/2010 07:49 PM, Luiz Capitulino wrote:
> >> Hi there,
> >> 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", into the monitor
> state. This lets us keep the current mechanisms to report success /
> failure (return >= 0 / -1; non-NULL / NULL, non-zero / zero, ...).
Yes, also note that some functions are used by other subsystems. If they
return -1/0 we'd have to adjust other call chains as well.
- [Qemu-devel] [PATCH 17/21] Monitor: Debugging support, (continued)
- [Qemu-devel] [PATCH 17/21] Monitor: Debugging support, Luiz Capitulino, 2010/02/10
- [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