Re: [Qemu-devel] [PATCH 1/6] slirp: Remove default_mon usage

[Markus Armbruster]

Cole Robinson <address@hidden> writes:

> On 03/12/2014 04:13 AM, Markus Armbruster wrote:
>> Cole Robinson <address@hidden> writes:
>> 
>>> These errors don't seem user initiated, so forcibly printing to the
>>> monitor doesn't seem right. Just print to stderr.
>>>
>>> Drop lprint since it's now unused.
>>>
>>> Cc: Jan Kiszka <address@hidden>
>>> Signed-off-by: Cole Robinson <address@hidden>
>>> ---
>>> checkpatch flags some pre-existing tab issues, but I didn't retab. Should I?
>>>
>>>  slirp/misc.c  | 13 ++-----------
>>>  slirp/slirp.c |  8 ++++----
>>>  slirp/slirp.h |  2 --
>>>  3 files changed, 6 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/slirp/misc.c b/slirp/misc.c
>>> index 6c1636f..662fb1d 100644
>>> --- a/slirp/misc.c
>>> +++ b/slirp/misc.c
>>> @@ -136,7 +136,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
>>>             if ((s = qemu_socket(AF_INET, SOCK_STREAM, 0)) < 0 ||
>>>                 bind(s, (struct sockaddr *)&addr, addrlen) < 0 ||
>>>                 listen(s, 1) < 0) {
>>> -                   lprint("Error: inet socket: %s\n", strerror(errno));
>>> +                   fprintf(stderr, "Error: inet socket: %s\n", 
>>> strerror(errno));
>>>                     closesocket(s);
>>>  
>>>                     return 0;
>> 
>> Why not error_report()?
>> 
>> [...]
>> 
>
> AFAICT this is only guest initiated, never from the monitor. error_report
> sends output to cur_mon if a monitor command is being processed. If this
> message triggers while a different monitor command is being processed, do we
> really want this unrelated output to go to the monitor?
>
> (Maybe that can't happen in practice, I don't know enough about qemu's
> threading model, but I was being cautious)

If cur_mon is non-null while we're executing anything but a monitor in
the main thread, it's a bug.

That leaves signal handlers (which shouldn't care), and other threads.
Other threads used not to exist, and then our first ones didn't care,
but with more of them around now, I wonder whether we should make
cur_mon thread-local.

error_report() should remain usable when fprintf() is.