Hi Markus, On Thu, Sep 18, 2014 at 8:11 PM, Markus Armbruster <arm...@redhat.com> wrote: > Zifei Tong <zifeit...@gmail.com> writes: > >> 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 <batuz...@ispras.ru> >> Cc: Nikolay Nikolaev <n.nikol...@virtualopensystems.com> >> Cc: Markus Armbruster <arm...@redhat.com> >> Cc: Anthony Liguori <aligu...@amazon.com> >> Signed-off-by: Zifei Tong <zifeit...@gmail.com> >> --- >> qemu-char.c | 8 +------- >> 1 file changed, 1 insertion(+), 7 deletions(-) >> >> diff --git a/qemu-char.c b/qemu-char.c >> index 2a3cb9f..6cc69fa 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,7 +2699,7 @@ 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) { >> + if (size == 0 || (size < 0 && !(errno == EAGAIN || errno == EINTR))) { >> /* connection closed */ >> tcp_chr_disconnect(chr); >> } else if (size > 0) { > > What about EWOULDBLOCK?
I thought EAGAIN and EWOULDBLOCK was the same thing, until Google told me it's not the case on some platforms like HP-UX, AIX and Tru64. Shall we add one more check to support these old platforms? > The /* connection closed */ comment is now more inaccurate :) > > If size == 0, the peer has shut down its sending end of the connection. > > If size < 0, our attempt to receive encountered an error. > > I'd just drop the comment. Will do this in v3. Thanks, Zifei