[Qemu-devel] [PATCH] hw/char/serial: retry write if EAGAIN

[Qemu-devel] [PATCH] hw/char/serial: retry write if EAGAIN

[Marc-André Lureau]

If the chardev returns -1 with EAGAIN errno on write(), it should try
to send it again (EINTR is handled by the chardev itself).

This fixes commit 019288bf137183bf3407c9824655b753bfafc99f
"hw/char/serial: Only retry if qemu_chr_fe_write returns 0"

Tested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/char/serial.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index cd7d747c68..8bd1da2fa3 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -261,15 +261,20 @@ static void serial_xmit(SerialState *s)
         if (s->mcr & UART_MCR_LOOP) {
             /* in loopback mode, say that we just received a char */
             serial_receive1(s, &s->tsr, 1);
-        } else if (qemu_chr_fe_write(&s->chr, &s->tsr, 1) == 0 &&
-                   s->tsr_retry < MAX_XMIT_RETRY) {
-            assert(s->watch_tag == 0);
-            s->watch_tag =
-                qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
-                                      serial_watch_cb, s);
-            if (s->watch_tag > 0) {
-                s->tsr_retry++;
-                return;
+        } else {
+            int rc = qemu_chr_fe_write(&s->chr, &s->tsr, 1);
+
+            if ((rc == 0 ||
+                 (rc == -1 && errno == EAGAIN)) &&
+                s->tsr_retry < MAX_XMIT_RETRY) {
+                assert(s->watch_tag == 0);
+                s->watch_tag =
+                    qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
+                                          serial_watch_cb, s);
+                if (s->watch_tag > 0) {
+                    s->tsr_retry++;
+                    return;
+                }
             }
         }
         s->tsr_retry = 0;
-- 
2.18.0.129.ge3331758f1

Re: [Qemu-devel] [PATCH] hw/char/serial: retry write if EAGAIN

[Paolo Bonzini]

On 16/07/2018 13:07, Marc-André Lureau wrote:
> If the chardev returns -1 with EAGAIN errno on write(), it should try
> to send it again (EINTR is handled by the chardev itself).
> 
> This fixes commit 019288bf137183bf3407c9824655b753bfafc99f
> "hw/char/serial: Only retry if qemu_chr_fe_write returns 0"
> 
> Tested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  hw/char/serial.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index cd7d747c68..8bd1da2fa3 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -261,15 +261,20 @@ static void serial_xmit(SerialState *s)
>          if (s->mcr & UART_MCR_LOOP) {
>              /* in loopback mode, say that we just received a char */
>              serial_receive1(s, &s->tsr, 1);
> -        } else if (qemu_chr_fe_write(&s->chr, &s->tsr, 1) == 0 &&
> -                   s->tsr_retry < MAX_XMIT_RETRY) {
> -            assert(s->watch_tag == 0);
> -            s->watch_tag =
> -                qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
> -                                      serial_watch_cb, s);
> -            if (s->watch_tag > 0) {
> -                s->tsr_retry++;
> -                return;
> +        } else {
> +            int rc = qemu_chr_fe_write(&s->chr, &s->tsr, 1);
> +
> +            if ((rc == 0 ||
> +                 (rc == -1 && errno == EAGAIN)) &&
> +                s->tsr_retry < MAX_XMIT_RETRY) {
> +                assert(s->watch_tag == 0);
> +                s->watch_tag =
> +                    qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
> +                                          serial_watch_cb, s);
> +                if (s->watch_tag > 0) {
> +                    s->tsr_retry++;
> +                    return;
> +                }
>              }
>          }
>          s->tsr_retry = 0;
> 

Queued, thanks.

Paolo