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

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

[pyssling]

From: Nils Carlson <pyssling@ludd.ltu.se>

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 seems to be to simply check for a HUP after
reading all available data, i.e. recv returns a negative value.

Signed-off-by: Nils Carlson <pyssling@ludd.ltu.se>
---
 qemu-char.c |    8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 617e034..f92fcee 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2847,12 +2847,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 +2854,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 && (cond & G_IO_HUP))) {
         /* connection closed */
         tcp_chr_disconnect(chr);
     } else if (size > 0) {
-- 
1.7.10.4

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

[Paolo Bonzini]

On 17/07/2015 19:11, pyssling@ludd.ltu.se wrote:
> From: Nils Carlson <pyssling@ludd.ltu.se>
> 
> 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 seems to be to simply check for a HUP after
> reading all available data, i.e. recv returns a negative value.
> 
> Signed-off-by: Nils Carlson <pyssling@ludd.ltu.se>
> ---
>  qemu-char.c |    8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 617e034..f92fcee 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2847,12 +2847,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 +2854,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 && (cond & G_IO_HUP))) {
>          /* connection closed */
>          tcp_chr_disconnect(chr);
>      } else if (size > 0) {
> 

This looks good, but I'll also wait for your report on testing the patch
that uses the recv return value.

Paolo