Re: [Qemu-devel] [PATCH 7/8] spice-qemu-char: Register interface on post load

[Alon Levy]

> On 03/14/13 17:36, Hans de Goede wrote:
> > From: Alon Levy <address@hidden>
> > 
> > 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.

But they will both get a different state pointer s.

> 
> 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 ...

It is being migrated in virtio-serial-bus (port->guest_connected) but we need 
to pass that on to spice-server, which there is no hook for in the chardev 
layer, I could add one. I would need to query the backend, in this case 
virtio-serial-bus, something like this:

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index e2d1c58..f36a973 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -88,6 +88,16 @@ static void guest_close(VirtIOSerialPort *port)
     qemu_chr_fe_close(vcon->chr);
 }
 
+static void guest_migrated(VirtIOSerialPort *port)
+{
+    VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+    if (!vcon->chr) {
+        return;
+    }
+    qemu_chr_fe_migrated(vcon->chr, port->guest_connected);
+}
+
 /* Readiness of the guest to accept data on a port */
 static int chr_can_read(void *opaque)
 {
@@ -177,6 +187,7 @@ static void virtserialport_class_init(ObjectClass *klass, 
void *data)
     k->have_data = flush_buf;
     k->guest_open = guest_open;
     k->guest_close = guest_close;
+    k->guest_migrated = guest_migrated;
     dc->props = virtserialport_properties;
 }
 
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 7d0515f..b6b1c3b 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -664,6 +664,7 @@ static int fetch_active_ports_list(QEMUFile *f, int 
version_id,
     /* Items in struct VirtIOSerialPort */
     for (i = 0; i < nr_active_ports; i++) {
         VirtIOSerialPort *port;
+        VirtIOSerialPortClass *vsc;
         uint32_t id;
 
         id = qemu_get_be32(f);
@@ -671,6 +672,7 @@ static int fetch_active_ports_list(QEMUFile *f, int 
version_id,
         if (!port) {
             return -EINVAL;
         }
+        vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
 
         port->guest_connected = qemu_get_byte(f);
         s->post_load->connected[i].port = port;
@@ -698,6 +700,7 @@ static int fetch_active_ports_list(QEMUFile *f, int 
version_id,
                 virtio_serial_throttle_port(port, false);
             }
         }
+        vsc->guest_migrated(port);
     }
     qemu_mod_timer(s->post_load->timer, 1);
     return 0;
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index d2d9fb7..992ef55 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -107,6 +107,8 @@ typedef struct VirtIOSerialPortClass {
      */
     ssize_t (*have_data)(VirtIOSerialPort *port, const uint8_t *buf,
                          size_t len);
+
+    void (*guest_migrated)(VirtIOSerialPort *port);
 } VirtIOSerialPortClass;
 
 /*
diff --git a/include/char/char.h b/include/char/char.h
index d6a0351..b7c8cb1 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -70,6 +70,7 @@ struct CharDriverState {
     void (*chr_set_echo)(struct CharDriverState *chr, bool echo);
     void (*chr_guest_open)(struct CharDriverState *chr);
     void (*chr_guest_close)(struct CharDriverState *chr);
+    void (*chr_guest_migrated)(struct CharDriverState *chr, int connected);
     void *opaque;
     int idle_tag;
     char *label;
@@ -143,6 +144,8 @@ void qemu_chr_fe_open(struct CharDriverState *chr);
  */
 void qemu_chr_fe_close(struct CharDriverState *chr);
 
+void qemu_chr_fe_migrated(struct CharDriverState *chr, int connected);
+
 /**
  * @qemu_chr_fe_printf:
  *
diff --git a/qemu-char.c b/qemu-char.c
index e633797..cda785f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3390,6 +3390,13 @@ void qemu_chr_fe_open(struct CharDriverState *chr)
     }
 }
 
+void qemu_chr_fe_migrated(struct CharDriverState *chr, int connected)
+{
+    if (chr->chr_guest_migrated) {
+        chr->chr_guest_migrated(chr, connected);
+    }
+}
+
 void qemu_chr_fe_close(struct CharDriverState *chr)
 {
     if (chr->chr_guest_close) {
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 8a9236d..11a1888 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -205,6 +205,14 @@ static void print_allowed_subtypes(void)
     fprintf(stderr, "\n");
 }
 
+static void spice_chr_guest_migrated(struct CharDriverState *chr,
+                                     int connected)
+{
+    if (connected) {
+        // setup timer for spice_chr_guest_open
+    }
+}
+
 static CharDriverState *chr_open(const char *subtype)
 {
     CharDriverState *chr;
@@ -220,6 +228,7 @@ static CharDriverState *chr_open(const char *subtype)
     chr->chr_close = spice_chr_close;
     chr->chr_guest_open = spice_chr_guest_open;
     chr->chr_guest_close = spice_chr_guest_close;
+    chr->chr_guest_migrated = spice_chr_guest_migrated;
 
     QLIST_INSERT_HEAD(&spice_chars, s, next);
 


> 
> cheers,
>   Gerd
> 
>