Re: [Qemu-devel] Re: [PATCH 14/17] monitor: Decouple terminals

[Anthony Liguori <anthony@codemonkey.ws>]

Jan Kiszka wrote:
> Anthony Liguori wrote:
>> 1) Make monitor_printf() take a monitor state.  The easiest thing to do
>> would be to introduce this in your previous rename patch making
>> everything use current_monitor.
>>     
>
> Lazy /me was hoping to get around this...
>   

You are already changing every instance of term_printf().  How hard can 
it be to add another parameter that gets completely ignored for now :-)

The reason I want to see this in this changeset is that you're touching 
every term_printf().  Adjusting it to the right interface means we don't 
have to churn again and we don't have the hurdle of touching every 
term_printf() to get started.

>> 2) Introduce current_monitor and default_monitor global variables.  They
>> map to what you describe above and should be maintained as such.
>> 3) Make all monitor callbacks take a monitor state
>> 4) Convert monitor_printf()s called from monitor callbacks to use the
>> passed monitor state
>> 5) Eliminate all uses of current_monitor/default_monitor.
>>
>> I'd say, 1 and 2 are required for this patchset.  I think 3 and 4 would
>> be pretty easy to add to your patchset.  I think 5 is probably tougher
>> and could wait for another day.
>>     
>
> My feeling is (though I have not sound stats at hand) that a lot of
> functions all over the place will have to be extended in order to pass
> the target monitor around: From the command callbacks, through all the
> subsystems, finally reaching the monitor API.

Yes, my feeling is the same.  That's why I suggest an incremental 
approach.  You already have introduced a concept of an active and 
default terminal.  You hide this behind the monitor_printf() function.  
I'm simply suggesting making it an explicit part of the interface.

>  Some use cases only
> consist of the command callback itself, OK,

Yes, these are the low hanging fruit for conversion.

>  but the others worry me a
> bit. All doable, for sure, but I just want to make sure that we all
> agree on the result before starting this "tough" endeavor. :)
>   

It's been discussed a lot and there's a strong desire to make the 
monitor interface better for management tools.  I don't expect you to 
address the whole problem in this one series but, as I said earlier, 
since you're already making changes everywhere, let's make the right 
changes at least :-)

Regards,

Anthony Liguori

> Jan
>
>