[2/7] virtio-console: Remove any pending watches on close

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/virtio-console.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

On (Wed) 13 Mar 2013 [14:59:42], Hans de Goede wrote:
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  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

Hi,

On 03/14/2013 08:35 AM, Amit Shah wrote:
> On (Wed) 13 Mar 2013 [14:59:42], Hans de Goede wrote:
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   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?

There is no reason, this was a bad attempt by me to deal with closing
the watch on virtio-port hot-unplug. I've removed this in my
local version of the patchset and instead added an exit callback
to deal with virtio-port hot-unplug the proper way.

I still have a few other patches for spice-qemu-char.c I'm working on,
once those are finished, I'll resend the series with this fixed and
rebased on top of Gerd Hoffman's chardev.v5 tree.

Regards,

Hans