Re: [PATCH 2/3] chardev: report blocked write to chardev backend

["Marc-André Lureau" <marcandre.lureau@redhat.com>]

Hi

On Thu, Nov 16, 2023 at 3:54 PM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> If a chardev socket is not read, it will eventually fill and QEMU
> can block attempting to write to it. A difficult bug in avocado
> tests where the console socket was not being read from caused this
> hang.
>
> warn if a chardev write is blocked for 100ms.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> This is not necessary for the fix but it does trigger in the
> failing avocado test without the previous patch applied. Maybe
> it would be helpful?
>
> Thanks,
> Nick
>
>  chardev/char.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 996a024c7a..7c375e3cc4 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -114,6 +114,8 @@ static int qemu_chr_write_buffer(Chardev *s,
>  {
>      ChardevClass *cc = CHARDEV_GET_CLASS(s);
>      int res = 0;
> +    int nr_retries = 0;
> +
>      *offset = 0;
>
>      qemu_mutex_lock(&s->chr_write_lock);
> @@ -126,6 +128,10 @@ static int qemu_chr_write_buffer(Chardev *s,
>              } else {
>                  g_usleep(100);
>              }
> +            if (++nr_retries == 1000) { /* 100ms */
> +                warn_report("Chardev '%s' write blocked for > 100ms, "
> +                            "socket buffer full?", s->label);
> +            }

That shouldn't happen, the frontend should poll and only write when it
can. What is the qemu command being used here?
I think this change can be worth for debugging though.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>