On 3/28/24 15:02, Avihai Horon wrote:
There are several places where postcopy_start() fails without setting errp. This can cause a null pointer de-reference, as in case of error, the caller of postcopy_start() copies/prints the error set in errp.Fix it by setting errp in all of postcopy_start() error paths. Fixes: 908927db28ea ("migration: Update error description whenever migration fails") Signed-off-by: Avihai Horon <avih...@nvidia.com>
Reviewed-by: Cédric Le Goater <c...@redhat.com> Thanks, C.
--- migration/migration.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/migration/migration.c b/migration/migration.c index b73ae3a72c4..86bf76e9258 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2510,6 +2510,8 @@ static int postcopy_start(MigrationState *ms, Error **errp) migration_wait_main_channel(ms); if (postcopy_preempt_establish_channel(ms)) { migrate_set_state(&ms->state, ms->state, MIGRATION_STATUS_FAILED); + error_setg(errp, "%s: Failed to establish preempt channel", + __func__); return -1; } } @@ -2525,17 +2527,22 @@ static int postcopy_start(MigrationState *ms, Error **errp)ret = migration_stop_vm(ms, RUN_STATE_FINISH_MIGRATE);if (ret < 0) { + error_setg_errno(errp, -ret, "%s: Failed to stop the VM", __func__); goto fail; }ret = migration_maybe_pause(ms, &cur_state,MIGRATION_STATUS_POSTCOPY_ACTIVE); if (ret < 0) { + error_setg_errno(errp, -ret, "%s: Failed in migration_maybe_pause()", + __func__); goto fail; }ret = bdrv_inactivate_all();if (ret < 0) { + error_setg_errno(errp, -ret, "%s: Failed in bdrv_inactivate_all()", + __func__); goto fail; } restart_block = true; @@ -2612,6 +2619,7 @@ static int postcopy_start(MigrationState *ms, Error **errp)/* Now send that blob */if (qemu_savevm_send_packaged(ms->to_dst_file, bioc->data, bioc->usage)) { + error_setg(errp, "%s: Failed to send packaged data", __func__); goto fail_closefb; } qemu_fclose(fb);
