[Qemu-devel] Re: [PATCH 28/50] error: Let converted handlers print in human monitor

[Markus Armbruster]

Luiz Capitulino <address@hidden> writes:

> On Thu, 4 Mar 2010 17:50:20 -0300
> Luiz Capitulino <address@hidden> wrote:
>
>> On Thu,  4 Mar 2010 16:56:49 +0100
>> Markus Armbruster <address@hidden> wrote:
>> 
>> > While fully converted handlers are not supposed to print anything when
>> > running in a QMP monitor, they are free to print in a human monitor.
>> 
>>  I disagree.
>
>  I've talked to Markus by irc about this one and he convinced me that
> this is the best solution for the immediate term.
>
>  Actually, I found out that only error is printed in the human monitor
> (through qerror_report()) so this is not as serious as I thought at first.

In case I confused not just you: the patch does not affect QMP monitors
at all.

I wish I had explained this better.  Let me retry: the patch provides
for partially converted handlers.  Without it, CONFIG_DEBUG_MONITOR
cries bloody murder, and converted errors get delayed until after all
output from the not-yet-converted parts of the handler.

For instance, it takes me four steps to fully convert do_device_add().
I want all the partially converted intermediate versions to work.

Even more compelling is the case of utility functions: I want to be able
to convert a function to QError without having to convert all the
handlers using it to QError / QObject in the same commit, because doing
that would result in an unreviewable hairball of a patch.

>  So, I won't nack it and the bigger mid-term discussion we should have
> is whether or not it's ok to mix qerror_report(), error_printf() &
> friends in handlers.

I think mixing is both unavoidable and harmless in intermediate
conversion steps.

Whether we want to allow stuff like error_printf_unless_qmp() in the
final state is a separate and valid question.  error_printf_unless_qmp()
solved the problem at hand for me, but of course I'm open to other
ideas.  I don't think delaying this series could help with that, though.