On 03/08/2016 18:22, 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 the host side is not connected the handle_output() > method in virtio-serial-bus.c will drop any data sent > by the guest, before it even reaches the virtio-console.c > code. This means that if the chardev has a logfile > configured, the data will never get logged. > > 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. > > There is no need for virtio-serial-bus.c to aggressively > drop the data for console devices, as the chardev code is > prefectly capable of discarding the data itself. > > So this patch changes virtconsole devices so that they > are always marked as having the host side open. This > ensures that the guest OS will always send any data it > has (Linux virtio-console hvc driver actually ignores > the host open state and sends data regardless, but we > should not rely on that), and also prevents the > virtio-serial-bus code prematurely discarding data. > > The behaviour of virtserialport devices is *not* changed, > only virtconsole, because for the former, it is important > that the guest OSknow exactly when the host side is opened > / closed so it can do any protocol re-negotiation that may > be required. > > Fixes bug: https://bugs.launchpad.net/qemu/+bug/1599214 > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > --- > hw/char/virtio-console.c | 25 +++++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > 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); > + } > } > } > >
I think this patch should go in 2.7, perhaps patch 2 too. Everything else can wait for 2.8. Paolo
