From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35801) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFjoA-0006Un-D0 for qemu-devel@nongnu.org; Wed, 22 Jun 2016 11:11:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bFjo3-0008OV-Nn for qemu-devel@nongnu.org; Wed, 22 Jun 2016 11:11:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48110) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFjo3-0008OE-HK for qemu-devel@nongnu.org; Wed, 22 Jun 2016 11:11:07 -0400 References: <1466432945-28682-1-git-send-email-pbonzini@redhat.com> <1466432945-28682-3-git-send-email-pbonzini@redhat.com> <20160622150544.GC6604@work-vm> From: Paolo Bonzini Message-ID: <5485a609-f1ff-e147-64bc-1def08621561@redhat.com> Date: Wed, 22 Jun 2016 17:11:03 +0200 MIME-Version: 1.0 In-Reply-To: <20160622150544.GC6604@work-vm> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/6] serial: reinstate watch after migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: qemu-devel@nongnu.org, bcketchum@gmail.com On 22/06/2016 17:05, Dr. David Alan Gilbert wrote: > * Paolo Bonzini (pbonzini@redhat.com) wrote: >> Otherwise, a serial port can get stuck if it is migrated while flow control >> is in effect. >> >> Signed-off-by: Paolo Bonzini >> --- >> hw/char/serial.c | 16 ++++++++++++++-- >> include/hw/char/serial.h | 1 + >> 2 files changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/hw/char/serial.c b/hw/char/serial.c >> index e65e9e0..6f0a49e 100644 >> --- a/hw/char/serial.c >> +++ b/hw/char/serial.c >> @@ -639,8 +639,20 @@ static int serial_post_load(void *opaque, int version_id) >> if (s->thr_ipending == -1) { >> s->thr_ipending = ((s->iir & UART_IIR_ID) == UART_IIR_THRI); >> } >> - if (s->tsr_retry > MAX_XMIT_RETRY) { >> - s->tsr_retry = MAX_XMIT_RETRY; > > Why remove the check you just added? Because I'm dumb. :) >> + >> + if (s->tsr_retry > 0) { >> + /* tsr_retry > 0 implies LSR.TEMT = 0 (transmitter not empty). */ >> + if (s->lsr & UART_LSR_TEMT) { >> + return -1; >> + } >> + >> + assert(s->watch_tag == 0); >> + s->watch_tag = qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, serial_xmit, s); >> + } else { >> + /* tsr_retry == 0 implies LSR.TEMT = 1 (transmitter empty). */ >> + if (!(s->lsr & UART_LSR_TEMT)) { >> + return -1; > > If you're failing migration (-1) then please error_report to say why > so when someone hits it I can immediately see why. Ok, something like error_report("inconsistent state in serial device"); ? Paolo