Re: [Qemu-devel] [PATCH v0 00/21]: Monitor: improve handlers error handling

[Markus Armbruster]

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, ...).