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?
> +
> +/* #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,...
> +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?
> @@ -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.
s/informations(checkpoint info etc.),/information (checkpoint info etc.)/
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
