[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/5] virtio-console: set frontend open permanent
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 1/5] virtio-console: set frontend open permanently for console devs |
Date: |
Wed, 3 Aug 2016 18:37:26 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
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 <address@hidden>
> ---
> 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
- [Qemu-devel] [PATCH 0/5] Global fix / workaround usage of qemu_chr_fe_write, Daniel P. Berrange, 2016/08/03
- [Qemu-devel] [PATCH 4/5] hw: replace most use of qemu_chr_fe_write with qemu_chr_fe_write_all, Daniel P. Berrange, 2016/08/03
- [Qemu-devel] [PATCH 1/5] virtio-console: set frontend open permanently for console devs, Daniel P. Berrange, 2016/08/03
- [Qemu-devel] [PATCH 2/5] impi: check return of qemu_chr_fe_write() for errors, Daniel P. Berrange, 2016/08/03
- [Qemu-devel] [PATCH 3/5] sclpconsole: remove bogus check for -EAGAIN, Daniel P. Berrange, 2016/08/03
- [Qemu-devel] [PATCH 5/5] char: convert qemu_chr_fe_write to qemu_chr_fe_write_all, Daniel P. Berrange, 2016/08/03