Re: [Qemu-devel] [PATCH] virtio-console: set frontend open permanently for console devs

[Daniel P. Berrange]

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 <address@hidden>
> ---
>  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 :|