Patchwork [7/8] spice-qemu-char: Register interface on post load

login
register
mail settings
Submitter Hans de Goede
Date March 14, 2013, 4:36 p.m.
Message ID <1363279017-3894-8-git-send-email-hdegoede@redhat.com>
Download mbox | patch
Permalink /patch/227736/
State New
Headers show

Comments

Hans de Goede - March 14, 2013, 4:36 p.m.
From: Alon Levy <alevy@redhat.com>

The target has not seen the guest_connected event via
spice_chr_guest_open or spice_chr_write, and so spice server wrongly
assumes there is no agent active, while the client continues to send
motion events only by the agent channel, which the server ignores. The
net effect is that the mouse is static in the guest.

By registering the interface on post load spice server will pass on the
agent messages fixing the mouse behavior after migration.

RHBZ #725965

HdG: Port to new chardev code and test

Signed-off-by: Alon Levy <alevy@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 spice-qemu-char.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)
Gerd Hoffmann - March 19, 2013, 7:05 a.m.
On 03/14/13 17:36, Hans de Goede wrote:
> From: Alon Levy <alevy@redhat.com>
> 
> The target has not seen the guest_connected event via
> spice_chr_guest_open or spice_chr_write, and so spice server wrongly
> assumes there is no agent active, while the client continues to send
> motion events only by the agent channel, which the server ignores. The
> net effect is that the mouse is static in the guest.
> 
> By registering the interface on post load spice server will pass on the
> agent messages fixing the mouse behavior after migration.

> +static VMStateDescription spice_chr_vmstate = {
> +    .name               = "spice-chr",
> +    .version_id         = 1,
> +    .minimum_version_id = 1,
> +    .post_load          = spice_chr_post_load,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(guest_open, SpiceCharDriver),
> +        VMSTATE_END_OF_LIST()
> +    },
> +};

> +    vmstate_register(NULL, -1, &spice_chr_vmstate, s);
> +

That is a showstopper.  If there are two of these there is no reliable
way to figure which is which.

Do we actually need that in the first place?  I assume virtio-serial
will migrate the guest-open state anyway.  So if we can use that somehow
we don't have to live migrate the chardev state ...

cheers,
  Gerd

Patch

diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 530d9a6..bdbe59e 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -2,6 +2,7 @@ 
 #include "trace.h"
 #include "ui/qemu-spice.h"
 #include "char/char.h"
+#include "migration/vmstate.h"
 #include <spice.h>
 #include <spice-experimental.h>
 #include <spice/protocol.h>
@@ -16,6 +17,8 @@  typedef struct SpiceCharDriver {
     bool                  blocked;
     const uint8_t         *datapos;
     int                   datalen;
+    uint32_t              guest_open;
+    QEMUTimer             *post_load_timer;
     QLIST_ENTRY(SpiceCharDriver) next;
 } SpiceCharDriver;
 
@@ -116,6 +119,8 @@  static SpiceCharDeviceInterface vmc_interface = {
 
 static void vmc_register_interface(SpiceCharDriver *scd)
 {
+    scd->guest_open = 1;
+
     if (scd->active) {
         return;
     }
@@ -127,6 +132,8 @@  static void vmc_register_interface(SpiceCharDriver *scd)
 
 static void vmc_unregister_interface(SpiceCharDriver *scd)
 {
+    scd->guest_open = 0;
+
     if (!scd->active) {
         return;
     }
@@ -211,6 +218,35 @@  static void spice_chr_guest_close(struct CharDriverState *chr)
     vmc_unregister_interface(s);
 }
 
+static void spice_chr_post_load_cb(void *opaque)
+{
+    SpiceCharDriver *s = opaque;
+
+    if (s->chr && s->guest_open)
+        spice_chr_guest_open(s->chr);
+}
+
+static int spice_chr_post_load(void *opaque, int version_id)
+{
+    SpiceCharDriver *s = opaque;
+
+    if (s->chr && s->guest_open)
+        qemu_mod_timer(s->post_load_timer, 1);
+
+    return 0;
+}
+
+static VMStateDescription spice_chr_vmstate = {
+    .name               = "spice-chr",
+    .version_id         = 1,
+    .minimum_version_id = 1,
+    .post_load          = spice_chr_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(guest_open, SpiceCharDriver),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
 static void spice_chr_close(struct CharDriverState *chr)
 {
     SpiceCharDriver *s = chr->opaque;
@@ -218,6 +254,10 @@  static void spice_chr_close(struct CharDriverState *chr)
     vmc_unregister_interface(s);
     QLIST_REMOVE(s, next);
 
+    qemu_del_timer(s->post_load_timer);
+    qemu_free_timer(s->post_load_timer);
+    vmstate_unregister(NULL, &spice_chr_vmstate, s);
+
     g_free((char *)s->sin.subtype);
 #if SPICE_SERVER_VERSION >= 0x000c02
     g_free((char *)s->sin.portname);
@@ -252,6 +292,8 @@  static CharDriverState *chr_open(const char *subtype)
     s->chr = chr;
     s->active = false;
     s->sin.subtype = g_strdup(subtype);
+    s->post_load_timer = qemu_new_timer_ns(vm_clock,
+                                           spice_chr_post_load_cb, s);
     chr->opaque = s;
     chr->chr_write = spice_chr_write;
     chr->chr_add_watch = spice_chr_add_watch;
@@ -259,6 +301,8 @@  static CharDriverState *chr_open(const char *subtype)
     chr->chr_guest_open = spice_chr_guest_open;
     chr->chr_guest_close = spice_chr_guest_close;
 
+    vmstate_register(NULL, -1, &spice_chr_vmstate, s);
+
     QLIST_INSERT_HEAD(&spice_chars, s, next);
 
     return chr;