| Message ID | 49e9640f3fc040a9c9ab40f93935a6572b34bdff.1541085026.git.artem.k.pisarenko@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtserialport/virtconsole: fix messy opening/closing port | expand |
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.pisarenko@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). > */ >
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). */
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.pisarenko@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!). 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(-)