Tejus GK <tejus...@nutanix.com> wrote:
> There are places in migration.c where the migration is marked failed with
> MIGRATION_STATUS_FAILED, but the failure reason is never updated. Hence
> libvirt doesn't know why the migration failed when it queries for it.
>
> Signed-off-by: Tejus GK <tejus...@nutanix.com>
Reviewed-by: Juan Quintela <quint...@redhat.com>
> ---
> migration/migration.c | 23 ++++++++++++-----------
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 00d8ba8da0..864150d01d 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1683,15 +1683,11 @@ void qmp_migrate(const char *uri, bool has_blk, bool
> blk,
> } else if (strstart(uri, "fd:", &p)) {
> fd_start_outgoing_migration(s, p, &local_err);
> } else {
> - if (!(has_resume && resume)) {
> - yank_unregister_instance(MIGRATION_YANK_INSTANCE);
> - }
> - error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri",
> + error_setg(&local_err, QERR_INVALID_PARAMETER_VALUE, "uri",
> "a valid migration protocol");
> migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> MIGRATION_STATUS_FAILED);
> block_cleanup_parameters();
> - return;
If you have to respin, please split the removal of the yan_unregister()
and return in one patch, and let of this patch the error bits.
> }
>
> if (local_err) {
> @@ -2073,7 +2069,7 @@ migration_wait_main_channel(MigrationState *ms)
> * Switch from normal iteration to postcopy
> * Returns non-0 on error
> */
> -static int postcopy_start(MigrationState *ms)
> +static int postcopy_start(MigrationState *ms, Error **errp)
> {
> int ret;
> QIOChannelBuffer *bioc;
> @@ -2183,7 +2179,7 @@ static int postcopy_start(MigrationState *ms)
> */
> ret = qemu_file_get_error(ms->to_dst_file);
> if (ret) {
> - error_report("postcopy_start: Migration stream errored (pre
> package)");
> + error_setg(errp, "postcopy_start: Migration stream errored (pre
> package)");
> goto fail_closefb;
> }
>
> @@ -2220,7 +2216,7 @@ static int postcopy_start(MigrationState *ms)
>
> ret = qemu_file_get_error(ms->to_dst_file);
> if (ret) {
> - error_report("postcopy_start: Migration stream errored");
> + error_setg(errp, "postcopy_start: Migration stream errored");
> migrate_set_state(&ms->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
> MIGRATION_STATUS_FAILED);
> }
> @@ -2741,6 +2737,7 @@ typedef enum {
> static MigIterateState migration_iteration_run(MigrationState *s)
> {
> uint64_t must_precopy, can_postcopy;
> + Error *local_err = NULL;
> bool in_postcopy = s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE;
>
> qemu_savevm_state_pending_estimate(&must_precopy, &can_postcopy);
> @@ -2763,8 +2760,9 @@ static MigIterateState
> migration_iteration_run(MigrationState *s)
> /* Still a significant amount to transfer */
> if (!in_postcopy && must_precopy <= s->threshold_size &&
> qatomic_read(&s->start_postcopy)) {
> - if (postcopy_start(s)) {
> - error_report("%s: postcopy failed to start", __func__);
> + if (postcopy_start(s, &local_err)) {
> + migrate_set_error(s, local_err);
> + error_report_err(local_err);
Not your fault.
But the interface of migrate_set_error() is a mess. It is followed
sometimes from error_report_err() another from error_free() and another
by ...
