| Message ID | 1328902266-25308-7-git-send-email-lcapitulino@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 <aliguori@us.ibm.com> > Signed-off-by: Luiz Capitulino <lcapitulino@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? 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);
On Wed, 15 Feb 2012 17:07:16 +0800 Wen Congyang <wency@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 <aliguori@us.ibm.com> > > Signed-off-by: Luiz Capitulino <lcapitulino@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); >
Luiz Capitulino <lcapitulino@redhat.com> 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. > +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); 1 second is one eternity. I will ask for something smaller (100ms or 200ms range). > + qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err); This is the only user of qmp_migrate, why do we need to pass both has_blk & blk (and inc and !!inc?) Both parameters are bool, I am really confused here. Later, Juan.
On 2012-02-10 20:31, 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 <aliguori@us.ibm.com> > Signed-off-by: Luiz Capitulino <lcapitulino@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); status->mon can be NULL if suspend is not supported, see below. > + 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); Lacks return code check and handling, see original version. > + qemu_mod_timer(status->timer, qemu_get_clock_ms(rt_clock)); > + } > +} Jan
On 2012-02-15 14:31, Jan Kiszka wrote: > On 2012-02-10 20:31, 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 <aliguori@us.ibm.com> >> Signed-off-by: Luiz Capitulino <lcapitulino@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); > > status->mon can be NULL if suspend is not supported, see below. > >> + qemu_del_timer(status->timer); >> + g_free(status); >> + >> +out: Ah, and "else" please, no need for goto here. Jan
On Wed, Feb 15, 2012 at 02:25:49PM +0100, Juan Quintela wrote: > Luiz Capitulino <lcapitulino@redhat.com> 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. > > > +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); > > 1 second is one eternity. I will ask for something smaller (100ms or > 200ms range). > > > + qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err); > > This is the only user of qmp_migrate, why do we need to pass both > has_blk & blk (and inc and !!inc?) Both parameters are bool, I am > really confused here. QAPI-generated qmp_marshal_input_migrate() is another user, and we don't want to decide the defaults for incoming/blk there, so we need some way to tell qmp_migrate() whether or not the bools we pass should be ignored, or whether they were explicitly defined by the qmp user. It looks weird in this particular case, but other than having multiple variants of the qmp_migrate() command it's the only way to avoid having randomly/un- initialized values clobber the defaults expected by the user. > > Later, Juan. >
On Wed, 15 Feb 2012 10:01:22 -0600 Michael Roth <mdroth@linux.vnet.ibm.com> wrote: > On Wed, Feb 15, 2012 at 02:25:49PM +0100, Juan Quintela wrote: > > Luiz Capitulino <lcapitulino@redhat.com> 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. > > > > > +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); > > > > 1 second is one eternity. I will ask for something smaller (100ms or > > 200ms range). Well, I see two issues in reducing the timeout. The first one is that, we'll be polling more aggressively for something that doesn't finish quickly. The one is that, this is HMP, so I'm not sure humans will perceive the difference most of the time. > > > + qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err); > > > > This is the only user of qmp_migrate, why do we need to pass both > > has_blk & blk (and inc and !!inc?) Both parameters are bool, I am > > really confused here. > > QAPI-generated qmp_marshal_input_migrate() is another user, and we don't > want to decide the defaults for incoming/blk there, so we need some way > to tell qmp_migrate() whether or not the bools we pass should be > ignored, or whether they were explicitly defined by the qmp user. > > It looks weird in this particular case, For all qapi functions that take optionals parameters, yes. > but other than having multiple > variants of the qmp_migrate() command it's the only way to avoid having > randomly/un- initialized values clobber the defaults expected by the user. This question always comes from time to time, I'm wondering if we could generate a wrapper that assumes optionals are always passed or have good default values (like checking if a pointer argument is NULL, and if it's setting the has_ bool to false). I don't think this is a must have, but is a nice feature.
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); + 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);