Received: from eggs.gnu.org ([2001:4830:134:3::10]:51892) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WNkPc-0007B7-4G for qemu-devel@nongnu.org; Wed, 12 Mar 2014 10:45:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WNkPT-0008K0-OH for qemu-devel@nongnu.org; Wed, 12 Mar 2014 10:45:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:7115) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WNkPT-0008Jw-F7 for qemu-devel@nongnu.org; Wed, 12 Mar 2014 10:45:31 -0400 From: Markus Armbruster References: <176c909f6e01409b9dd8d37108f2df5dcf8edd17.1394579440.git.crobinso@redhat.com> <871ty796n5.fsf@blackfin.pond.sub.org> <53205F9E.6000802@redhat.com> Date: Wed, 12 Mar 2014 15:45:27 +0100 In-Reply-To: <53205F9E.6000802@redhat.com> (Cole Robinson's message of "Wed, 12 Mar 2014 09:22:38 -0400") Message-ID: <87ha731nnc.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 1/6] slirp: Remove default_mon usage List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cole Robinson Cc: Jan Kiszka , qemu-devel@nongnu.org, Luiz Capitulino Cole Robinson writes: > On 03/12/2014 04:13 AM, Markus Armbruster wrote: >> Cole Robinson 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 >>> Signed-off-by: Cole Robinson >>> --- >>> 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.