qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] hw/char/serial: Only retry if qemu_chr_fe_write


From: Marc-André Lureau
Subject: Re: [Qemu-devel] [PATCH] hw/char/serial: Only retry if qemu_chr_fe_write returns 0
Date: Fri, 13 Jul 2018 16:55:15 +0200

Hi

On Tue, Jul 10, 2018 at 3:48 PM, Igor Mammedov <address@hidden> wrote:
> On Tue, 5 Jun 2018 11:18:35 +0200
> Paolo Bonzini <address@hidden> wrote:
>
>> On 05/06/2018 09:54, Sergio Lopez wrote:
>> > Only retry on serial_xmit if qemu_chr_fe_write returns 0, as this is the
>> > only recoverable error.
>> >
>> > Retrying with any other scenario, in addition to being a waste of CPU
>> > cycles, can compromise the Guest stability if by the vCPU issuing the
>> > write and the main loop thread are, by chance or explicit pinning,
>> > running on the same pCPU.
>> >
>> > Previous discussion:
>> >
>> > https://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg06998.html
>> >
>> > Signed-off-by: Sergio Lopez <address@hidden>
>> > ---
>> >  hw/char/serial.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/hw/char/serial.c b/hw/char/serial.c
>> > index 605b0d0..6de6c29 100644
>> > --- a/hw/char/serial.c
>> > +++ b/hw/char/serial.c
>> > @@ -260,7 +260,7 @@ 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) != 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 =
>> >
> Hi Sergio, Paolo,
>
> it looks like commit
> commit 019288bf137183bf3407c9824655b753bfafc99f
> Author: Sergio Lopez <address@hidden>
> Date:   Tue Jun 5 03:54:55 2018 -0400
>
>     hw/char/serial: Only retry if qemu_chr_fe_write returns 0
>
> introduced regression wrt 2.12 and broke windows guest remote kernel debug 
> over
> serial, where windbg can't connect to target and target hangs when windbg 
> tries
> to connect to it.
>
> Reverting the commit fixes issue
>

is this a possible solution?

diff --git a/hw/char/serial.c b/hw/char/serial.c
index cd7d747c68..046c4684ff 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 || errno == EINTR))) &&
+                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;


-- 
Marc-André Lureau



reply via email to

[Prev in Thread] Current Thread [Next in Thread]