| Message ID | 1353850754-22704-1-git-send-email-alevy@redhat.com |
|---|---|
| State | New |
| Headers | show |
On (Sun) 25 Nov 2012 [15:39:14], Alon Levy wrote: > When migrating a host with with a spice agent running the mouse becomes > non operational after the migration due to the agent state being > inconsistent between the guest and the client if the client is using > semi-seamless or switch host migration. > > After migration the target client has never received the guest_open > initiated spice message. Virtio-serial holds this state information and > migrates it, so replay that over the chardev post migration. Fix is not > spice specific but spice is the only client that cares about it. Thanks for continuing to pursue this :) > rhbz #725965. > --- > hw/virtio-serial-bus.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > index efa8a81..ccce1fa 100644 > --- a/hw/virtio-serial-bus.c > +++ b/hw/virtio-serial-bus.c > @@ -641,6 +641,7 @@ static void virtio_serial_post_load_timer_cb(void *opaque) > VirtIOSerial *s = opaque; > VirtIOSerialPort *port; > uint8_t host_connected; > + VirtIOSerialPortClass *vsc; > > for (i = 0 ; i < s->post_load.nr_active_ports; ++i) { > port = s->post_load.connected[i].port; > @@ -653,6 +654,11 @@ static void virtio_serial_post_load_timer_cb(void *opaque) > send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, > port->host_connected); > } > + vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port); > + if (port->guest_connected && vsc->guest_open) { > + /* replay guest open */ > + vsc->guest_open(port); > + } I think the last time we discussed this, my objection was the guest isn't really doing an open again, and since spice depends on the guest's connectedness, spice should have a post-load hook or a similar bh that would query virtio-serial for guest connectedness status, and, if connected, do whatever setup is necessary. Adding Gerd and Markus as I think they were involved in the discussion last time as well. Amit
Amit Shah <amit.shah@redhat.com> writes: > On (Sun) 25 Nov 2012 [15:39:14], Alon Levy wrote: >> When migrating a host with with a spice agent running the mouse becomes >> non operational after the migration due to the agent state being >> inconsistent between the guest and the client if the client is using >> semi-seamless or switch host migration. >> >> After migration the target client has never received the guest_open >> initiated spice message. Virtio-serial holds this state information and >> migrates it, so replay that over the chardev post migration. Fix is not >> spice specific but spice is the only client that cares about it. > > Thanks for continuing to pursue this :) > >> rhbz #725965. >> --- >> hw/virtio-serial-bus.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c >> index efa8a81..ccce1fa 100644 >> --- a/hw/virtio-serial-bus.c >> +++ b/hw/virtio-serial-bus.c >> @@ -641,6 +641,7 @@ static void virtio_serial_post_load_timer_cb(void *opaque) >> VirtIOSerial *s = opaque; >> VirtIOSerialPort *port; >> uint8_t host_connected; >> + VirtIOSerialPortClass *vsc; >> >> for (i = 0 ; i < s->post_load.nr_active_ports; ++i) { >> port = s->post_load.connected[i].port; >> @@ -653,6 +654,11 @@ static void virtio_serial_post_load_timer_cb(void *opaque) >> send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, >> port->host_connected); >> } >> + vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port); >> + if (port->guest_connected && vsc->guest_open) { >> + /* replay guest open */ >> + vsc->guest_open(port); >> + } > > I think the last time we discussed this, my objection was the guest > isn't really doing an open again, and since spice depends on the > guest's connectedness, spice should have a post-load hook or a similar > bh that would query virtio-serial for guest connectedness status, and, > if connected, do whatever setup is necessary. > > Adding Gerd and Markus as I think they were involved in the discussion > last time as well. Got a pointer to the old thread?
On (Tue) 27 Nov 2012 [15:10:06], Markus Armbruster wrote: > Amit Shah <amit.shah@redhat.com> writes: > > Adding Gerd and Markus as I think they were involved in the discussion > > last time as well. > > Got a pointer to the old thread? https://lists.nongnu.org/archive/html/qemu-devel/2011-07/msg02986.html I didn't go through it entirely, but I'm looking for guidance here -- should spice handle the migration (as I suggest), or should virtio-serial-bus replay guest_open (as this patch by Alon does)? Amit
Amit Shah <amit.shah@redhat.com> writes: > On (Sun) 25 Nov 2012 [15:39:14], Alon Levy wrote: >> When migrating a host with with a spice agent running the mouse becomes >> non operational after the migration due to the agent state being >> inconsistent between the guest and the client if the client is using >> semi-seamless or switch host migration. >> >> After migration the target client has never received the guest_open >> initiated spice message. Virtio-serial holds this state information and >> migrates it, so replay that over the chardev post migration. Fix is not >> spice specific but spice is the only client that cares about it. > > Thanks for continuing to pursue this :) > >> rhbz #725965. >> --- >> hw/virtio-serial-bus.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c >> index efa8a81..ccce1fa 100644 >> --- a/hw/virtio-serial-bus.c >> +++ b/hw/virtio-serial-bus.c >> @@ -641,6 +641,7 @@ static void virtio_serial_post_load_timer_cb(void *opaque) >> VirtIOSerial *s = opaque; >> VirtIOSerialPort *port; >> uint8_t host_connected; >> + VirtIOSerialPortClass *vsc; >> >> for (i = 0 ; i < s->post_load.nr_active_ports; ++i) { >> port = s->post_load.connected[i].port; >> @@ -653,6 +654,11 @@ static void virtio_serial_post_load_timer_cb(void *opaque) >> send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, >> port->host_connected); >> } >> + vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port); >> + if (port->guest_connected && vsc->guest_open) { >> + /* replay guest open */ >> + vsc->guest_open(port); >> + } > > I think the last time we discussed this, my objection was the guest > isn't really doing an open again, and since spice depends on the > guest's connectedness, spice should have a post-load hook or a similar > bh that would query virtio-serial for guest connectedness status, and, > if connected, do whatever setup is necessary. Agreed. Regards, Anthony Liguori > > Adding Gerd and Markus as I think they were involved in the discussion > last time as well. > > Amit
Adds a new char device backend callback to check connectedness, implemented for virtio console, and used by spice-char-dev in post migration. Is using NULL for DeviceState the intention for non device vmstates? It works fine in practice. Alon Levy (3): virtio-serial: add virtio_serial_guest_connected qemu-char: add qemu_chr_be_connected spice-qemu-char: register interface on post load backends/rng-egd.c | 4 ++-- gdbstub.c | 2 +- hw/ccid-card-passthru.c | 1 + hw/debugcon.c | 2 +- hw/ivshmem.c | 8 ++++---- hw/qdev-properties.c | 2 +- hw/serial.c | 4 ++-- hw/usb/dev-serial.c | 4 ++-- hw/usb/redirect.c | 2 +- hw/virtio-console.c | 12 ++++++++++-- hw/virtio-serial-bus.c | 9 +++++++++ hw/virtio-serial.h | 5 +++++ main-loop.h | 1 + monitor.c | 4 ++-- net/slirp.c | 2 +- qemu-char.c | 11 ++++++++++- qemu-char.h | 13 +++++++++++++ qtest.c | 2 +- spice-qemu-char.c | 34 ++++++++++++++++++++++++++++++++++ 19 files changed, 101 insertions(+), 21 deletions(-)
On (Wed) 28 Nov 2012 [11:05:37], Alon Levy wrote: > Adds a new char device backend callback to check connectedness, implemented > for virtio console, and used by spice-char-dev in post migration. > > Is using NULL for DeviceState the intention for non device vmstates? It works > fine in practice. Any more opinions / reviews on this series? Anthony? Amit
Amit Shah <amit.shah@redhat.com> writes: > On (Wed) 28 Nov 2012 [11:05:37], Alon Levy wrote: >> Adds a new char device backend callback to check connectedness, implemented >> for virtio console, and used by spice-char-dev in post migration. >> >> Is using NULL for DeviceState the intention for non device vmstates? It works >> fine in practice. > > Any more opinions / reviews on this series? Anthony? Spice should save its state and not rely on the chardev layer to replay it. I thought this was already pointed out in this thread? Regards, Anthony Liguori > > Amit
On (Thu) 13 Dec 2012 [08:25:08], Anthony Liguori wrote: > Amit Shah <amit.shah@redhat.com> writes: > > > On (Wed) 28 Nov 2012 [11:05:37], Alon Levy wrote: > >> Adds a new char device backend callback to check connectedness, implemented > >> for virtio console, and used by spice-char-dev in post migration. > >> > >> Is using NULL for DeviceState the intention for non device vmstates? It works > >> fine in practice. > > > > Any more opinions / reviews on this series? Anthony? > > Spice should save its state and not rely on the chardev layer to replay > it. > > I thought this was already pointed out in this thread? Indeed. Alon, just adding save/load to spice-qemu-char.c doesn't work for you? Amit
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index efa8a81..ccce1fa 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -641,6 +641,7 @@ static void virtio_serial_post_load_timer_cb(void *opaque) VirtIOSerial *s = opaque; VirtIOSerialPort *port; uint8_t host_connected; + VirtIOSerialPortClass *vsc; for (i = 0 ; i < s->post_load.nr_active_ports; ++i) { port = s->post_load.connected[i].port; @@ -653,6 +654,11 @@ static void virtio_serial_post_load_timer_cb(void *opaque) send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, port->host_connected); } + vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port); + if (port->guest_connected && vsc->guest_open) { + /* replay guest open */ + vsc->guest_open(port); + } } g_free(s->post_load.connected); s->post_load.connected = NULL;