On Wed, 15 Feb 2012 17:07:16 +0800
Wen Congyang <we...@cn.fujitsu.com> wrote:
> At 02/11/2012 03:31 AM, Luiz Capitulino Wrote:
> > 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 was 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 <aligu...@us.ibm.com>
> > Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
> > ---
> > hmp-commands.hx | 3 +-
> > hmp.c | 51 +++++++++++++++++++++++++++++++++++++++++
> > 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, 100 insertions(+), 71 deletions(-)
> >
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index 573b823..10d3f1b 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -792,8 +792,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 8ff8c94..70bd574 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)
> > @@ -851,3 +852,53 @@ 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);
> > + goto out;
> > + }
> > +
> > + monitor_resume(status->mon);
> > + qemu_del_timer(status->timer);
> > + g_free(status);
> > +
> > +out:
> > + 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 = g_malloc0(sizeof(*status));
> > + status->timer = qemu_new_timer_ms(rt_clock, hmp_migrate_status_cb,
> > + status);
> > + status->mon = mon;
> > + monitor_suspend(mon);
> > + qemu_mod_timer(status->timer, qemu_get_clock_ms(rt_clock));
> > + }
> > +}
> > diff --git a/hmp.h b/hmp.h
> > index 18eecbd..989a613 100644
> > --- a/hmp.h
> > +++ b/hmp.h
> > @@ -58,5 +58,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..99f192d 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 = qemu_get_fd(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 5c5c94c..6f148f9 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);
>
> It seems that the variable detach is not used now.
> Does it mean we always run migration at the background?
In QMP, yes.
>
> Thanks
> Wen Congyang
> > + 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 d02ee86..0cdc3ce 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -1582,3 +1582,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 b5e2ab8..84b348a 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -425,14 +425,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 3a0f683..18c07ca 100644
> > --- a/sysemu.h
> > +++ b/sysemu.h
> > @@ -65,7 +65,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);
>
