[Qemu-devel] [PATCH 4/4] qapi: Convert migrate

[Luiz Capitulino <lcapitulino@redhat.com>]

The migrate command is one of those commands where HMP and QMP completely
mix up together. This made the conversion to the QAPI (which separates the
command into QMP and HMP parts) a bit difficult.

The first important change to be noticed is that this commit completes the
removal of the Monitor object from migration code, started by the previous
commit.

Another important and tricky change is about supporting the non-detached
mode. That's, if the user doesn't pass '-d' the migrate command will lock
the monitor and will only release it when migration is finished.

To support that in the new HMP command (hmp_migrate()), it is necessary
to create a timer which runs every second and checks if the migration is
still active. If it's, the timer callback will re-schedule itself to run
one second in the future. If the migration has already finished, the
monitor lock is relased and the user can use it normally.

All these changes should be transparent to the user.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hmp-commands.hx  |    3 +--
 hmp.c            |   56 +++++++++++++++++++++++++++++++++++++++++++++
 hmp.h            |    1 +
 migration-fd.c   |    2 +-
 migration.c      |   66 ++++++++++++++----------------------------------------
 migration.h      |    3 ---
 qapi-schema.json |   21 +++++++++++++++++
 qmp-commands.hx  |    9 +-------
 savevm.c         |   13 +++++------
 sysemu.h         |    2 +-
 10 files changed, 105 insertions(+), 71 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index ed88877..a86f53f 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -806,8 +806,7 @@ ETEXI
 		      " full copy of disk\n\t\t\t -i for migration without "
 		      "shared storage with incremental copy of disk "
 		      "(base image shared between src and destination)",
-        .user_print = monitor_user_noop,	
-	.mhandler.cmd_new = do_migrate,
+        .mhandler.cmd = hmp_migrate,
     },
 
 
diff --git a/hmp.c b/hmp.c
index 3a54455..0f62b3f 100644
--- a/hmp.c
+++ b/hmp.c
@@ -14,6 +14,7 @@
  */
 
 #include "hmp.h"
+#include "qemu-timer.h"
 #include "qmp-commands.h"
 
 static void hmp_handle_error(Monitor *mon, Error **errp)
@@ -856,3 +857,58 @@ void hmp_block_job_cancel(Monitor *mon, const QDict *qdict)
 
     hmp_handle_error(mon, &error);
 }
+
+typedef struct MigrationStatus
+{
+    QEMUTimer *timer;
+    Monitor *mon;
+} MigrationStatus;
+
+static void hmp_migrate_status_cb(void *opaque)
+{
+    MigrationStatus *status = opaque;
+    MigrationInfo *info;
+
+    info = qmp_query_migrate(NULL);
+    if (!info->has_status || strcmp(info->status, "active") == 0) {
+        qemu_mod_timer(status->timer, qemu_get_clock_ms(rt_clock) + 1000);
+    } else {
+        monitor_resume(status->mon);
+        qemu_del_timer(status->timer);
+        g_free(status);
+    }
+
+    qapi_free_MigrationInfo(info);
+}
+
+void hmp_migrate(Monitor *mon, const QDict *qdict)
+{
+    int detach = qdict_get_try_bool(qdict, "detach", 0);
+    int blk = qdict_get_try_bool(qdict, "blk", 0);
+    int inc = qdict_get_try_bool(qdict, "inc", 0);
+    const char *uri = qdict_get_str(qdict, "uri");
+    Error *err = NULL;
+
+    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err);
+    if (err) {
+        monitor_printf(mon, "migrate: %s\n", error_get_pretty(err));
+        error_free(err);
+        return;
+    }
+
+    if (!detach) {
+        MigrationStatus *status;
+
+        if (monitor_suspend(mon) < 0) {
+            monitor_printf(mon, "terminal does not allow synchronous "
+                           "migration, continuing detached\n");
+            return;
+        }
+
+        status = g_malloc0(sizeof(*status));
+        status->mon = mon;
+        status->timer = qemu_new_timer_ms(rt_clock, hmp_migrate_status_cb,
+                                          status);
+        qemu_mod_timer(status->timer, qemu_get_clock_ms(rt_clock));
+    }
+}
diff --git a/hmp.h b/hmp.h
index 5409464..8807853 100644
--- a/hmp.h
+++ b/hmp.h
@@ -59,5 +59,6 @@ void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict);
 void hmp_block_stream(Monitor *mon, const QDict *qdict);
 void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict);
 void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
+void hmp_migrate(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/migration-fd.c b/migration-fd.c
index 5a068c6..50138ed 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -75,7 +75,7 @@ static int fd_close(MigrationState *s)
 
 int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
 {
-    s->fd = monitor_get_fd(s->mon, fdname);
+    s->fd = monitor_get_fd(cur_mon, fdname);
     if (s->fd == -1) {
         DPRINTF("fd_migration: invalid file descriptor identifier\n");
         goto err_after_get_fd;
diff --git a/migration.c b/migration.c
index b21b2df..8c119ba 100644
--- a/migration.c
+++ b/migration.c
@@ -158,16 +158,6 @@ MigrationInfo *qmp_query_migrate(Error **errp)
 
 /* shared migration helpers */
 
-static void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
-{
-    if (monitor_suspend(mon) == 0) {
-        DPRINTF("suspending monitor\n");
-    } else {
-        monitor_printf(mon, "terminal does not allow synchronous "
-                       "migration, continuing detached\n");
-    }
-}
-
 static int migrate_fd_cleanup(MigrationState *s)
 {
     int ret = 0;
@@ -178,10 +168,6 @@ static int migrate_fd_cleanup(MigrationState *s)
         DPRINTF("closing file\n");
         ret = qemu_fclose(s->file);
         s->file = NULL;
-    } else {
-        if (s->mon) {
-            monitor_resume(s->mon);
-        }
     }
 
     if (s->fd != -1) {
@@ -321,9 +307,6 @@ static int migrate_fd_close(void *opaque)
 {
     MigrationState *s = opaque;
 
-    if (s->mon) {
-        monitor_resume(s->mon);
-    }
     qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
     return s->close(s);
 }
@@ -376,7 +359,7 @@ void migrate_fd_connect(MigrationState *s)
     migrate_fd_put_ready(s);
 }
 
-static MigrationState *migrate_init(Monitor *mon, int detach, int blk, int inc)
+static MigrationState *migrate_init(int blk, int inc)
 {
     MigrationState *s = migrate_get_current();
     int64_t bandwidth_limit = s->bandwidth_limit;
@@ -386,18 +369,9 @@ static MigrationState *migrate_init(Monitor *mon, int detach, int blk, int inc)
     s->blk = blk;
     s->shared = inc;
 
-    /* s->mon is used for two things:
-       - pass fd in fd migration
-       - suspend/resume monitor for not detached migration
-    */
-    s->mon = mon;
     s->bandwidth_limit = bandwidth_limit;
     s->state = MIG_STATE_SETUP;
 
-    if (!detach) {
-        migrate_fd_monitor_suspend(s, mon);
-    }
-
     return s;
 }
 
@@ -413,32 +387,29 @@ void migrate_del_blocker(Error *reason)
     migration_blockers = g_slist_remove(migration_blockers, reason);
 }
 
-int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
+void qmp_migrate(const char *uri, bool has_blk, bool blk,
+                 bool has_inc, bool inc, bool has_detach, bool detach,
+                 Error **errp)
 {
     MigrationState *s = migrate_get_current();
     const char *p;
-    int detach = qdict_get_try_bool(qdict, "detach", 0);
-    int blk = qdict_get_try_bool(qdict, "blk", 0);
-    int inc = qdict_get_try_bool(qdict, "inc", 0);
-    const char *uri = qdict_get_str(qdict, "uri");
     int ret;
 
     if (s->state == MIG_STATE_ACTIVE) {
-        monitor_printf(mon, "migration already in progress\n");
-        return -1;
+        error_set(errp, QERR_MIGRATION_ACTIVE);
+        return;
     }
 
-    if (qemu_savevm_state_blocked(mon)) {
-        return -1;
+    if (qemu_savevm_state_blocked(errp)) {
+        return;
     }
 
     if (migration_blockers) {
-        Error *err = migration_blockers->data;
-        qerror_report_err(err);
-        return -1;
+        *errp = error_copy(migration_blockers->data);
+        return;
     }
 
-    s = migrate_init(mon, detach, blk, inc);
+    s = migrate_init(blk, inc);
 
     if (strstart(uri, "tcp:", &p)) {
         ret = tcp_start_outgoing_migration(s, p);
@@ -451,21 +422,18 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
         ret = fd_start_outgoing_migration(s, p);
 #endif
     } else {
-        monitor_printf(mon, "unknown migration protocol: %s\n", uri);
-        ret  = -EINVAL;
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "uri", "a valid migration protocol");
+        return;
     }
 
     if (ret < 0) {
-        monitor_printf(mon, "migration failed: %s\n", strerror(-ret));
-        return ret;
-    }
-
-    if (detach) {
-        s->mon = NULL;
+        DPRINTF("migration failed: %s\n", strerror(-ret));
+        /* FIXME: we should return meaningful errors */
+        error_set(errp, QERR_UNDEFINED_ERROR);
+        return;
     }
 
     notifier_list_notify(&migration_state_notifiers, s);
-    return 0;
 }
 
 void qmp_migrate_cancel(Error **errp)
diff --git a/migration.h b/migration.h
index 0e44197..691b367 100644
--- a/migration.h
+++ b/migration.h
@@ -26,7 +26,6 @@ struct MigrationState
     int64_t bandwidth_limit;
     QEMUFile *file;
     int fd;
-    Monitor *mon;
     int state;
     int (*get_error)(MigrationState *s);
     int (*close)(MigrationState *s);
@@ -40,8 +39,6 @@ void process_incoming_migration(QEMUFile *f);
 
 int qemu_start_incoming_migration(const char *uri);
 
-int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data);
-
 uint64_t migrate_max_downtime(void);
 
 void do_info_migrate_print(Monitor *mon, const QObject *data);
diff --git a/qapi-schema.json b/qapi-schema.json
index 5f293c4..c0780ab 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1631,3 +1631,24 @@
 { 'command': 'qom-list-types',
   'data': { '*implements': 'str', '*abstract': 'bool' },
   'returns': [ 'ObjectTypeInfo' ] }
+
+##
+# @migrate
+#
+# Migrates the current running guest to another Virtual Machine.
+#
+# @uri: the Uniform Resource Identifier of the destination VM
+#
+# @blk: #optional do block migration (full disk copy)
+#
+# @inc: #optional incremental disk copy migration
+#
+# @detach: this argument exists only for compatibility reasons and should not
+#          be used.
+#
+# Returns: nothing on success
+#
+# Since: 0.14.0
+##
+{ 'command': 'migrate',
+  'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 0c9bfac..f328bc1 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -446,14 +446,7 @@ EQMP
     {
         .name       = "migrate",
         .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
-        .params     = "[-d] [-b] [-i] uri",
-        .help       = "migrate to URI (using -d to not wait for completion)"
-		      "\n\t\t\t -b for migration without shared storage with"
-		      " full copy of disk\n\t\t\t -i for migration without "
-		      "shared storage with incremental copy of disk "
-		      "(base image shared between src and destination)",
-        .user_print = monitor_user_noop,	
-	.mhandler.cmd_new = do_migrate,
+        .mhandler.cmd_new = qmp_marshal_input_migrate,
     },
 
 SQMP
diff --git a/savevm.c b/savevm.c
index 70f5c4f..5fdc3e1 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1540,14 +1540,13 @@ static void vmstate_save(QEMUFile *f, SaveStateEntry *se)
 #define QEMU_VM_SECTION_FULL         0x04
 #define QEMU_VM_SUBSECTION           0x05
 
-bool qemu_savevm_state_blocked(Monitor *mon)
+bool qemu_savevm_state_blocked(Error **errp)
 {
     SaveStateEntry *se;
 
     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
         if (se->no_migrate) {
-            monitor_printf(mon, "state blocked by non-migratable device '%s'\n",
-                           se->idstr);
+            error_set(errp, QERR_MIGRATION_NOT_SUPPORTED, se->idstr);
             return true;
         }
     }
@@ -1698,11 +1697,11 @@ void qemu_savevm_state_cancel(QEMUFile *f)
     }
 }
 
-static int qemu_savevm_state(Monitor *mon, QEMUFile *f)
+static int qemu_savevm_state(QEMUFile *f)
 {
     int ret;
 
-    if (qemu_savevm_state_blocked(mon)) {
+    if (qemu_savevm_state_blocked(NULL)) {
         ret = -EINVAL;
         goto out;
     }
@@ -1836,7 +1835,7 @@ int qemu_loadvm_state(QEMUFile *f)
     unsigned int v;
     int ret;
 
-    if (qemu_savevm_state_blocked(default_mon)) {
+    if (qemu_savevm_state_blocked(NULL)) {
         return -EINVAL;
     }
 
@@ -2080,7 +2079,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
         monitor_printf(mon, "Could not open VM state file\n");
         goto the_end;
     }
-    ret = qemu_savevm_state(mon, f);
+    ret = qemu_savevm_state(f);
     vm_state_size = qemu_ftell(f);
     qemu_fclose(f);
     if (ret < 0) {
diff --git a/sysemu.h b/sysemu.h
index 29b0e96..bc2c788 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -76,7 +76,7 @@ void do_info_snapshots(Monitor *mon);
 
 void qemu_announce_self(void);
 
-bool qemu_savevm_state_blocked(Monitor *mon);
+bool qemu_savevm_state_blocked(Error **errp);
 int qemu_savevm_state_begin(QEMUFile *f, int blk_enable, int shared);
 int qemu_savevm_state_iterate(QEMUFile *f);
 int qemu_savevm_state_complete(QEMUFile *f);
-- 
1.7.9.2.384.g4a92a