[Qemu-devel] [PATCH v2 0/5] add support for spice migration v2

[Qemu-devel] [PATCH v2 0/5] add support for spice migration v2

[Yonit Halperin]

v2:
Notify spice about vm state changes only via spice_server_vm_start/stop
spice repo:  http://cgit.freedesktop.org/~yhalperi/spice/log/?h=seamless-migration.v2
-----------
Hi,

The following series introduces support for keeping the spice session active after migration.
For more details about spice seamless migration, see
http://lists.freedesktop.org/archives/spice-devel/2012-August/010412.html

Spice branches with seamless migration support can be found at:
spice-protocol: http://cgit.freedesktop.org/~yhalperi/spice-protocol/log/?h=seamless-migration.v1
spice-common: http://cgit.freedesktop.org/~yhalperi/spice-common/log/
spice: http://cgit.freedesktop.org/~yhalperi/spice/log/?h=seamless-migration.v1
spicei-gtk: http://cgit.freedesktop.org/~yhalperi/spice-gtk/log/

Regards,
Yonit.

Yonit Halperin (5):
  spice: notify spice server on vm start/stop
  spice: notify on vm state change only via spice_server_vm_start/stop
  spice migration: add QEVENT_SPICE_MIGRATE_COMPLETED
  spice: add 'migrated' flag to spice info
  spice: adding seamless-migration option to the command line

 hmp.c              |    2 +
 monitor.c          |    1 +
 monitor.h          |    1 +
 qapi-schema.json   |    5 ++-
 qemu-config.c      |    3 ++
 qemu-options.hx    |    3 ++
 ui/spice-core.c    |   78 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 ui/spice-display.c |    6 ++++
 ui/spice-display.h |    2 +
 9 files changed, 99 insertions(+), 2 deletions(-)

-- 
1.7.7.6

[Qemu-devel] [PATCH v2 1/5] spice: notify spice server on vm start/stop

[Yonit Halperin]

Spice server needs to know about the vm state in order to prevent
attempts to write to devices when they are stopped, mainly during
the non-live stage of migration.
Instead, spice will take care of restoring this writes, on the migration
target side, after migration completes.

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 ui/spice-core.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 4fc48f8..32de1f1 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -545,6 +545,18 @@ static int add_channel(const char *name, const char *value, void *opaque)
     return 0;
 }
 
+static void vm_change_state_handler(void *opaque, int running,
+                                    RunState state)
+{
+#if SPICE_SERVER_VERSION >= 0x000b02 /* 0.11.2 */
+    if (running) {
+        spice_server_vm_start(spice_server);
+    } else {
+        spice_server_vm_stop(spice_server);
+    }
+#endif
+}
+
 void qemu_spice_init(void)
 {
     QemuOpts *opts = QTAILQ_FIRST(&qemu_spice_opts.head);
@@ -718,6 +730,8 @@ void qemu_spice_init(void)
     qemu_spice_input_init();
     qemu_spice_audio_init();
 
+    qemu_add_vm_change_state_handler(vm_change_state_handler, &spice_server);
+
     g_free(x509_key_file);
     g_free(x509_cert_file);
     g_free(x509_cacert_file);
-- 
1.7.7.6

[Qemu-devel] [PATCH v2 2/5] spice: notify on vm state change only via spice_server_vm_start/stop

[Yonit Halperin]

QXLWorker->start/stop are deprecated since spice-server 0.11.2

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 ui/spice-core.c    |   44 ++++++++++++++++++++++++++++++++++++++++++++
 ui/spice-display.c |    6 ++++++
 ui/spice-display.h |    2 ++
 3 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 32de1f1..97b45f8 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -37,6 +37,7 @@
 #include "migration.h"
 #include "monitor.h"
 #include "hw/hw.h"
+#include "spice-display.h"
 
 /* core bits */
 
@@ -55,6 +56,16 @@ struct SpiceTimer {
 };
 static QTAILQ_HEAD(, SpiceTimer) timers = QTAILQ_HEAD_INITIALIZER(timers);
 
+typedef struct DisplayListItem DisplayListItem;
+
+struct DisplayListItem {
+    SimpleSpiceDisplay *display;
+    QLIST_ENTRY(DisplayListItem) link;
+};
+static QLIST_HEAD(, DisplayListItem) display_list =
+    QLIST_HEAD_INITIALIZER(display_list);
+
+
 static SpiceTimer *timer_add(SpiceTimerFunc func, void *opaque)
 {
     SpiceTimer *timer;
@@ -545,14 +556,36 @@ static int add_channel(const char *name, const char *value, void *opaque)
     return 0;
 }
 
+#if SPICE_SERVER_VERSION >= 0x000b02 /* 0.11.2 */
+static void display_start(void)
+{
+    DisplayListItem *item;
+
+    QLIST_FOREACH(item, &display_list, link) {
+        item->display->running = true;
+    }
+}
+
+static void display_stop(void)
+{
+    DisplayListItem *item;
+
+    QLIST_FOREACH(item, &display_list, link) {
+        item->display->running = false;
+    }
+}
+#endif
+
 static void vm_change_state_handler(void *opaque, int running,
                                     RunState state)
 {
 #if SPICE_SERVER_VERSION >= 0x000b02 /* 0.11.2 */
     if (running) {
+        display_start();
         spice_server_vm_start(spice_server);
     } else {
         spice_server_vm_stop(spice_server);
+        display_stop();
     }
 #endif
 }
@@ -754,6 +787,17 @@ int qemu_spice_add_interface(SpiceBaseInstance *sin)
         spice_server = spice_server_new();
         spice_server_init(spice_server, &core_interface);
     }
+
+    if (strcmp(sin->sif->type, SPICE_INTERFACE_QXL) == 0) {
+        QXLInstance *qxl;
+        DisplayListItem *item;
+
+        qxl = container_of(sin, QXLInstance, base);
+        item = g_malloc0(sizeof(*item));
+        item->display = container_of(qxl, SimpleSpiceDisplay, qxl);
+
+        QLIST_INSERT_HEAD(&display_list, item, link);
+    }
     return spice_server_add_interface(spice_server, sin);
 }
 
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 3e8f0b3..073a354 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -126,6 +126,7 @@ void qemu_spice_wakeup(SimpleSpiceDisplay *ssd)
     ssd->worker->wakeup(ssd->worker);
 }
 
+#if SPICE_SERVER_VERSION < 0x000b02 /* before 0.11.2 */
 void qemu_spice_start(SimpleSpiceDisplay *ssd)
 {
     trace_qemu_spice_start(ssd->qxl.id);
@@ -137,6 +138,7 @@ void qemu_spice_stop(SimpleSpiceDisplay *ssd)
     trace_qemu_spice_stop(ssd->qxl.id);
     ssd->worker->stop(ssd->worker);
 }
+#endif
 
 static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 {
@@ -272,6 +274,7 @@ void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd)
 void qemu_spice_vm_change_state_handler(void *opaque, int running,
                                         RunState state)
 {
+#if SPICE_SERVER_VERSION < 0x000b02 /* before 0.11.2 */
     SimpleSpiceDisplay *ssd = opaque;
 
     if (running) {
@@ -281,6 +284,7 @@ void qemu_spice_vm_change_state_handler(void *opaque, int running,
         qemu_spice_stop(ssd);
         ssd->running = false;
     }
+#endif
 }
 
 void qemu_spice_display_init_common(SimpleSpiceDisplay *ssd, DisplayState *ds)
@@ -518,7 +522,9 @@ void qemu_spice_display_init(DisplayState *ds)
     qemu_spice_add_interface(&sdpy.qxl.base);
     assert(sdpy.worker);
 
+#if SPICE_SERVER_VERSION < 0x000b02 /* before 0.11.2 */
     qemu_add_vm_change_state_handler(qemu_spice_vm_change_state_handler, &sdpy);
+#endif
     qemu_spice_create_host_memslot(&sdpy);
     qemu_spice_create_host_primary(&sdpy);
 }
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 12e50b6..cf7a8e7 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -129,5 +129,7 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
 void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
                                         uint32_t id, qxl_async_io async);
 void qemu_spice_wakeup(SimpleSpiceDisplay *ssd);
+#if SPICE_SERVER_VERSION < 0x000b02 /* before 0.11.2 */
 void qemu_spice_start(SimpleSpiceDisplay *ssd);
 void qemu_spice_stop(SimpleSpiceDisplay *ssd);
+#endif
-- 
1.7.7.6

[Qemu-devel] [PATCH v2 3/5] spice migration: add QEVENT_SPICE_MIGRATE_COMPLETED

[Yonit Halperin]

When migrating, libvirt queries the migration status, and upon migration
completions, it closes the migration src. On the other hand, when
migration is completed, spice transfers data from the src to destination
via the client. This data is required for keeping the spice session
after migration, without suffering from data loss and inconsistencies.
In order to allow this data transfer, we add QEVENT for signaling
libvirt that spice migration has completed, and libvirt needs to wait
for this event before quitting the src process.

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 monitor.c       |    1 +
 monitor.h       |    1 +
 ui/spice-core.c |    9 ++++++++-
 3 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index ce42466..584efe0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -455,6 +455,7 @@ static const char *monitor_event_names[] = {
     [QEVENT_SUSPEND_DISK] = "SUSPEND_DISK",
     [QEVENT_WAKEUP] = "WAKEUP",
     [QEVENT_BALLOON_CHANGE] = "BALLOON_CHANGE",
+    [QEVENT_SPICE_MIGRATE_COMPLETED] = "SPICE_MIGRATE_COMPLETED",
 };
 QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
 
diff --git a/monitor.h b/monitor.h
index 47d556b..5fc2983 100644
--- a/monitor.h
+++ b/monitor.h
@@ -43,6 +43,7 @@ typedef enum MonitorEvent {
     QEVENT_SUSPEND_DISK,
     QEVENT_WAKEUP,
     QEVENT_BALLOON_CHANGE,
+    QEVENT_SPICE_MIGRATE_COMPLETED,
 
     /* Add to 'monitor_event_names' array in monitor.c when
      * defining new events here */
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 97b45f8..ad7c866 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -295,6 +295,7 @@ typedef struct SpiceMigration {
 } SpiceMigration;
 
 static void migrate_connect_complete_cb(SpiceMigrateInstance *sin);
+static void migrate_end_complete_cb(SpiceMigrateInstance *sin);
 
 static const SpiceMigrateInterface migrate_interface = {
     .base.type = SPICE_INTERFACE_MIGRATION,
@@ -302,7 +303,7 @@ static const SpiceMigrateInterface migrate_interface = {
     .base.major_version = SPICE_INTERFACE_MIGRATION_MAJOR,
     .base.minor_version = SPICE_INTERFACE_MIGRATION_MINOR,
     .migrate_connect_complete = migrate_connect_complete_cb,
-    .migrate_end_complete = NULL,
+    .migrate_end_complete = migrate_end_complete_cb,
 };
 
 static SpiceMigration spice_migrate;
@@ -315,6 +316,11 @@ static void migrate_connect_complete_cb(SpiceMigrateInstance *sin)
     }
     sm->connect_complete.cb = NULL;
 }
+
+static void migrate_end_complete_cb(SpiceMigrateInstance *sin)
+{
+    monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
+}
 #endif
 
 /* config string parsing */
@@ -498,6 +504,7 @@ static void migration_state_notifier(Notifier *notifier, void *data)
     } else if (migration_has_finished(s)) {
 #ifndef SPICE_INTERFACE_MIGRATION
         spice_server_migrate_switch(spice_server);
+        monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
 #else
         spice_server_migrate_end(spice_server, true);
     } else if (migration_has_failed(s)) {
-- 
1.7.7.6

[Qemu-devel] [PATCH v2 4/5] spice: add 'migrated' flag to spice info

[Yonit Halperin]

The flag is 'true' when spice migration has completed on the src side.
It is needed for a case where libvirt dies before migration completes
and it misses the event QEVENT_SPICE_MIGRATE_COMPLETED.
When libvirt is restored and queries the migration status, it also needs
to query spice and check if its migration has completed.

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 hmp.c            |    2 ++
 qapi-schema.json |    5 ++++-
 ui/spice-core.c  |    4 ++++
 3 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/hmp.c b/hmp.c
index a9d5675..62f4dee 100644
--- a/hmp.c
+++ b/hmp.c
@@ -413,6 +413,8 @@ void hmp_info_spice(Monitor *mon)
         monitor_printf(mon, "     address: %s:%" PRId64 " [tls]\n",
                        info->host, info->tls_port);
     }
+    monitor_printf(mon, "    migrated: %s\n",
+                   info->migrated ? "true" : "false");
     monitor_printf(mon, "        auth: %s\n", info->auth);
     monitor_printf(mon, "    compiled: %s\n", info->compiled_version);
     monitor_printf(mon, "  mouse-mode: %s\n",
diff --git a/qapi-schema.json b/qapi-schema.json
index 3d2b2d1..bf3f924 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -808,6 +808,9 @@
 #
 # @enabled: true if the SPICE server is enabled, false otherwise
 #
+# @migrated: true if the last guest migration completed and spice
+#            migration had completed as well. false otherwise.
+#
 # @host: #optional The hostname the SPICE server is bound to.  This depends on
 #        the name resolution on the host and may be an IP address.
 #
@@ -833,7 +836,7 @@
 # Since: 0.14.0
 ##
 { 'type': 'SpiceInfo',
-  'data': {'enabled': 'bool', '*host': 'str', '*port': 'int',
+  'data': {'enabled': 'bool', 'migrated': 'bool', '*host': 'str', '*port': 'int',
            '*tls-port': 'int', '*auth': 'str', '*compiled-version': 'str',
            'mouse-mode': 'SpiceQueryMouseMode', '*channels': ['SpiceChannel']} }
 
diff --git a/ui/spice-core.c b/ui/spice-core.c
index ad7c866..ad7f01a 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -46,6 +46,7 @@ static Notifier migration_state;
 static const char *auth = "spice";
 static char *auth_passwd;
 static time_t auth_expires = TIME_MAX;
+static int spice_migration_completed;
 int using_spice = 0;
 
 static QemuThread me;
@@ -320,6 +321,7 @@ static void migrate_connect_complete_cb(SpiceMigrateInstance *sin)
 static void migrate_end_complete_cb(SpiceMigrateInstance *sin)
 {
     monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
+    spice_migration_completed = true;
 }
 #endif
 
@@ -452,6 +454,7 @@ SpiceInfo *qmp_query_spice(Error **errp)
     }
 
     info->enabled = true;
+    info->migrated = spice_migration_completed;
 
     addr = qemu_opt_get(opts, "addr");
     port = qemu_opt_get_number(opts, "port", 0);
@@ -505,6 +508,7 @@ static void migration_state_notifier(Notifier *notifier, void *data)
 #ifndef SPICE_INTERFACE_MIGRATION
         spice_server_migrate_switch(spice_server);
         monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
+        spice_migration_completed = true;
 #else
         spice_server_migrate_end(spice_server, true);
     } else if (migration_has_failed(s)) {
-- 
1.7.7.6

[Qemu-devel] [PATCH v2 5/5] spice: adding seamless-migration option to the command line

[Yonit Halperin]

The seamless-migration flag is required in order to identify
whether libvirt supports the new QEVENT_SPICE_MIGRATE_COMPLETED or not
(by default the flag is off).
New libvirt versions that wait for QEVENT_SPICE_MIGRATE_COMPLETED should turn on this flag.
When this flag is off, spice fallbacks to its old migration method, which
can result in data loss.

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 qemu-config.c   |    3 +++
 qemu-options.hx |    3 +++
 ui/spice-core.c |    7 +++++++
 3 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 5c3296b..b7bb28f 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -524,6 +524,9 @@ QemuOptsList qemu_spice_opts = {
         },{
             .name = "playback-compression",
             .type = QEMU_OPT_BOOL,
+        }, {
+            .name = "seamless-migration",
+            .type = QEMU_OPT_BOOL,
         },
         { /* end of list */ }
     },
diff --git a/qemu-options.hx b/qemu-options.hx
index 47cb5bd..0727f4f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -917,6 +917,9 @@ Enable/disable passing mouse events via vdagent.  Default is on.
 @item playback-compression=[on|off]
 Enable/disable audio stream compression (using celt 0.5.1).  Default is on.
 
+@item seamless-migration=[on|off]
+Enable/disable spice seamless migration. Default is off.
+
 @end table
 ETEXI
 
diff --git a/ui/spice-core.c b/ui/spice-core.c
index ad7f01a..08a6da9 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -614,6 +614,9 @@ void qemu_spice_init(void)
     int port, tls_port, len, addr_flags;
     spice_image_compression_t compression;
     spice_wan_compression_t wan_compr;
+#if SPICE_SERVER_VERSION >= 0x000b02 /* 0.11.2 */
+    bool seamless_migration;
+#endif
 
     qemu_thread_get_self(&me);
 
@@ -757,6 +760,10 @@ void qemu_spice_init(void)
     spice_server_set_uuid(spice_server, qemu_uuid);
 #endif
 
+#if SPICE_SERVER_VERSION >= 0x000b02 /* 0.11.2 */
+    seamless_migration = qemu_opt_get_bool(opts, "seamless-migration", 0);
+    spice_server_set_seamless_migration(spice_server, seamless_migration);
+#endif
     if (0 != spice_server_init(spice_server, &core_interface)) {
         error_report("failed to initialize spice server");
         exit(1);
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH v2 2/5] spice: notify on vm state change only via spice_server_vm_start/stop

[Gerd Hoffmann]

  Hi,

> +#if SPICE_SERVER_VERSION >= 0x000b02 /* 0.11.2 */
> +static void display_start(void)
> +{
> +    DisplayListItem *item;
> +
> +    QLIST_FOREACH(item, &display_list, link) {
> +        item->display->running = true;
> +    }
> +}

I think we should simply use a global variable for 'running' here.  It's
global state anyway.  Having this in per-display state struct buys us
nothing and adds alot of code for updating and display list maintainance.

>  void qemu_spice_vm_change_state_handler(void *opaque, int running,
>                                          RunState state)
>  {
> +#if SPICE_SERVER_VERSION < 0x000b02 /* before 0.11.2 */
>      SimpleSpiceDisplay *ssd = opaque;

> +#if SPICE_SERVER_VERSION < 0x000b02 /* before 0.11.2 */
>      qemu_add_vm_change_state_handler(qemu_spice_vm_change_state_handler, &sdpy);
> +#endif

If you don't register qemu_spice_vm_change_state_handler on new
spice-server you should #ifdef the whole function, not the body only.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2 2/5] spice: notify on vm state change only via spice_server_vm_start/stop

[Yonit Halperin]

Hi,
On 08/21/2012 09:58 AM, Gerd Hoffmann wrote:
>    Hi,
>
>> +#if SPICE_SERVER_VERSION>= 0x000b02 /* 0.11.2 */
>> +static void display_start(void)
>> +{
>> +    DisplayListItem *item;
>> +
>> +    QLIST_FOREACH(item,&display_list, link) {
>> +        item->display->running = true;
>> +    }
>> +}
>
> I think we should simply use a global variable for 'running' here.  It's
> global state anyway.  Having this in per-display state struct buys us
> nothing and adds alot of code for updating and display list maintainance.
>
ok, good point.
>>   void qemu_spice_vm_change_state_handler(void *opaque, int running,
>>                                           RunState state)
>>   {
>> +#if SPICE_SERVER_VERSION<  0x000b02 /* before 0.11.2 */
>>       SimpleSpiceDisplay *ssd = opaque;
>
>> +#if SPICE_SERVER_VERSION<  0x000b02 /* before 0.11.2 */
>>       qemu_add_vm_change_state_handler(qemu_spice_vm_change_state_handler,&sdpy);
>> +#endif
>
> If you don't register qemu_spice_vm_change_state_handler on new
> spice-server you should #ifdef the whole function, not the body only.
>
qemu_spice_vm_change_state_handler is also called from qxl.c, as part of 
its own vm_change_state handler. I didn't want to add another ifdef there.

> cheers,
>    Gerd

Re: [Qemu-devel] [PATCH v2 2/5] spice: notify on vm state change only via spice_server_vm_start/stop

[Gerd Hoffmann]

On 08/21/12 09:09, Yonit Halperin wrote:
> Hi,
> On 08/21/2012 09:58 AM, Gerd Hoffmann wrote:
>>    Hi,
>>
>>> +#if SPICE_SERVER_VERSION>= 0x000b02 /* 0.11.2 */
>>> +static void display_start(void)
>>> +{
>>> +    DisplayListItem *item;
>>> +
>>> +    QLIST_FOREACH(item,&display_list, link) {
>>> +        item->display->running = true;
>>> +    }
>>> +}
>>
>> I think we should simply use a global variable for 'running' here.  It's
>> global state anyway.  Having this in per-display state struct buys us
>> nothing and adds alot of code for updating and display list maintainance.
>>
> ok, good point.
>>>   void qemu_spice_vm_change_state_handler(void *opaque, int running,
>>>                                           RunState state)
>>>   {
>>> +#if SPICE_SERVER_VERSION<  0x000b02 /* before 0.11.2 */
>>>       SimpleSpiceDisplay *ssd = opaque;
>>
>>> +#if SPICE_SERVER_VERSION<  0x000b02 /* before 0.11.2 */
>>>      
>>> qemu_add_vm_change_state_handler(qemu_spice_vm_change_state_handler,&sdpy);
>>>
>>> +#endif
>>
>> If you don't register qemu_spice_vm_change_state_handler on new
>> spice-server you should #ifdef the whole function, not the body only.
>>
> qemu_spice_vm_change_state_handler is also called from qxl.c, as part of
> its own vm_change_state handler. I didn't want to add another ifdef there.

Fair point, but then you can drop the #ifdef for the
qemu_add_vm_change_state_handler call too.

cheers,
  Gerd