[Qemu-devel] [PATCH 0/2] spice migration interface (RHBZ 737921)

[Qemu-devel] [PATCH 0/2] spice migration interface (RHBZ 737921)

[Yonit Halperin]

Spice client is required to connect to the migration target before/as migration
starts. Previously, it connected upon migration completion, however, the ticket
was set in the beginning, thus when migration time was >
ticket_expiration_time, spice failed to connect to the target.

Since the migration target is blocked after migration starts, we execute spice-client connection to the target before migration, upon client_migrate_info. We wait till the client is connected to the target, or till a timeout occurs.
In order to not block the iothread, this patch turns client_migrate_info to
asynchronous.

In addition, we changed the spice api:
(1) client_migrate_info need to call spice_server_migrate_connect
(2) spice_server_migrate_start/end need to be called upon migration start/end

** spice_server_start and the migrate_end_complete callback, were added for
   future use, in case we implement a real seamless spice migration

Spice server patches will be emailed next. They are for the 0.8 spice branch,
but will be ported to upstream soon.

Yonit Halperin (2):
  spice: turn client_migrate_info to async
  spice: support the new migration interface (spice 0.8.3)

 hmp-commands.hx |    3 +-
 monitor.c       |    6 +++-
 qmp-commands.hx |    3 +-
 ui/qemu-spice.h |   13 +++++++++-
 ui/spice-core.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 5 files changed, 81 insertions(+), 10 deletions(-)

-- 
1.7.4.4

[Qemu-devel] [PATCH 1/2] spice: turn client_migrate_info to async

[Yonit Halperin]

RHBZ 737921
Spice client is required to connect to the migration target before/as migration
starts. Since after migration starts, the target qemu is blocked and cannot accept new spice client
we trigger the connection to the target upon client_migrate_info command.
client_migrate_info completion cb will be called after spice client has been
connected to the target (or a timeout). See following patches and spice patches.

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 hmp-commands.hx |    3 ++-
 monitor.c       |    6 ++++--
 qmp-commands.hx |    3 ++-
 ui/qemu-spice.h |   13 +++++++++++--
 ui/spice-core.c |   10 +++++++---
 5 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9e1cca8..6f390a0 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -827,7 +827,8 @@ ETEXI
         .params     = "protocol hostname port tls-port cert-subject",
         .help       = "send migration info to spice/vnc client",
         .user_print = monitor_user_noop,
-        .mhandler.cmd_new = client_migrate_info,
+        .mhandler.cmd_async = client_migrate_info,
+        .flags      = MONITOR_CMD_ASYNC,
     },
 
 STEXI
diff --git a/monitor.c b/monitor.c
index df0f622..0374dcc 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1221,7 +1221,8 @@ static int add_graphics_client(Monitor *mon, const QDict *qdict, QObject **ret_d
     return -1;
 }
 
-static int client_migrate_info(Monitor *mon, const QDict *qdict, QObject **ret_data)
+static int client_migrate_info(Monitor *mon, const QDict *qdict,
+                               MonitorCompletion cb, void *opaque)
 {
     const char *protocol = qdict_get_str(qdict, "protocol");
     const char *hostname = qdict_get_str(qdict, "hostname");
@@ -1236,7 +1237,8 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict, QObject **ret_d
             return -1;
         }
 
-        ret = qemu_spice_migrate_info(hostname, port, tls_port, subject);
+        ret = qemu_spice_migrate_info(hostname, port, tls_port, subject,
+                                      cb, opaque);
         if (ret != 0) {
             qerror_report(QERR_UNDEFINED_ERROR);
             return -1;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 27cc66e..321fb10 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -578,7 +578,8 @@ EQMP
         .params     = "protocol hostname port tls-port cert-subject",
         .help       = "send migration info to spice/vnc client",
         .user_print = monitor_user_noop,
-        .mhandler.cmd_new = client_migrate_info,
+        .mhandler.cmd_async = client_migrate_info,
+        .flags      = MONITOR_CMD_ASYNC,
     },
 
 SQMP
diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
index f34be69..0edc47b 100644
--- a/ui/qemu-spice.h
+++ b/ui/qemu-spice.h
@@ -25,6 +25,7 @@
 #include "qemu-option.h"
 #include "qemu-config.h"
 #include "qemu-char.h"
+#include "monitor.h"
 
 extern int using_spice;
 
@@ -37,7 +38,8 @@ int qemu_spice_set_passwd(const char *passwd,
                           bool fail_if_connected, bool disconnect_if_connected);
 int qemu_spice_set_pw_expire(time_t expires);
 int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
-                            const char *subject);
+                            const char *subject,
+                            MonitorCompletion cb, void *opaque);
 
 void do_info_spice_print(Monitor *mon, const QObject *data);
 void do_info_spice(Monitor *mon, QObject **ret_data);
@@ -57,7 +59,14 @@ static inline int qemu_spice_set_pw_expire(time_t expires)
 {
     return -1;
 }
-static inline int qemu_spice_migrate_info(const char *h, int p, int t, const char *s)
+static inline int qemu_spice_migrate_info(const char *h, int p, int t,
+                                          const char *s,
+                                          MonitorCompletion cb, void *opaque)
+{
+    cb(opaque, NULL);
+    return -1;
+}
+
 { return -1; }
 
 #endif /* CONFIG_SPICE */
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 3cbc721..50c0d7d 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -457,10 +457,14 @@ static void migration_state_notifier(Notifier *notifier, void *data)
 }
 
 int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
-                            const char *subject)
+                            const char *subject,
+                            MonitorCompletion *cb, void *opaque)
 {
-    return spice_server_migrate_info(spice_server, hostname,
-                                     port, tls_port, subject);
+    int ret;
+    ret = spice_server_migrate_info(spice_server, hostname,
+                                    port, tls_port, subject);
+    cb(opaque, NULL);
+    return ret;
 }
 
 static int add_channel(const char *name, const char *value, void *opaque)
-- 
1.7.4.4

[Qemu-devel] [PATCH 2/2] spice: support the new migration interface (spice 0.8.3)

[Yonit Halperin]

- call spice_server_migrate_(start|end|connect).
- register spice_migrate_connect completion callback

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

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 50c0d7d..457cf61 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -288,6 +288,38 @@ static SpiceCoreInterface core_interface = {
 #endif
 };
 
+#ifdef SPICE_INTERFACE_MIGRATION
+typedef struct SpiceMigration {
+    SpiceMigrateInstance sin;
+    struct {
+        MonitorCompletion *cb;
+        void *opaque;
+    } connect_complete;
+} SpiceMigration;
+
+static void migrate_connect_complete_cb(SpiceMigrateInstance *sin);
+
+static const SpiceMigrateInterface migrate_interface = {
+    .base.type = SPICE_INTERFACE_MIGRATION,
+    .base.description = "migration",
+    .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,
+};
+
+static SpiceMigration spice_migrate;
+
+static void migrate_connect_complete_cb(SpiceMigrateInstance *sin)
+{
+    SpiceMigration *sm = container_of(sin, SpiceMigration, sin);
+    if (sm->connect_complete.cb) {
+        sm->connect_complete.cb(sm->connect_complete.opaque, NULL);
+    }
+    sm->connect_complete.cb = NULL;
+}
+#endif
+
 /* config string parsing */
 
 static int name2enum(const char *string, const char *table[], int entries)
@@ -449,9 +481,19 @@ static void migration_state_notifier(Notifier *notifier, void *data)
 {
     int state = get_migration_state();
 
-    if (state == MIG_STATE_COMPLETED) {
+    if (state == MIG_STATE_ACTIVE) {
+#ifdef SPICE_INTERFACE_MIGRATION
+        spice_server_migrate_start(spice_server);
+#endif
+    } else if (state == MIG_STATE_COMPLETED) {
 #if SPICE_SERVER_VERSION >= 0x000701 /* 0.7.1 */
+#ifndef SPICE_INTERFACE_MIGRATION
         spice_server_migrate_switch(spice_server);
+#else
+        spice_server_migrate_end(spice_server, true);
+    } else if (state == MIG_STATE_CANCELLED || state == MIG_STATE_ERROR) {
+        spice_server_migrate_end(spice_server, false);
+#endif
 #endif
     }
 }
@@ -461,9 +503,16 @@ int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
                             MonitorCompletion *cb, void *opaque)
 {
     int ret;
+#ifdef SPICE_INTERFACE_MIGRATION
+    spice_migrate.connect_complete.cb = cb;
+    spice_migrate.connect_complete.opaque = opaque;
+    ret = spice_server_migrate_connect(spice_server, hostname,
+                                       port, tls_port, subject);
+#else
     ret = spice_server_migrate_info(spice_server, hostname,
                                     port, tls_port, subject);
     cb(opaque, NULL);
+#endif
     return ret;
 }
 
@@ -654,6 +703,11 @@ void qemu_spice_init(void)
 
     migration_state.notify = migration_state_notifier;
     add_migration_state_change_notifier(&migration_state);
+#ifdef SPICE_INTERFACE_MIGRATION
+    spice_migrate.sin.base.sif = &migrate_interface.base;
+    spice_migrate.connect_complete.cb = NULL;
+    qemu_spice_add_interface(&spice_migrate.sin.base);
+#endif
 
     qemu_spice_input_init();
     qemu_spice_audio_init();
-- 
1.7.4.4

Re: [Qemu-devel] [PATCH 2/2] spice: support the new migration interface (spice 0.8.3)

[Alon Levy]

On Wed, Sep 21, 2011 at 06:50:44PM +0300, Yonit Halperin wrote:
> - call spice_server_migrate_(start|end|connect).
> - register spice_migrate_connect completion callback
> 

Reviewed-by: Alon Levy <alevy@redhat.com>

Will just fix the one place below in the pull request.

> Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
> ---
>  ui/spice-core.c |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 55 insertions(+), 1 deletions(-)
> 
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 50c0d7d..457cf61 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -288,6 +288,38 @@ static SpiceCoreInterface core_interface = {
>  #endif
>  };
>  
> +#ifdef SPICE_INTERFACE_MIGRATION
> +typedef struct SpiceMigration {
> +    SpiceMigrateInstance sin;
> +    struct {
> +        MonitorCompletion *cb;
> +        void *opaque;
> +    } connect_complete;
> +} SpiceMigration;
> +
> +static void migrate_connect_complete_cb(SpiceMigrateInstance *sin);
> +
> +static const SpiceMigrateInterface migrate_interface = {
> +    .base.type = SPICE_INTERFACE_MIGRATION,
> +    .base.description = "migration",
> +    .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,
> +};
> +
> +static SpiceMigration spice_migrate;
> +
> +static void migrate_connect_complete_cb(SpiceMigrateInstance *sin)
> +{
> +    SpiceMigration *sm = container_of(sin, SpiceMigration, sin);

Here.

> +    if (sm->connect_complete.cb) {
> +        sm->connect_complete.cb(sm->connect_complete.opaque, NULL);
> +    }
> +    sm->connect_complete.cb = NULL;
> +}
> +#endif
> +
>  /* config string parsing */
>  
>  static int name2enum(const char *string, const char *table[], int entries)
> @@ -449,9 +481,19 @@ static void migration_state_notifier(Notifier *notifier, void *data)
>  {
>      int state = get_migration_state();
>  
> -    if (state == MIG_STATE_COMPLETED) {
> +    if (state == MIG_STATE_ACTIVE) {
> +#ifdef SPICE_INTERFACE_MIGRATION
> +        spice_server_migrate_start(spice_server);
> +#endif

This is slightly ugly in that if the ifdef is false it is an empty
block, but it isn't critical (and I'm not going to redo my testing).

> +    } else if (state == MIG_STATE_COMPLETED) {
>  #if SPICE_SERVER_VERSION >= 0x000701 /* 0.7.1 */
> +#ifndef SPICE_INTERFACE_MIGRATION
>          spice_server_migrate_switch(spice_server);
> +#else
> +        spice_server_migrate_end(spice_server, true);
> +    } else if (state == MIG_STATE_CANCELLED || state == MIG_STATE_ERROR) {
> +        spice_server_migrate_end(spice_server, false);
> +#endif
>  #endif
>      }
>  }
> @@ -461,9 +503,16 @@ int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
>                              MonitorCompletion *cb, void *opaque)
>  {
>      int ret;
> +#ifdef SPICE_INTERFACE_MIGRATION
> +    spice_migrate.connect_complete.cb = cb;
> +    spice_migrate.connect_complete.opaque = opaque;
> +    ret = spice_server_migrate_connect(spice_server, hostname,
> +                                       port, tls_port, subject);
> +#else
>      ret = spice_server_migrate_info(spice_server, hostname,
>                                      port, tls_port, subject);
>      cb(opaque, NULL);
> +#endif
>      return ret;
>  }
>  
> @@ -654,6 +703,11 @@ void qemu_spice_init(void)
>  
>      migration_state.notify = migration_state_notifier;
>      add_migration_state_change_notifier(&migration_state);
> +#ifdef SPICE_INTERFACE_MIGRATION
> +    spice_migrate.sin.base.sif = &migrate_interface.base;
> +    spice_migrate.connect_complete.cb = NULL;
> +    qemu_spice_add_interface(&spice_migrate.sin.base);
> +#endif
>  
>      qemu_spice_input_init();
>      qemu_spice_audio_init();
> -- 
> 1.7.4.4
> 

Re: [Qemu-devel] [PATCH 0/2] spice migration interface (RHBZ 737921)

[Gerd Hoffmann]

   Hi,

> Yonit Halperin (2):
>    spice: turn client_migrate_info to async
>    spice: support the new migration interface (spice 0.8.3)

Added to spice patch queue.

thanks,
   Gerd

Re: [Qemu-devel] [PATCH 1/2] spice: turn client_migrate_info to async

[Gerd Hoffmann]

On 09/21/11 17:50, Yonit Halperin wrote:
> RHBZ 737921
> Spice client is required to connect to the migration target before/as migration
> starts. Since after migration starts, the target qemu is blocked and cannot accept new spice client
> we trigger the connection to the target upon client_migrate_info command.
> client_migrate_info completion cb will be called after spice client has been
> connected to the target (or a timeout). See following patches and spice patches.

Patch failes to build with spice disabled.
Please fix.

thanks,
   Gerd