On 2015/2/17 7:27, Eric Blake wrote:
On 02/11/2015 08:16 PM, zhanghailiang wrote:
Add a migrate state: MIG_STATE_COLO, enter this migration state
after the first live migration successfully finished.
Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com>
Signed-off-by: Gonglei <arei.gong...@huawei.com>
Signed-off-by: Lai Jiangshan <la...@cn.fujitsu.com>
---
include/migration/migration-colo.h | 2 ++
include/migration/migration.h | 13 +++++++
migration/Makefile.objs | 1 +
migration/colo.c | 72 ++++++++++++++++++++++++++++++++++++++
migration/migration.c | 38 +++++++++++---------
stubs/Makefile.objs | 1 +
stubs/migration-colo.c | 17 +++++++++
7 files changed, 128 insertions(+), 16 deletions(-)
create mode 100644 migration/colo.c
create mode 100644 stubs/migration-colo.c
+++ b/include/migration/migration.h
@@ -65,6 +65,19 @@ struct MigrationState
int64_t dirty_sync_count;
};
+enum {
+ MIG_STATE_ERROR = -1,
+ MIG_STATE_NONE,
+ MIG_STATE_SETUP,
+ MIG_STATE_CANCELLING,
+ MIG_STATE_CANCELLED,
+ MIG_STATE_ACTIVE,
+ MIG_STATE_COLO,
+ MIG_STATE_COMPLETED,
+};
Is the new state intended to be user-visible? If so, wouldn't it be
better to expose this enum via qapi-schema.json?
No, for now it is only used internally.
+
+/* #define DEBUG_COLO */
+
+#ifdef DEBUG_COLO
+#define DPRINTF(fmt, ...) \
+do { fprintf(stdout, "colo: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif
+
Same comment as in 3/27 about avoiding bit-rotting debug statements. Or
even better,...
OK, will fix it.
+static QEMUBH *colo_bh;
+
+static void *colo_thread(void *opaque)
+{
+ MigrationState *s = opaque;
+
+ qemu_mutex_lock_iothread();
+ vm_start();
+ qemu_mutex_unlock_iothread();
+ DPRINTF("vm resume to run\n");
...why not add tracepoints instead of using DPRINTF?
Hmm, we will change it to using tracepoints, for now, we use DPRINTF just for
convenience.
@@ -227,6 +218,11 @@ MigrationInfo *qmp_query_migrate(Error **errp)
get_xbzrle_cache_stats(info);
break;
+ case MIG_STATE_COLO:
+ info->has_status = true;
+ info->status = g_strdup("colo");
+ /* TODO: display COLO specific informations(checkpoint info etc.),*/
+ break;
Uggh. We REALLY need to fix MigrationInfo to convert 'status' to use an
enum type, instead of an open-coded 'str' (such a conversion is
backwards compatible, and better documented). Then it would be more
obvious that you are adding an enum value. Doing the conversion would
be a good prerequisite patch.
Good idea, i will do this, send a patch like that. ;)
s/informations(checkpoint info etc.),/information (checkpoint info etc.)/
Will fix it, thanks.
