On 01/11/18 16:11, Artem Pisarenko wrote:
> This fixes wrong interfacing between virtio serial port and bus
> models, and corresponding chardev backends, caused extra and incorrect
> activity during guest boot process (when virtserialport device used).
>
> Signed-off-by: Artem Pisarenko <artem.k.pisare...@gmail.com>
> ---
>
> Notes:
> Although this doesn't trigger any issue/bug (known to me), this may
> prevent them in future.
> Also this patch optimizes emulation performance by avoiding extra
> activity and fixes a case, when serial port wasn't closed if backend closed
> while being disabled (even worse - serial port will open again!).
Marc-André, can you review this?
Thanks,
Paolo
> hw/char/virtio-console.c | 24 ++++++++++++++++++++----
> hw/char/virtio-serial-bus.c | 8 +++++++-
> include/hw/virtio/virtio-serial.h | 8 ++++++++
> 3 files changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
> index 2cbe1d4..634e9bf 100644
> --- a/hw/char/virtio-console.c
> +++ b/hw/char/virtio-console.c
> @@ -25,6 +25,7 @@
> typedef struct VirtConsole {
> VirtIOSerialPort parent_obj;
>
> + bool backend_active;
> CharBackend chr;
> guint watch;
> } VirtConsole;
> @@ -149,6 +150,11 @@ static void chr_event(void *opaque, int event)
> VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(vcon);
>
> trace_virtio_console_chr_event(port->id, event);
> +
> + if (!vcon->backend_active) {
> + return;
> + }
> +
> switch (event) {
> case CHR_EVENT_OPENED:
> virtio_serial_open(port);
> @@ -187,17 +193,24 @@ static int chr_be_change(void *opaque)
> return 0;
> }
>
> +static bool virtconsole_is_backend_enabled(VirtIOSerialPort *port)
> +{
> + VirtConsole *vcon = VIRTIO_CONSOLE(port);
> +
> + return vcon->backend_active;
> +}
> +
> static void virtconsole_enable_backend(VirtIOSerialPort *port, bool enable)
> {
> VirtConsole *vcon = VIRTIO_CONSOLE(port);
>
> - if (!qemu_chr_fe_backend_connected(&vcon->chr)) {
> - return;
> - }
> -
> if (enable) {
> VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
>
> + if (!k->is_console && virtio_serial_is_opened(port)
> + && !qemu_chr_fe_backend_open(&vcon->chr)) {
> + virtio_serial_close(port);
> + }
> qemu_chr_fe_set_handlers(&vcon->chr, chr_can_read, chr_read,
> k->is_console ? NULL : chr_event,
> chr_be_change, vcon, NULL, false);
> @@ -205,6 +218,7 @@ static void virtconsole_enable_backend(VirtIOSerialPort
> *port, bool enable)
> qemu_chr_fe_set_handlers(&vcon->chr, NULL, NULL, NULL,
> NULL, NULL, NULL, false);
> }
> + vcon->backend_active = enable;
> }
>
> static void virtconsole_realize(DeviceState *dev, Error **errp)
> @@ -220,6 +234,7 @@ static void virtconsole_realize(DeviceState *dev, Error
> **errp)
> }
>
> if (qemu_chr_fe_backend_connected(&vcon->chr)) {
> + vcon->backend_active = true;
> /*
> * For consoles we don't block guest data transfer just
> * because nothing is connected - we'll just let it go
> @@ -278,6 +293,7 @@ static void virtserialport_class_init(ObjectClass *klass,
> void *data)
> k->unrealize = virtconsole_unrealize;
> k->have_data = flush_buf;
> k->set_guest_connected = set_guest_connected;
> + k->is_backend_enabled = virtconsole_is_backend_enabled;
> k->enable_backend = virtconsole_enable_backend;
> k->guest_writable = guest_writable;
> dc->props = virtserialport_properties;
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index 04e3ebe..d23d99d 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -258,6 +258,11 @@ static size_t send_control_event(VirtIOSerial *vser,
> uint32_t port_id,
> }
>
> /* Functions for use inside qemu to open and read from/write to ports */
> +bool virtio_serial_is_opened(VirtIOSerialPort *port)
> +{
> + return port->host_connected;
> +}
> +
> int virtio_serial_open(VirtIOSerialPort *port)
> {
> /* Don't allow opening an already-open port */
> @@ -643,7 +648,8 @@ static void set_status(VirtIODevice *vdev, uint8_t status)
>
> QTAILQ_FOREACH(port, &vser->ports, next) {
> VirtIOSerialPortClass *vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
> - if (vsc->enable_backend) {
> + if (vsc->is_backend_enabled && vsc->enable_backend
> + && (vsc->is_backend_enabled(port) != vdev->vm_running)) {
> vsc->enable_backend(port, vdev->vm_running);
> }
> }
> diff --git a/include/hw/virtio/virtio-serial.h
> b/include/hw/virtio/virtio-serial.h
> index 12657a9..4b72562 100644
> --- a/include/hw/virtio/virtio-serial.h
> +++ b/include/hw/virtio/virtio-serial.h
> @@ -58,6 +58,8 @@ typedef struct VirtIOSerialPortClass {
> /* Guest opened/closed device. */
> void (*set_guest_connected)(VirtIOSerialPort *port, int guest_connected);
>
> + /* Is backend currently enabled for virtio serial port */
> + bool (*is_backend_enabled)(VirtIOSerialPort *port);
> /* Enable/disable backend for virtio serial port */
> void (*enable_backend)(VirtIOSerialPort *port, bool enable);
>
> @@ -194,6 +196,12 @@ struct VirtIOSerial {
> /* Interface to the virtio-serial bus */
>
> /*
> + * Checks status of connection to the port
> + * Returns true if connected, false otherwise.
> + */
> +bool virtio_serial_is_opened(VirtIOSerialPort *port);
> +
> +/*
> * Open a connection to the port
> * Returns 0 on success (always).
> */
>
