Re: [Qemu-devel] [PATCH v4] qemu-char: Fix missed data on unix socket

[Paolo Bonzini]

On 19/07/2015 22:39, address@hidden wrote:
> From: Nils Carlson <address@hidden>
> 
> Commit 812c1057 introduced HUP detection on unix and tcp sockets prior
> to a read in tcp_chr_read. This unfortunately broke CloudStack 4.2
> which relied on the old behaviour where data on a socket was readable
> even if a HUP was present.
> 
> A working solution is to properly check the return values from recv,
> handling a closed socket once there is no more data to read.
> 
> Also enable polling for G_IO_NVAL to ensure the callback is called
> for all possible events as these should now be possible to handle
> with the improved error detection.
> 
> Signed-off-by: Nils Carlson <address@hidden>
> ---
>  qemu-char.c |   15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 617e034..3cbfe3e 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -807,7 +807,8 @@ static gboolean io_watch_poll_prepare(GSource *source, 
> gint *timeout_)
>      }
>  
>      if (now_active) {
> -        iwp->src = g_io_create_watch(iwp->channel, G_IO_IN | G_IO_ERR | 
> G_IO_HUP);
> +        iwp->src = g_io_create_watch(iwp->channel,
> +                                     G_IO_IN | G_IO_ERR | G_IO_HUP | 
> G_IO_NVAL);
>          g_source_set_callback(iwp->src, iwp->fd_read, iwp->opaque, NULL);
>          g_source_attach(iwp->src, NULL);
>      } else {
> @@ -2847,12 +2848,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;
>      }
> @@ -2860,7 +2855,11 @@ 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 == EWOULDBLOCK ||
> +           errno == EINTR))) {

You need to check socket_error() here instead of errno.  Also, EINTR is
not a disconnection.  However, I can fix these up for you.  I'll send a
pull request.

Paolo

>          /* connection closed */
>          tcp_chr_disconnect(chr);
>      } else if (size > 0) {
>