From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38289) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZwq7-0007u0-5l for qemu-devel@nongnu.org; Thu, 10 Sep 2015 04:04:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZZwq1-0001dI-D4 for qemu-devel@nongnu.org; Thu, 10 Sep 2015 04:04:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58614) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZwq1-0001dA-7f for qemu-devel@nongnu.org; Thu, 10 Sep 2015 04:04:09 -0400 Date: Thu, 10 Sep 2015 11:04:05 +0300 From: "Michael S. Tsirkin" Message-ID: <20150910110346-mutt-send-email-mst@redhat.com> References: <1411110773-489-1-git-send-email-zifeitong@gmail.com> <87tx2z6tdf.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87tx2z6tdf.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v3] qemu-char: Do not disconnect when there's data for reading List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org, Nikolay Nikolaev , Kirill Batuzov , Anthony Liguori , Paolo Bonzini , Zifei Tong On Mon, Oct 20, 2014 at 10:24:44AM +0200, Markus Armbruster wrote: > MAINTAINERS points to Anthony, and you duly cc'ed him, but he's > effectively retired. Cc'ing recent committers include Paolo and Peter. > > Zifei Tong writes: I merged the patch that broke this, so I'll merge the fix too. > > After commit 812c1057f6175ac9a9829fa2920a2b5783814193 (Handle G_IO_HUP > > in tcp_chr_read for tcp chardev), connections are disconnected when in > > G_IO_HUP condition. > > > > However, it's possible that there is still data for reading in the channel. > > In that case, the remaining data is not handled. > > > > I saw a related bug when running socat in write-only mode, after > > > > $ echo "quit" | socat -u - UNIX-CONNECT:qemu-monitor > > > > the monitor won't not run the 'quit' command. > > > > Instead of GIOCondition, this patch uses the return value of tcp_chr_recv() > > to check the state of connection as suggested by Kirill. > > > > Cc: Kirill Batuzov > > Cc: Nikolay Nikolaev > > Cc: Markus Armbruster > > Cc: Anthony Liguori > > Signed-off-by: Zifei Tong > > --- > > Changes in v3: handle EWOULDBLOCK, remove inaccurate comment > > > > qemu-char.c | 10 ++-------- > > 1 file changed, 2 insertions(+), 8 deletions(-) > > > > diff --git a/qemu-char.c b/qemu-char.c > > index 2a3cb9f..d1893a0 100644 > > --- a/qemu-char.c > > +++ b/qemu-char.c > > @@ -2692,12 +2692,6 @@ static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque) > > uint8_t buf[READ_BUF_LEN]; > > int len, size; > > > > - if (cond & G_IO_HUP) { > > - /* connection closed */ > > - tcp_chr_disconnect(chr); > > - return TRUE; > > - } > > - > > if (!s->connected || s->max_size <= 0) { > > return TRUE; > > } > > @@ -2705,8 +2699,8 @@ static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque) > > if (len > s->max_size) > > len = s->max_size; > > size = tcp_chr_recv(chr, (void *)buf, len); > > - if (size == 0) { > > - /* connection closed */ > > + if (size == 0 || > > + (size < 0 && !(errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR))) { > > tcp_chr_disconnect(chr); > > } else if (size > 0) { > > if (s->do_telnetopt)