Re: [Qemu-devel] [PATCH 2/7] virtio-console: Remove any pending watches on close

[Amit Shah]

On (Wed) 13 Mar 2013 [14:59:42], Hans de Goede wrote:
> Signed-off-by: Hans de Goede <address@hidden>
> ---
>  hw/virtio-console.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio-console.c b/hw/virtio-console.c
> index 1d87c5b..ec0f91b 100644
> --- a/hw/virtio-console.c
> +++ b/hw/virtio-console.c
> @@ -18,6 +18,7 @@
>  typedef struct VirtConsole {
>      VirtIOSerialPort port;
>      CharDriverState *chr;
> +    guint watch;
>  } VirtConsole;
>  
>  /*
> @@ -29,6 +30,7 @@ static gboolean chr_write_unblocked(GIOChannel *chan, 
> GIOCondition cond,
>  {
>      VirtConsole *vcon = opaque;
>  
> +    vcon->watch = 0;
>      virtio_serial_throttle_port(&vcon->port, false);
>      return FALSE;
>  }
> @@ -60,8 +62,10 @@ static ssize_t flush_buf(VirtIOSerialPort *port, const 
> uint8_t *buf, size_t len)
>              ret = 0;
>          if (!k->is_console) {
>              virtio_serial_throttle_port(port, true);
> -            qemu_chr_fe_add_watch(vcon->chr, G_IO_OUT, chr_write_unblocked,
> -                                  vcon);
> +            if (!vcon->watch) {
> +                vcon->watch = qemu_chr_fe_add_watch(vcon->chr, G_IO_OUT,
> +                                                    chr_write_unblocked, 
> vcon);
> +            }
>          }
>      }
>      return ret;
> @@ -86,6 +90,10 @@ static void guest_close(VirtIOSerialPort *port)
>      if (!vcon->chr) {
>          return;
>      }
> +    if (vcon->watch) {
> +        g_source_remove(vcon->watch);
> +        vcon->watch = 0;
> +    }
>      qemu_chr_fe_close(vcon->chr);
>  }

Even if a guest closed the connection, we want the data the guest sent
before the close to go out.

What's the reason to not do that?

> @@ -116,6 +124,10 @@ static void chr_event(void *opaque, int event)
>          virtio_serial_open(&vcon->port);
>          break;
>      case CHR_EVENT_CLOSED:
> +        if (vcon->watch) {
> +            g_source_remove(vcon->watch);
> +            vcon->watch = 0;
> +        }
>          virtio_serial_close(&vcon->port);
>          break;
>      }

Agreed with the rest.


                Amit