[Qemu-devel] [PATCH] Handle G_IO_HUP in tcp_chr_read for tcp chardev

[Qemu-devel] [PATCH] Handle G_IO_HUP in tcp_chr_read for tcp chardev

[Kirill Batuzov]

Due to GLib limitations it is not possible to create several watches on one
channel on Windows hosts. See bug #338943 in GNOME bugzilla for details:
https://bugzilla.gnome.org/show_bug.cgi?id=338943

Handle G_IO_HUP in tcp_chr_read. It is already watched by corresponding watch.
Also remove the second watch with its handler.

This reverts commit cdaa86a54b232572bba594bf87a7416e527e460c.
("Add G_IO_HUP handler for socket chardev") but keeps its functionality.

Cc: Antonios Motakis <a.motakis@virtualopensystems.com>
Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
Signed-off-by: Nikita Belov <zodiac@ispras.ru>
---

GLib limitation resulted in a bug on Windows host. Steps to reproduce:

Start qemu: qemu-system-i386 -qmp tcp:127.0.0.1:4444:server:nowait
Connect with telnet: telnet 127.0.0.1 4444
Try sending some data from telnet.
Expected result: answers from QEMU.
Observed result: no answers (actually tcp_chr_read is not called at all).

---
 include/sysemu/char.h |    1 -
 qemu-char.c           |   27 ++++++---------------------
 2 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index c8b15f9..0bbd631 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -84,7 +84,6 @@ struct CharDriverState {
     int avail_connections;
     int is_mux;
     guint fd_in_tag;
-    guint fd_hup_tag;
     QemuOpts *opts;
     QTAILQ_ENTRY(CharDriverState) next;
 };
diff --git a/qemu-char.c b/qemu-char.c
index 51917de..22a9777 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2673,6 +2673,12 @@ 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;
     }
@@ -2724,25 +2730,6 @@ CharDriverState *qemu_chr_open_eventfd(int eventfd)
 }
 #endif
 
-static gboolean tcp_chr_chan_close(GIOChannel *channel, GIOCondition cond,
-                                   void *opaque)
-{
-    CharDriverState *chr = opaque;
-
-    if (cond != G_IO_HUP) {
-        return FALSE;
-    }
-
-    /* connection closed */
-    tcp_chr_disconnect(chr);
-    if (chr->fd_hup_tag) {
-        g_source_remove(chr->fd_hup_tag);
-        chr->fd_hup_tag = 0;
-    }
-
-    return TRUE;
-}
-
 static void tcp_chr_connect(void *opaque)
 {
     CharDriverState *chr = opaque;
@@ -2752,8 +2739,6 @@ static void tcp_chr_connect(void *opaque)
     if (s->chan) {
         chr->fd_in_tag = io_add_watch_poll(s->chan, tcp_chr_read_poll,
                                            tcp_chr_read, chr);
-        chr->fd_hup_tag = g_io_add_watch(s->chan, G_IO_HUP, tcp_chr_chan_close,
-                                         chr);
     }
     qemu_chr_be_generic_open(chr);
 }
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH] Handle G_IO_HUP in tcp_chr_read for tcp chardev

[Paolo Bonzini]

Il 01/07/2014 13:52, Kirill Batuzov ha scritto:
> Due to GLib limitations it is not possible to create several watches on one
> channel on Windows hosts. See bug #338943 in GNOME bugzilla for details:
> https://bugzilla.gnome.org/show_bug.cgi?id=338943
>
> Handle G_IO_HUP in tcp_chr_read. It is already watched by corresponding watch.
> Also remove the second watch with its handler.
>
> This reverts commit cdaa86a54b232572bba594bf87a7416e527e460c.
> ("Add G_IO_HUP handler for socket chardev") but keeps its functionality.
>
> Cc: Antonios Motakis <a.motakis@virtualopensystems.com>
> Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
> Signed-off-by: Nikita Belov <zodiac@ispras.ru>
> ---
>
> GLib limitation resulted in a bug on Windows host. Steps to reproduce:
>
> Start qemu: qemu-system-i386 -qmp tcp:127.0.0.1:4444:server:nowait
> Connect with telnet: telnet 127.0.0.1 4444
> Try sending some data from telnet.
> Expected result: answers from QEMU.
> Observed result: no answers (actually tcp_chr_read is not called at all).

I think Michael is on holiday; CCing Gerd.

Paolo

> ---
>  include/sysemu/char.h |    1 -
>  qemu-char.c           |   27 ++++++---------------------
>  2 files changed, 6 insertions(+), 22 deletions(-)
>
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index c8b15f9..0bbd631 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -84,7 +84,6 @@ struct CharDriverState {
>      int avail_connections;
>      int is_mux;
>      guint fd_in_tag;
> -    guint fd_hup_tag;
>      QemuOpts *opts;
>      QTAILQ_ENTRY(CharDriverState) next;
>  };
> diff --git a/qemu-char.c b/qemu-char.c
> index 51917de..22a9777 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2673,6 +2673,12 @@ 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;
>      }
> @@ -2724,25 +2730,6 @@ CharDriverState *qemu_chr_open_eventfd(int eventfd)
>  }
>  #endif
>
> -static gboolean tcp_chr_chan_close(GIOChannel *channel, GIOCondition cond,
> -                                   void *opaque)
> -{
> -    CharDriverState *chr = opaque;
> -
> -    if (cond != G_IO_HUP) {
> -        return FALSE;
> -    }
> -
> -    /* connection closed */
> -    tcp_chr_disconnect(chr);
> -    if (chr->fd_hup_tag) {
> -        g_source_remove(chr->fd_hup_tag);
> -        chr->fd_hup_tag = 0;
> -    }
> -
> -    return TRUE;
> -}
> -
>  static void tcp_chr_connect(void *opaque)
>  {
>      CharDriverState *chr = opaque;
> @@ -2752,8 +2739,6 @@ static void tcp_chr_connect(void *opaque)
>      if (s->chan) {
>          chr->fd_in_tag = io_add_watch_poll(s->chan, tcp_chr_read_poll,
>                                             tcp_chr_read, chr);
> -        chr->fd_hup_tag = g_io_add_watch(s->chan, G_IO_HUP, tcp_chr_chan_close,
> -                                         chr);
>      }
>      qemu_chr_be_generic_open(chr);
>  }
>

Re: [Qemu-devel] [PATCH] Handle G_IO_HUP in tcp_chr_read for tcp chardev

[Michael S. Tsirkin]

On Tue, Jul 01, 2014 at 02:04:38PM +0200, Paolo Bonzini wrote:
> Il 01/07/2014 13:52, Kirill Batuzov ha scritto:
> >Due to GLib limitations it is not possible to create several watches on one
> >channel on Windows hosts. See bug #338943 in GNOME bugzilla for details:
> >https://bugzilla.gnome.org/show_bug.cgi?id=338943
> >
> >Handle G_IO_HUP in tcp_chr_read. It is already watched by corresponding watch.
> >Also remove the second watch with its handler.
> >
> >This reverts commit cdaa86a54b232572bba594bf87a7416e527e460c.
> >("Add G_IO_HUP handler for socket chardev") but keeps its functionality.
> >
> >Cc: Antonios Motakis <a.motakis@virtualopensystems.com>
> >Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> >Cc: Michael S. Tsirkin <mst@redhat.com>
> >Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
> >Signed-off-by: Nikita Belov <zodiac@ispras.ru>
> >---
> >
> >GLib limitation resulted in a bug on Windows host. Steps to reproduce:
> >
> >Start qemu: qemu-system-i386 -qmp tcp:127.0.0.1:4444:server:nowait
> >Connect with telnet: telnet 127.0.0.1 4444
> >Try sending some data from telnet.
> >Expected result: answers from QEMU.
> >Observed result: no answers (actually tcp_chr_read is not called at all).
> 
> I think Michael is on holiday; CCing Gerd.
> 
> Paolo

Not really :)


> >---
> > include/sysemu/char.h |    1 -
> > qemu-char.c           |   27 ++++++---------------------
> > 2 files changed, 6 insertions(+), 22 deletions(-)
> >
> >diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> >index c8b15f9..0bbd631 100644
> >--- a/include/sysemu/char.h
> >+++ b/include/sysemu/char.h
> >@@ -84,7 +84,6 @@ struct CharDriverState {
> >     int avail_connections;
> >     int is_mux;
> >     guint fd_in_tag;
> >-    guint fd_hup_tag;
> >     QemuOpts *opts;
> >     QTAILQ_ENTRY(CharDriverState) next;
> > };
> >diff --git a/qemu-char.c b/qemu-char.c
> >index 51917de..22a9777 100644
> >--- a/qemu-char.c
> >+++ b/qemu-char.c
> >@@ -2673,6 +2673,12 @@ 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;
> >     }
> >@@ -2724,25 +2730,6 @@ CharDriverState *qemu_chr_open_eventfd(int eventfd)
> > }
> > #endif
> >
> >-static gboolean tcp_chr_chan_close(GIOChannel *channel, GIOCondition cond,
> >-                                   void *opaque)
> >-{
> >-    CharDriverState *chr = opaque;
> >-
> >-    if (cond != G_IO_HUP) {
> >-        return FALSE;
> >-    }
> >-
> >-    /* connection closed */
> >-    tcp_chr_disconnect(chr);
> >-    if (chr->fd_hup_tag) {
> >-        g_source_remove(chr->fd_hup_tag);
> >-        chr->fd_hup_tag = 0;
> >-    }
> >-
> >-    return TRUE;
> >-}
> >-
> > static void tcp_chr_connect(void *opaque)
> > {
> >     CharDriverState *chr = opaque;
> >@@ -2752,8 +2739,6 @@ static void tcp_chr_connect(void *opaque)
> >     if (s->chan) {
> >         chr->fd_in_tag = io_add_watch_poll(s->chan, tcp_chr_read_poll,
> >                                            tcp_chr_read, chr);
> >-        chr->fd_hup_tag = g_io_add_watch(s->chan, G_IO_HUP, tcp_chr_chan_close,
> >-                                         chr);
> >     }
> >     qemu_chr_be_generic_open(chr);
> > }
> >

Re: [Qemu-devel] [PATCH] Handle G_IO_HUP in tcp_chr_read for tcp chardev

[Michael S. Tsirkin]

On Tue, Jul 01, 2014 at 03:52:32PM +0400, Kirill Batuzov wrote:
> Due to GLib limitations it is not possible to create several watches on one
> channel on Windows hosts. See bug #338943 in GNOME bugzilla for details:
> https://bugzilla.gnome.org/show_bug.cgi?id=338943
> 
> Handle G_IO_HUP in tcp_chr_read. It is already watched by corresponding watch.
> Also remove the second watch with its handler.
> 
> This reverts commit cdaa86a54b232572bba594bf87a7416e527e460c.
> ("Add G_IO_HUP handler for socket chardev") but keeps its functionality.
> 
> Cc: Antonios Motakis <a.motakis@virtualopensystems.com>
> Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
> Signed-off-by: Nikita Belov <zodiac@ispras.ru>
> ---

Thanks, applied.

No reason to put below after --: it's useful in commit log.

> GLib limitation resulted in a bug on Windows host. Steps to reproduce:
> 
> Start qemu: qemu-system-i386 -qmp tcp:127.0.0.1:4444:server:nowait
> Connect with telnet: telnet 127.0.0.1 4444
> Try sending some data from telnet.
> Expected result: answers from QEMU.
> Observed result: no answers (actually tcp_chr_read is not called at all).
> 
> ---
>  include/sysemu/char.h |    1 -
>  qemu-char.c           |   27 ++++++---------------------
>  2 files changed, 6 insertions(+), 22 deletions(-)
> 
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index c8b15f9..0bbd631 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -84,7 +84,6 @@ struct CharDriverState {
>      int avail_connections;
>      int is_mux;
>      guint fd_in_tag;
> -    guint fd_hup_tag;
>      QemuOpts *opts;
>      QTAILQ_ENTRY(CharDriverState) next;
>  };
> diff --git a/qemu-char.c b/qemu-char.c
> index 51917de..22a9777 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2673,6 +2673,12 @@ 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;
>      }
> @@ -2724,25 +2730,6 @@ CharDriverState *qemu_chr_open_eventfd(int eventfd)
>  }
>  #endif
>  
> -static gboolean tcp_chr_chan_close(GIOChannel *channel, GIOCondition cond,
> -                                   void *opaque)
> -{
> -    CharDriverState *chr = opaque;
> -
> -    if (cond != G_IO_HUP) {
> -        return FALSE;
> -    }
> -
> -    /* connection closed */
> -    tcp_chr_disconnect(chr);
> -    if (chr->fd_hup_tag) {
> -        g_source_remove(chr->fd_hup_tag);
> -        chr->fd_hup_tag = 0;
> -    }
> -
> -    return TRUE;
> -}
> -
>  static void tcp_chr_connect(void *opaque)
>  {
>      CharDriverState *chr = opaque;
> @@ -2752,8 +2739,6 @@ static void tcp_chr_connect(void *opaque)
>      if (s->chan) {
>          chr->fd_in_tag = io_add_watch_poll(s->chan, tcp_chr_read_poll,
>                                             tcp_chr_read, chr);
> -        chr->fd_hup_tag = g_io_add_watch(s->chan, G_IO_HUP, tcp_chr_chan_close,
> -                                         chr);
>      }
>      qemu_chr_be_generic_open(chr);
>  }
> -- 
> 1.7.10.4

Re: [Qemu-devel] [PATCH] Handle G_IO_HUP in tcp_chr_read for tcp chardev

[Alex Bennée]

Kirill Batuzov writes:

> Due to GLib limitations it is not possible to create several watches on one
> channel on Windows hosts. See bug #338943 in GNOME bugzilla for details:
> https://bugzilla.gnome.org/show_bug.cgi?id=338943
>
> Handle G_IO_HUP in tcp_chr_read. It is already watched by corresponding watch.
> Also remove the second watch with its handler.
>
> This reverts commit cdaa86a54b232572bba594bf87a7416e527e460c.
> ("Add G_IO_HUP handler for socket chardev") but keeps its functionality.
<snip>
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2673,6 +2673,12 @@ 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;
> +    }

Is this right. AIUI we could return FALSE here to remove the watch for
the now closed channel.

-- 
Alex Bennée

Re: [Qemu-devel] [PATCH] Handle G_IO_HUP in tcp_chr_read for tcp chardev

[Kirill Batuzov]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1938 bytes --]

On Tue, 1 Jul 2014, Alex Bennée wrote:

> 
> Kirill Batuzov writes:
> 
> > Due to GLib limitations it is not possible to create several watches on one
> > channel on Windows hosts. See bug #338943 in GNOME bugzilla for details:
> > https://bugzilla.gnome.org/show_bug.cgi?id=338943
> >
> > Handle G_IO_HUP in tcp_chr_read. It is already watched by corresponding watch.
> > Also remove the second watch with its handler.
> >
> > This reverts commit cdaa86a54b232572bba594bf87a7416e527e460c.
> > ("Add G_IO_HUP handler for socket chardev") but keeps its functionality.
> <snip>
> > --- a/qemu-char.c
> > +++ b/qemu-char.c
> > @@ -2673,6 +2673,12 @@ 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;
> > +    }
> 
> Is this right. AIUI we could return FALSE here to remove the watch for
> the now closed channel.
> 
>
tcp_chr_disconnect will remove the watch and perform other non-obvious
cleanup. Also we already have tcp_chr_disconnect followed by return TRUE
in tcp_chr_read:

    size = tcp_chr_recv(chr, (void *)buf, len);
    if (size == 0) {
        /* connection closed */
        tcp_chr_disconnect(chr);
    } else if (size > 0) {
	    /* ... */
    }

    return TRUE;

Actually I'm not sure that tcp_chr_read handles all corner cases
correctly. For eaxmple, should not it be "size <= 0" instead of
"size == 0" in the above code? tcp_chr_recv may return negative value on error.
Should not tcp_chr_recv handle G_IO_ERR condition? It is watched in
corresponding watch.

But since we are in soft-freeze before 2.1 I decided to keep the patch as
simple as possible and not to fix theoretical bugs for which I do not
have real examples.

-- 
Kirill

Re: [Qemu-devel] [PATCH] Handle G_IO_HUP in tcp_chr_read for tcp chardev

[Nikolay Nikolaev]

On Tue, Jul 1, 2014 at 2:52 PM, Kirill Batuzov <batuzovk@ispras.ru> wrote:
> Due to GLib limitations it is not possible to create several watches on one
> channel on Windows hosts. See bug #338943 in GNOME bugzilla for details:
> https://bugzilla.gnome.org/show_bug.cgi?id=338943
>
> Handle G_IO_HUP in tcp_chr_read. It is already watched by corresponding watch.
> Also remove the second watch with its handler.
Shall this work when the connection is idle and the remote end closes it?

>
> This reverts commit cdaa86a54b232572bba594bf87a7416e527e460c.
> ("Add G_IO_HUP handler for socket chardev") but keeps its functionality.
>
> Cc: Antonios Motakis <a.motakis@virtualopensystems.com>
> Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
> Signed-off-by: Nikita Belov <zodiac@ispras.ru>
> ---
>
> GLib limitation resulted in a bug on Windows host. Steps to reproduce:
>
> Start qemu: qemu-system-i386 -qmp tcp:127.0.0.1:4444:server:nowait
> Connect with telnet: telnet 127.0.0.1 4444
> Try sending some data from telnet.
> Expected result: answers from QEMU.
> Observed result: no answers (actually tcp_chr_read is not called at all).
>
> ---
>  include/sysemu/char.h |    1 -
>  qemu-char.c           |   27 ++++++---------------------
>  2 files changed, 6 insertions(+), 22 deletions(-)
>
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index c8b15f9..0bbd631 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -84,7 +84,6 @@ struct CharDriverState {
>      int avail_connections;
>      int is_mux;
>      guint fd_in_tag;
> -    guint fd_hup_tag;
>      QemuOpts *opts;
>      QTAILQ_ENTRY(CharDriverState) next;
>  };
> diff --git a/qemu-char.c b/qemu-char.c
> index 51917de..22a9777 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2673,6 +2673,12 @@ 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;
>      }
> @@ -2724,25 +2730,6 @@ CharDriverState *qemu_chr_open_eventfd(int eventfd)
>  }
>  #endif
>
> -static gboolean tcp_chr_chan_close(GIOChannel *channel, GIOCondition cond,
> -                                   void *opaque)
> -{
> -    CharDriverState *chr = opaque;
> -
> -    if (cond != G_IO_HUP) {
> -        return FALSE;
> -    }
> -
> -    /* connection closed */
> -    tcp_chr_disconnect(chr);
> -    if (chr->fd_hup_tag) {
> -        g_source_remove(chr->fd_hup_tag);
> -        chr->fd_hup_tag = 0;
> -    }
> -
> -    return TRUE;
> -}
> -
>  static void tcp_chr_connect(void *opaque)
>  {
>      CharDriverState *chr = opaque;
> @@ -2752,8 +2739,6 @@ static void tcp_chr_connect(void *opaque)
>      if (s->chan) {
>          chr->fd_in_tag = io_add_watch_poll(s->chan, tcp_chr_read_poll,
>                                             tcp_chr_read, chr);
> -        chr->fd_hup_tag = g_io_add_watch(s->chan, G_IO_HUP, tcp_chr_chan_close,
> -                                         chr);
>      }
>      qemu_chr_be_generic_open(chr);
>  }
> --
> 1.7.10.4
>

Re: [Qemu-devel] [PATCH] Handle G_IO_HUP in tcp_chr_read for tcp chardev

[Kirill Batuzov]

On Mon, 7 Jul 2014, Nikolay Nikolaev wrote:

> On Tue, Jul 1, 2014 at 2:52 PM, Kirill Batuzov <batuzovk@ispras.ru> wrote:
> > Due to GLib limitations it is not possible to create several watches on one
> > channel on Windows hosts. See bug #338943 in GNOME bugzilla for details:
> > https://bugzilla.gnome.org/show_bug.cgi?id=338943
> >
> > Handle G_IO_HUP in tcp_chr_read. It is already watched by corresponding watch.
> > Also remove the second watch with its handler.
> Shall this work when the connection is idle and the remote end closes it?
>

Yes, this should work. Why do you think it may not? The watch is
enabled, so tcp_chr_read will be called. During testing it handled
disconnects just fine.

> >
> > This reverts commit cdaa86a54b232572bba594bf87a7416e527e460c.
> > ("Add G_IO_HUP handler for socket chardev") but keeps its functionality.
> >
> > Cc: Antonios Motakis <a.motakis@virtualopensystems.com>
> > Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
> > Signed-off-by: Nikita Belov <zodiac@ispras.ru>
> > ---
> >
> > GLib limitation resulted in a bug on Windows host. Steps to reproduce:
> >
> > Start qemu: qemu-system-i386 -qmp tcp:127.0.0.1:4444:server:nowait
> > Connect with telnet: telnet 127.0.0.1 4444
> > Try sending some data from telnet.
> > Expected result: answers from QEMU.
> > Observed result: no answers (actually tcp_chr_read is not called at all).
> >
> > ---
> >  include/sysemu/char.h |    1 -
> >  qemu-char.c           |   27 ++++++---------------------
> >  2 files changed, 6 insertions(+), 22 deletions(-)
> >
> > diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> > index c8b15f9..0bbd631 100644
> > --- a/include/sysemu/char.h
> > +++ b/include/sysemu/char.h
> > @@ -84,7 +84,6 @@ struct CharDriverState {
> >      int avail_connections;
> >      int is_mux;
> >      guint fd_in_tag;
> > -    guint fd_hup_tag;
> >      QemuOpts *opts;
> >      QTAILQ_ENTRY(CharDriverState) next;
> >  };
> > diff --git a/qemu-char.c b/qemu-char.c
> > index 51917de..22a9777 100644
> > --- a/qemu-char.c
> > +++ b/qemu-char.c
> > @@ -2673,6 +2673,12 @@ 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;
> >      }
> > @@ -2724,25 +2730,6 @@ CharDriverState *qemu_chr_open_eventfd(int eventfd)
> >  }
> >  #endif
> >
> > -static gboolean tcp_chr_chan_close(GIOChannel *channel, GIOCondition cond,
> > -                                   void *opaque)
> > -{
> > -    CharDriverState *chr = opaque;
> > -
> > -    if (cond != G_IO_HUP) {
> > -        return FALSE;
> > -    }
> > -
> > -    /* connection closed */
> > -    tcp_chr_disconnect(chr);
> > -    if (chr->fd_hup_tag) {
> > -        g_source_remove(chr->fd_hup_tag);
> > -        chr->fd_hup_tag = 0;
> > -    }
> > -
> > -    return TRUE;
> > -}
> > -
> >  static void tcp_chr_connect(void *opaque)
> >  {
> >      CharDriverState *chr = opaque;
> > @@ -2752,8 +2739,6 @@ static void tcp_chr_connect(void *opaque)
> >      if (s->chan) {
> >          chr->fd_in_tag = io_add_watch_poll(s->chan, tcp_chr_read_poll,
> >                                             tcp_chr_read, chr);
> > -        chr->fd_hup_tag = g_io_add_watch(s->chan, G_IO_HUP, tcp_chr_chan_close,
> > -                                         chr);
> >      }
> >      qemu_chr_be_generic_open(chr);
> >  }
> > --
> > 1.7.10.4
> >
>