| Message ID | 1331316786-7752-5-git-send-email-lcapitulino@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 03/09/2012 11:13 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 As a native speaker, I can state that it is unusual or even wrong to see a contraction immediately before a comma. I would do: s/That's,/That is,/ > 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 Another instance of the same grammar problem: s/it's/it is/ > one second in the future. If the migration has already finished, the > monitor lock is relased and the user can use it normally. s/relased/released/ > > All these changes should be transparent to the user. > > +++ 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. Libvirt is currently using it, but it appears that libvirt always sets it to true. Does this mean the argument is ignored no matter whether you pass true or false? Or does this value have a default if omitted, and it is safe to explicitly pass the default (I assume the default is true), and that it is wrong to pass an explicit value that contradicts the default (that is, I'm assuming passing an explicit 'detach':'false' is wrong)?
On Fri, 09 Mar 2012 11:26:53 -0700 Eric Blake <eblake@redhat.com> wrote: > On 03/09/2012 11:13 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 > > As a native speaker, I can state that it is unusual or even wrong to see > a contraction immediately before a comma. I would do: Oh, that rule is new to me. Thanks for letting me know! > > s/That's,/That is,/ > > > 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 > > Another instance of the same grammar problem: > > s/it's/it is/ > > > one second in the future. If the migration has already finished, the > > monitor lock is relased and the user can use it normally. > > s/relased/released/ > > > > > All these changes should be transparent to the user. > > > > > +++ 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. > > Libvirt is currently using it, but it appears that libvirt always sets > it to true. Does this mean the argument is ignored no matter whether > you pass true or false? Or does this value have a default if omitted, > and it is safe to explicitly pass the default (I assume the default is > true), and that it is wrong to pass an explicit value that contradicts > the default (that is, I'm assuming passing an explicit 'detach':'false' > is wrong)? 'detach' is completely ignored in QMP, it's never read.
On 03/09/2012 12:26 PM, Eric Blake wrote: > On 03/09/2012 11:13 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 > > As a native speaker, I can state that it is unusual or even wrong to see > a contraction immediately before a comma. I would do: > > s/That's,/That is,/ > >> 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 > > Another instance of the same grammar problem: > > s/it's/it is/ > >> one second in the future. If the migration has already finished, the >> monitor lock is relased and the user can use it normally. > > s/relased/released/ > >> >> All these changes should be transparent to the user. >> > >> +++ 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. > > Libvirt is currently using it, but it appears that libvirt always sets > it to true. Does this mean the argument is ignored no matter whether > you pass true or false? Yes, and we should specify that. The argument is ignored by QEMU. Regards, Anthony Liguori
On Fri, 09 Mar 2012 12:35:32 -0600 Anthony Liguori <aliguori@us.ibm.com> wrote: > >> +## > >> +# @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. > > > > Libvirt is currently using it, but it appears that libvirt always sets > > it to true. Does this mean the argument is ignored no matter whether > > you pass true or false? > > Yes, and we should specify that. The argument is ignored by QEMU. Will do for v3.
On Fri, Mar 09, 2012 at 03:13:06PM -0300, 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 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) { So using monitor_suspend is ok? is that a possible solution for the screendump issue? I could pass the Monitor through the vga callback. Tbh I think it's ugly, but at least it doesn't introduce any new command. > + 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 > >
On Sat, 10 Mar 2012 00:51:29 +0200 Alon Levy <alevy@redhat.com> wrote: > On Fri, Mar 09, 2012 at 03:13:06PM -0300, 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 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) { > > So using monitor_suspend is ok? is that a possible solution for the > screendump issue? I could pass the Monitor through the vga callback. Tbh > I think it's ugly, but at least it doesn't introduce any new command. This is only valid for HMP. > > > + 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 > > > > >
On Mon, Mar 12, 2012 at 10:21:20AM -0300, Luiz Capitulino wrote: > On Sat, 10 Mar 2012 00:51:29 +0200 > Alon Levy <alevy@redhat.com> wrote: > > > On Fri, Mar 09, 2012 at 03:13:06PM -0300, 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 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) { > > > > So using monitor_suspend is ok? is that a possible solution for the > > screendump issue? I could pass the Monitor through the vga callback. Tbh > > I think it's ugly, but at least it doesn't introduce any new command. > > This is only valid for HMP. > Yes, I'm thinking at this point for qmp to wait for the output file to be written via repeated poll / inotify. > > > > > + 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 > > > > > > > > >
On Mon, 12 Mar 2012 15:49:27 +0200 Alon Levy <alevy@redhat.com> wrote: > On Mon, Mar 12, 2012 at 10:21:20AM -0300, Luiz Capitulino wrote: > > On Sat, 10 Mar 2012 00:51:29 +0200 > > Alon Levy <alevy@redhat.com> wrote: > > > > > On Fri, Mar 09, 2012 at 03:13:06PM -0300, 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 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) { > > > > > > So using monitor_suspend is ok? is that a possible solution for the > > > screendump issue? I could pass the Monitor through the vga callback. Tbh > > > I think it's ugly, but at least it doesn't introduce any new command. > > > > This is only valid for HMP. > > > > Yes, I'm thinking at this point for qmp to wait for the output file to > be written via repeated poll / inotify. Making this synchronous you mean? This will hold the global mutex for too long, won't it?
On Mon, Mar 12, 2012 at 11:01:54AM -0300, Luiz Capitulino wrote: > On Mon, 12 Mar 2012 15:49:27 +0200 > Alon Levy <alevy@redhat.com> wrote: > > > On Mon, Mar 12, 2012 at 10:21:20AM -0300, Luiz Capitulino wrote: > > > On Sat, 10 Mar 2012 00:51:29 +0200 > > > Alon Levy <alevy@redhat.com> wrote: > > > > > > > On Fri, Mar 09, 2012 at 03:13:06PM -0300, 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 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) { > > > > > > > > So using monitor_suspend is ok? is that a possible solution for the > > > > screendump issue? I could pass the Monitor through the vga callback. Tbh > > > > I think it's ugly, but at least it doesn't introduce any new command. > > > > > > This is only valid for HMP. > > > > > > > Yes, I'm thinking at this point for qmp to wait for the output file to > > be written via repeated poll / inotify. > > Making this synchronous you mean? This will hold the global mutex for too > long, won't it? No, not at all what I mean. I'm talking about a libvirt change. There is nothing that we can do in qemu other than maybe implementing a qmp_suspend (automatic nack), already suggested screendump-async (nack by no update) and now waiting for comments on just the HMP fix for screendump.
On Mon, 12 Mar 2012 16:23:01 +0200 Alon Levy <alevy@redhat.com> wrote: > On Mon, Mar 12, 2012 at 11:01:54AM -0300, Luiz Capitulino wrote: > > On Mon, 12 Mar 2012 15:49:27 +0200 > > Alon Levy <alevy@redhat.com> wrote: > > > > > On Mon, Mar 12, 2012 at 10:21:20AM -0300, Luiz Capitulino wrote: > > > > On Sat, 10 Mar 2012 00:51:29 +0200 > > > > Alon Levy <alevy@redhat.com> wrote: > > > > > > > > > On Fri, Mar 09, 2012 at 03:13:06PM -0300, 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 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) { > > > > > > > > > > So using monitor_suspend is ok? is that a possible solution for the > > > > > screendump issue? I could pass the Monitor through the vga callback. Tbh > > > > > I think it's ugly, but at least it doesn't introduce any new command. > > > > > > > > This is only valid for HMP. > > > > > > > > > > Yes, I'm thinking at this point for qmp to wait for the output file to > > > be written via repeated poll / inotify. > > > > Making this synchronous you mean? This will hold the global mutex for too > > long, won't it? > > No, not at all what I mean. I'm talking about a libvirt change. Oh, good idea, that's a valid workaround. > There is > nothing that we can do in qemu other than maybe implementing a > qmp_suspend (automatic nack), already suggested screendump-async (nack > by no update) and now waiting for comments on just the HMP fix for > screendump. Will review it shortly.
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);