Re: [Qemu-devel] [PATCH v3] qemu-char: Do not disconnect when there's data for reading

[Michael S. Tsirkin]

On Thu, Sep 10, 2015 at 11:04:05AM +0300, Michael S. Tsirkin wrote:
> 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 <address@hidden> writes:
> 
> I merged the patch that broke this, so I'll merge the fix too.

Oh, Paolo merged it already. Thanks Paolo!

> 
> > > 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 <address@hidden>
> > > Cc: Nikolay Nikolaev <address@hidden>
> > > Cc: Markus Armbruster <address@hidden>
> > > Cc: Anthony Liguori <address@hidden>
> > > Signed-off-by: Zifei Tong <address@hidden>
> > > ---
> > > 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)