serial COM: windows serial COM PollingFunc don't sleep

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1802
Signed-off-by: Werner de Carne <werner@carne.de>
---
 chardev/char-win.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Hi Werner

On Tue, Aug 8, 2023 at 12:46 AM Werner de Carne <werner@carne.de> wrote:
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1802
> Signed-off-by: Werner de Carne <werner@carne.de>

This changes the polling callback to return 0 when I/O can't be
processed.  in util/main-loop.c, it results in an early break of
os_host_main_loop_wait().

How does that help?

thanks

> ---
>  chardev/char-win.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/chardev/char-win.c b/chardev/char-win.c
> index d4fb44c4dc..92a7016105 100644
> --- a/chardev/char-win.c
> +++ b/chardev/char-win.c
> @@ -28,7 +28,7 @@
>  #include "qapi/error.h"
>  #include "chardev/char-win.h"
>
> -static void win_chr_read(Chardev *chr, DWORD len)
> +static int win_chr_read(Chardev *chr, DWORD len)
>  {
>      WinChardev *s = WIN_CHARDEV(chr);
>      int max_size = qemu_chr_be_can_write(chr);
> @@ -40,7 +40,7 @@ static void win_chr_read(Chardev *chr, DWORD len)
>          len = max_size;
>      }
>      if (len == 0) {
> -        return;
> +        return 0;
>      }
>
>      ZeroMemory(&s->orecv, sizeof(s->orecv));
> @@ -56,6 +56,8 @@ static void win_chr_read(Chardev *chr, DWORD len)
>      if (size > 0) {
>          qemu_chr_be_write(chr, buf, size);
>      }
> +
> +    return size > 0 ? 1 : 0;
>  }
>
>  static int win_chr_serial_poll(void *opaque)
> @@ -67,8 +69,9 @@ static int win_chr_serial_poll(void *opaque)
>
>      ClearCommError(s->file, &comerr, &status);
>      if (status.cbInQue > 0) {
> -        win_chr_read(chr, status.cbInQue);
> -        return 1;
> +        if (win_chr_read(chr, status.cbInQue)) {
> +                       return 1;
> +               }
>      }
>      return 0;
>  }
> @@ -147,8 +150,9 @@ int win_chr_pipe_poll(void *opaque)
>
>      PeekNamedPipe(s->file, NULL, 0, NULL, &size, NULL);
>      if (size > 0) {
> -        win_chr_read(chr, size);
> -        return 1;
> +       if (win_chr_read(chr, size)) {
> +                       return 1;
> +               }
>      }
>      return 0;
>  }
> --
> 2.28.0.windows.1
>
>

Hi Marc-André,

when processing 2 or more characters, the guest machine mps2-an386 (uart 
has a 1 character fifo) will only process one character. Without the 
loop break, the guest gets no computing time and no further character 
can be processed. The guest gets no more computing time and so 
everything freezes. It is also sufficient to connect the uart0 from the 
guest to the Windows host with -serial COMx, but do not activate the 
uart0 in the guest. The receipt of a single character at the Windows 
host also results in the deadlock.

greetings

Am 07.08.2023 um 22:59 schrieb Marc-André Lureau:
> Hi Werner
>
> On Tue, Aug 8, 2023 at 12:46 AM Werner de Carne<werner@carne.de>  wrote:
>> Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1802
>> Signed-off-by: Werner de Carne<werner@carne.de>
> This changes the polling callback to return 0 when I/O can't be
> processed.  in util/main-loop.c, it results in an early break of
> os_host_main_loop_wait().
>
> How does that help?
>
> thanks
>
>> ---
>>   chardev/char-win.c | 16 ++++++++++------
>>   1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/chardev/char-win.c b/chardev/char-win.c
>> index d4fb44c4dc..92a7016105 100644
>> --- a/chardev/char-win.c
>> +++ b/chardev/char-win.c
>> @@ -28,7 +28,7 @@
>>   #include "qapi/error.h"
>>   #include "chardev/char-win.h"
>>
>> -static void win_chr_read(Chardev *chr, DWORD len)
>> +static int win_chr_read(Chardev *chr, DWORD len)
>>   {
>>       WinChardev *s = WIN_CHARDEV(chr);
>>       int max_size = qemu_chr_be_can_write(chr);
>> @@ -40,7 +40,7 @@ static void win_chr_read(Chardev *chr, DWORD len)
>>           len = max_size;
>>       }
>>       if (len == 0) {
>> -        return;
>> +        return 0;
>>       }
>>
>>       ZeroMemory(&s->orecv, sizeof(s->orecv));
>> @@ -56,6 +56,8 @@ static void win_chr_read(Chardev *chr, DWORD len)
>>       if (size > 0) {
>>           qemu_chr_be_write(chr, buf, size);
>>       }
>> +
>> +    return size > 0 ? 1 : 0;
>>   }
>>
>>   static int win_chr_serial_poll(void *opaque)
>> @@ -67,8 +69,9 @@ static int win_chr_serial_poll(void *opaque)
>>
>>       ClearCommError(s->file, &comerr, &status);
>>       if (status.cbInQue > 0) {
>> -        win_chr_read(chr, status.cbInQue);
>> -        return 1;
>> +        if (win_chr_read(chr, status.cbInQue)) {
>> +                       return 1;
>> +               }
>>       }
>>       return 0;
>>   }
>> @@ -147,8 +150,9 @@ int win_chr_pipe_poll(void *opaque)
>>
>>       PeekNamedPipe(s->file, NULL, 0, NULL, &size, NULL);
>>       if (size > 0) {
>> -        win_chr_read(chr, size);
>> -        return 1;
>> +       if (win_chr_read(chr, size)) {
>> +                       return 1;
>> +               }
>>       }
>>       return 0;
>>   }
>> --
>> 2.28.0.windows.1
>>
>>
>