From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41997) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XbEKV-0006gX-Iv for qemu-devel@nongnu.org; Mon, 06 Oct 2014 15:52:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XbEKQ-0002Ic-9y for qemu-devel@nongnu.org; Mon, 06 Oct 2014 15:52:23 -0400 Received: from mail-oi0-f41.google.com ([209.85.218.41]:41039) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XbEKQ-0002Fm-5l for qemu-devel@nongnu.org; Mon, 06 Oct 2014 15:52:18 -0400 Received: by mail-oi0-f41.google.com with SMTP id u20so4220189oif.0 for ; Mon, 06 Oct 2014 12:52:17 -0700 (PDT) Message-ID: <5432F2ED.8080105@mvista.com> Date: Mon, 06 Oct 2014 14:52:13 -0500 From: Corey Minyard MIME-Version: 1.0 References: <1412618363-14968-1-git-send-email-minyard@acm.org> <1412618363-14968-2-git-send-email-minyard@acm.org> <5432E14A.8030105@redhat.com> In-Reply-To: <5432E14A.8030105@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] qemu-char: Fix reconnect socket error reporting List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , minyard@acm.org, qemu-devel@nongnu.org On 10/06/2014 01:36 PM, Eric Blake wrote: > On 10/06/2014 11:59 AM, minyard@acm.org wrote: >> From: Corey Minyard >> >> If reconnect was set, errors wouldn't always be reported. >> Fix that and also only report a connect error once until a >> connection has been made. >> >> The primary purpose of this is to tell the user that a >> connection failed so they can know they need to figure out >> what went wrong. So we don't want to spew too much >> out here, just enough so they know. >> >> Signed-off-by: Corey Minyard >> --- >> qemu-char.c | 47 ++++++++++++++++++++++++++++++++--------------- >> 1 file changed, 32 insertions(+), 15 deletions(-) >> + bool connect_err_reported; >> } TCPCharDriver; >> >> static gboolean socket_reconnect_timeout(gpointer opaque); >> @@ -2521,6 +2522,17 @@ static void qemu_chr_socket_restart_timer(CharDriverState *chr) >> socket_reconnect_timeout, chr); >> } >> >> +static void check_report_connect_error(CharDriverState *chr, const char *str) >> +{ >> + TCPCharDriver *s = chr->opaque; >> + >> + if (!s->connect_err_reported) { >> + error_report("%s char device %s\n", str, chr->label); > No \n at the end of an error_report() message. Oops, I read that and forgot to change it. > >> + s->connect_err_reported = 1; > s/1/true/ since you typed it as bool. Sigh. Old habits die hard. > >> + } >> + qemu_chr_socket_restart_timer(chr); >> +} >> + >> static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void *opaque); >> >> #ifndef _WIN32 >> @@ -3045,12 +3057,14 @@ static void qemu_chr_finish_socket_connection(CharDriverState *chr, int fd) >> static void qemu_chr_socket_connected(int fd, void *opaque) >> { >> CharDriverState *chr = opaque; >> + TCPCharDriver *s = chr->opaque; >> >> if (fd < 0) { >> - qemu_chr_socket_restart_timer(chr); >> + check_report_connect_error(chr, "Unable to connect to"); > Works in English, but if there is ever translation of the message > printed in check_report_connect_error, you are probably doing > translators a disservice by splitting a sentence into two parts > separated by a number of lines of code. (Spoken by one who has never > contributed a translation, so take with a grain of salt) Yeah, I was avoiding having to add an error_vreport(). I should just add that. All other comments fixed. Thanks. -corey >> return; >> } >> >> + s->connect_err_reported = 0; > s/0/false/ > > >