From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48079) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cYUuX-0002hi-Vd for qemu-devel@nongnu.org; Tue, 31 Jan 2017 04:39:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cYUuU-0006JM-7A for qemu-devel@nongnu.org; Tue, 31 Jan 2017 04:39:38 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35004) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cYUuT-0006Hj-VB for qemu-devel@nongnu.org; Tue, 31 Jan 2017 04:39:34 -0500 Date: Tue, 31 Jan 2017 09:39:27 +0000 From: "Daniel P. Berrange" Message-ID: <20170131093927.GA20303@redhat.com> Reply-To: "Daniel P. Berrange" References: <20170127094033.GC26553@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [libvirt] char: Logging serial pty output when disconnected List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ed Swierk Cc: qemu-devel@nongnu.org, libvir-list@redhat.com, pbonzini@redhat.com, marcandre.lureau@redhat.com, Eric Blake On Mon, Jan 30, 2017 at 08:58:25PM -0800, Ed Swierk wrote: > On Fri, Jan 27, 2017 at 1:40 AM, Daniel P. Berrange wrote: > > On Thu, Jan 26, 2017 at 05:07:16PM -0800, Ed Swierk wrote: > >> Currently qemu_chr_fe_write() calls qemu_chr_fe_write_log() only for > >> data consumed by the backend chr_write function. With the pty backend, > >> pty_chr_write() returns 0 indicating that the data was not consumed > >> when the pty is disconnected. Simply changing it to return len instead > >> of 0 tricks the caller into logging the data even when the pty is > >> disconnected. I don't know what problems this might cause, but one > >> data point is that tcp_chr_write() already happens to work this way. > >> > >> Alternatively, qemu_chr_fe_write() could be modified to log everything > >> passed to it, regardless of how much data chr_write claims to have > >> consumed. The trouble is that the serial device retries writing > >> unconsumed data, so when the pty is disconnected you'd see every > >> character duplicated 4 times in the log file. > >> > >> Any opinions on either approach, or other suggestions? If there are no > >> objections to the first one, I'll prepare a patch. > > > > If the pty backend intends to just drop data into a blackhole when > > no client is connected, then its chr_write() impl should return > > the length of the data discarded, not zero. > > That's exactly the question: when no client is connected, should the > pty backend just drop the data into a black hole, returning the length > of the data discarded? Or should it return 0, letting the frontend > device decide what to do with it? It should return len of data discarded. > > I can't discern a consistent pattern across all the char backends. The > closest analog is the tcp backend, which does discard the data and > return len. In contrast, several backends call > io_channel_send{,_full}(), which returns -1 if the write would block > or fails for any other reason. > > It's not clear there's much the frontend can do to recover from an > error, but there's no consistent pattern across serial devices either. > Most just ignore the return value. But the 16550A serial device > retries 4 times after an error. Changing the pty backend to discard > the data on the first attempt would bypass this retry mechanism. Is > that a problem? I don't think so - retrying in this way is pointless IMHO - it is just going to get the same result on every retry on 99% of occassions. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|