From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50008) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bT9fY-0007t7-Ue for qemu-devel@nongnu.org; Fri, 29 Jul 2016 11:25:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bT9fX-0008Nx-9l for qemu-devel@nongnu.org; Fri, 29 Jul 2016 11:25:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50688) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bT9fX-0008Nd-1N for qemu-devel@nongnu.org; Fri, 29 Jul 2016 11:25:47 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B17A76370E for ; Fri, 29 Jul 2016 15:25:45 +0000 (UTC) Date: Fri, 29 Jul 2016 16:25:42 +0100 From: "Daniel P. Berrange" Message-ID: <20160729152542.GL4485@redhat.com> Reply-To: "Daniel P. Berrange" References: <1469791063-28222-1-git-send-email-berrange@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1469791063-28222-1-git-send-email-berrange@redhat.com> Subject: Re: [Qemu-devel] [PATCH] virtio-console: set frontend open permanently for console devs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Amit Shah , Paolo Bonzini On Fri, Jul 29, 2016 at 12:17:43PM +0100, Daniel P. Berrange wrote: > The virtio-console.c file handles both serial consoles > and interactive consoles, since they're backed by the > same device model. > > Since serial devices are expected to be reliable and > need to notify the guest when the backend is opened > or closed, the virtio-console.c file wires up support > for chardev events. This affects both serial consoles > and interactive consoles, using a network connection > based chardev backend such as 'socket', but not when > using a PTY based backend or plain 'file' backends. > > When a device is open, however, the behaviour is > different - if the backend chardev returns EAGAIN or > a short write, the serial console will block and > setup a watch to poll for writability, ensuring no > data is lost. The interactive consoles meanwhile > will simply discard data. > > This means that the interactive consoles have different > blocking behaviour depending on whether the chardev is > open or not. If open, data may be discarded if not > consumed, where as if closed, data will always be queued > pending an open. > > This behaviour is unhelpful in general - applications > outputting messages on the guest console should not be > blocked simply because no client is conencted to the host > side. > > Consider for example, configuring a x86_64 guest with a > plain UART serial port > > -chardev socket,id=charserial1,host=127.0.0.1,port=9001,server,nowait,logfile=console1.log,logappend=on > -device isa-serial,chardev=charserial1,id=serial1 > > vs a s390 guest which has to use the virtio-console port > > -chardev socket,id=charconsole1,host=127.0.0.1,port=9000,server,nowait,logfile=console2.log,logappend=on > -device virtconsole,chardev=charconsole1,id=console1 > > The isa-serial one gets data written to the log regardless > of whether a client is connected, while the virtioconsole > one only gets data written to the log when a client is > connected. > > This patch changes the behaviour so that virtconsole > devices work in same way as other traditional console > devices. Specifically, the frontend will now be marked > as permanently open, so data flows regardless of the > backend status. > > NB, the behaviour of virtserialport devices is *not* > changed, only virtconsole. > > Fixes bug: https://bugs.launchpad.net/qemu/+bug/1599214 > > Signed-off-by: Daniel P. Berrange > --- > hw/char/virtio-console.c | 25 +++++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > NB If there is a concern about backends compatibility with > this change, we could instead add a boolean property to > the virtio-console device 'explicit-open' which controls > whether the virtconsole device has the old behaviour or > the new behaviour and default to old. Personally I think > it is fine to just change behaviour for virtconsole > unconditionally though Ignore this patch for now. Looking around the users of chardev code I've noticed more inconsistency. So I want to analyse the broad usage and report on semantics across all, before proposing potential multiple fixes. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|