Friendly ping.
I've saw a couple of scripts affected by this bug.
Thanks,
Zifei
On Sep 19, 2014 10:03 PM, "Kirill Batuzov" <batuz...@ispras.ru> wrote:
> On 19.09.2014 11:12, Zifei Tong wrote:
>
>> After commit 812c1057f6175ac9a9829fa2920a2b5783814193 (Handle G_IO_HUP
>> in tcp_chr_read for tcp chardev), connections are disconnected when in
>> G_IO_HUP condition.
>>
>> However, it's possible that there is still data for reading in the
>> channel.
>> In that case, the remaining data is not handled.
>>
>> I saw a related bug when running socat in write-only mode, after
>>
>> $ echo "quit" | socat -u - UNIX-CONNECT:qemu-monitor
>>
>> the monitor won't not run the 'quit' command.
>>
>> Instead of GIOCondition, this patch uses the return value of
>> tcp_chr_recv()
>> to check the state of connection as suggested by Kirill.
>>
>> Cc: Kirill Batuzov <batuz...@ispras.ru>
>> Cc: Nikolay Nikolaev <n.nikol...@virtualopensystems.com>
>> Cc: Markus Armbruster <arm...@redhat.com>
>> Cc: Anthony Liguori <aligu...@amazon.com>
>> Signed-off-by: Zifei Tong <zifeit...@gmail.com>
>> ---
>> Changes in v3: handle EWOULDBLOCK, remove inaccurate comment
>>
>> qemu-char.c | 10 ++--------
>> 1 file changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index 2a3cb9f..d1893a0 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -2692,12 +2692,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;
>> }
>> @@ -2705,8 +2699,8 @@ 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) {
>> - /* connection closed */
>> + if (size == 0 ||
>> + (size < 0 && !(errno == EAGAIN || errno == EWOULDBLOCK || errno
>> == EINTR))) {
>> tcp_chr_disconnect(chr);
>> } else if (size > 0) {
>> if (s->do_telnetopt)
>>
>>
> Looks good to me.
>
> Reviewed-by: Kirill Batuzov <batuz...@ispras.ru>
>
> --
> Kirill
>
