Re: [Qemu-devel] Missing mon in monitor_cur_is_qmp() and qerror_report()

[Markus Armbruster]

Jan Kiszka <address@hidden> writes:

> Luiz,
>
> I missed this when the API was first proposed:
>
> cur_mon is scheduled for removal (one day...). It's just an intermediate
> step to convert all users to explicit 'mon' passing. Thus, new APIs
> should not rely it.
>
> I just realized that monitor_cur_is_qmp() does so. It should be
> refactored to monitor_is_qmp(Monitor *mon). And qerror should be enhance
> by a 'mon' argument as well. Callers that aren't passed a 'mon'
> themselves should either be fixed at this chance or could fall back to
> cur_mon for the time being.
>
> So far for the theory - do you see any pitfalls in the existing usage?

I put in the new uses of cur_mon, Luiz "only" ACKed them.

At any point in the program execution, we have one current monitor, or
none.  Passing around the current monitor within monitor code is
workable, if somewhat tedious.  But we need it not just in monitor code,
we need it anywhere where we report errors.  In other words, pretty much
everywhere.  Including places that do not and should not know about the
monitor.  Handing a monitor parameter down pretty much every call chain
is beyond tedious, it's impractical.

The code reporting an error generally does not and should not know
anything about *how* the error gets communicated to the user.
Insulating it from that detail is proper separation of concerns, and
global variable cur_mon is my tool to get it.  Good software
engineering.  Like many powerful tools, global variables should be used
sparingly and with care.  I feel this use is well justified.

Instead of eliminating cur_mon, I'd like it to be hidden within
monitor.c.  There are a few uses left outside it.