| Message ID | CAJ+F1CJVQn5zucYgn9dUFOu76MZkqd2Vaep52c89AORWF=sOrA@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13/07/2018 16:55, Marc-André Lureau wrote: > - 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; > + } I think EINTR should be handled by chardev, otherwise yes. Paolo
On Fri, 13 Jul 2018 16:55:15 +0200 Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > Hi > > On Tue, Jul 10, 2018 at 3:48 PM, Igor Mammedov <imammedo@redhat.com> wrote: > > On Tue, 5 Jun 2018 11:18:35 +0200 > > Paolo Bonzini <pbonzini@redhat.com> 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 <slp@redhat.com> > >> > --- > >> > 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 <slp@redhat.com> > > 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? it fixes windbg over serial (with and without EINTR) > 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; > >
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;