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

[Jan Kiszka <jan.kiszka@siemens.com>]

Anthony Liguori wrote:
> Jan Kiszka wrote:
>> Currently all registered (and activate) monitor terminals work in
>> broadcast mode: Everyone sees what someone else types on some other
>> terminal and what the monitor reports back. This model is broken when
>> you have a management monitor terminal that is automatically operated
>> and some other terminal used for independent guest inspection. Such an
>> additional terminal can be a multiplexed device channel or a gdb
>> frontend connected to QEMU's stub.
>>
>> Therefor, this patch decouples the buffers and states of all monitor
>> terminals, allowing the user to operate them independently. The basic
>> idea is stolen from Jason Wessel: When starting to handle a monitor
>> command or some terminal event, the current monitor terminal is set to
>> the one associated with the underlying char device, letting all
>> succeeding monitor_printf show up on only this selected terminal.
>>
>> There are still two asynchronous monitor writers: some error reporting
>> in VNC's audio_add and the log-to-monitor feature of the audio
>> subsystem.
> 
> That concerns me.  Nothing should output to the monitor asychronously.

Indeed.

> 
> I'd like to see a few changes to make things a bit closer to the long
> term goals for the monitor (having proper multiple monitors devoid of
> global state).
> 
> Here's what I'd suggest:
> 
> 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...

> 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. Some use cases only
consist of the command callback itself, OK, 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. :)

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux