On 07/07/2017 16:21, Pavel Butsykin wrote: > We should guarantee that RAM will not be modified while VM has a stopped > state, otherwise it can lead to negative consequences during post-copy > migration. In RUN_STATE_FINISH_MIGRATE step, it's expected that RAM on > source side will not be modified as this could lead to non-consistent vm state > on the destination side. Also RAM access during postcopy-ram migration with > enabled release-ram capability can lead to sad consequences. > > Let's add enable_backend() callback to avoid undesirable virtioqueue changes > in the guest memory. > > Signed-off-by: Pavel Butsykin <pbutsy...@virtuozzo.com>
To understand the patch better this doesn't fix _all_ stopped states, only migration, right? That said it's a valid bugfix even independent of the effects for stopped runstate. Thanks, Paolo > --- > hw/char/virtio-console.c | 21 +++++++++++++++++++++ > hw/char/virtio-serial-bus.c | 7 +++++++ > include/hw/virtio/virtio-serial.h | 3 +++ > 3 files changed, 31 insertions(+) > > diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c > index 0cb1668c8a..b55905892e 100644 > --- a/hw/char/virtio-console.c > +++ b/hw/char/virtio-console.c > @@ -163,6 +163,26 @@ static void chr_event(void *opaque, int event) > } > } > > +static void virtconsole_enable_backend(VirtIOSerialPort *port, bool enable) > +{ > + VirtConsole *vcon = VIRTIO_CONSOLE(port); > + > + if (!qemu_chr_fe_get_driver(&vcon->chr)) { > + return; > + } > + > + if (enable) { > + VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port); > + > + qemu_chr_fe_set_handlers(&vcon->chr, chr_can_read, chr_read, > + k->is_console ? NULL : chr_event, > + vcon, NULL, false); > + } else { > + qemu_chr_fe_set_handlers(&vcon->chr, NULL, NULL, > + NULL, NULL, NULL, false); > + } > +} > + > static void virtconsole_realize(DeviceState *dev, Error **errp) > { > VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(dev); > @@ -233,6 +253,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->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 f5bc173844..f0f18c8e7c 100644 > --- a/hw/char/virtio-serial-bus.c > +++ b/hw/char/virtio-serial-bus.c > @@ -637,6 +637,13 @@ static void set_status(VirtIODevice *vdev, uint8_t > status) > if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) { > guest_reset(vser); > } > + > + QTAILQ_FOREACH(port, &vser->ports, next) { > + VirtIOSerialPortClass *vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port); > + if (vsc->enable_backend) { > + vsc->enable_backend(port, vdev->vm_running); > + } > + } > } > > static void vser_reset(VirtIODevice *vdev) > diff --git a/include/hw/virtio/virtio-serial.h > b/include/hw/virtio/virtio-serial.h > index b19c44727f..12657a9f39 100644 > --- a/include/hw/virtio/virtio-serial.h > +++ b/include/hw/virtio/virtio-serial.h > @@ -58,6 +58,9 @@ typedef struct VirtIOSerialPortClass { > /* Guest opened/closed device. */ > void (*set_guest_connected)(VirtIOSerialPort *port, int guest_connected); > > + /* Enable/disable backend for virtio serial port */ > + void (*enable_backend)(VirtIOSerialPort *port, bool enable); > + > /* Guest is now ready to accept data (virtqueues set up). */ > void (*guest_ready)(VirtIOSerialPort *port); > >