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

[Daniel P. Berrange]

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

diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 2e36481..4f0e03d 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -85,8 +85,9 @@ static void set_guest_connected(VirtIOSerialPort *port, int 
guest_connected)
 {
     VirtConsole *vcon = VIRTIO_CONSOLE(port);
     DeviceState *dev = DEVICE(port);
+    VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
 
-    if (vcon->chr) {
+    if (vcon->chr && !k->is_console) {
         qemu_chr_fe_set_open(vcon->chr, guest_connected);
     }
 
@@ -156,9 +157,25 @@ static void virtconsole_realize(DeviceState *dev, Error 
**errp)
     }
 
     if (vcon->chr) {
-        vcon->chr->explicit_fe_open = 1;
-        qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event,
-                              vcon);
+        /*
+         * For consoles we don't block guest data transfer just
+         * because nothing is connected - we'll just let it go
+         * whetherever the chardev wants - /dev/null probably.
+         *
+         * For serial ports we need 100% reliable data transfer
+         * so we use the opened/closed signals from chardev to
+         * trigger open/close of the device
+         */
+        if (k->is_console) {
+            vcon->chr->explicit_fe_open = 0;
+            qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read,
+                                  NULL, vcon);
+            virtio_serial_open(port);
+        } else {
+            vcon->chr->explicit_fe_open = 1;
+            qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read,
+                                  chr_event, vcon);
+        }
     }
 }
 
-- 
2.5.5