From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45382) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFkHM-0002og-NO for qemu-devel@nongnu.org; Wed, 22 Jun 2016 11:41:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bFkHG-0000L0-KW for qemu-devel@nongnu.org; Wed, 22 Jun 2016 11:41:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59988) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFkHG-0000Kt-Es for qemu-devel@nongnu.org; Wed, 22 Jun 2016 11:41:18 -0400 Date: Wed, 22 Jun 2016 16:41:14 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20160622154114.GH6604@work-vm> References: <1466432945-28682-1-git-send-email-pbonzini@redhat.com> <1466432945-28682-3-git-send-email-pbonzini@redhat.com> <20160622150544.GC6604@work-vm> <5485a609-f1ff-e147-64bc-1def08621561@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5485a609-f1ff-e147-64bc-1def08621561@redhat.com> 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: Paolo Bonzini Cc: qemu-devel@nongnu.org, bcketchum@gmail.com * Paolo Bonzini (pbonzini@redhat.com) wrote: > > > 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"); > > ? Yeh, feel free to add tsr_retry and lsr to the message as well so that if you're looking at it you can figure out what happened. Dave > > Paolo -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK