On Thu, Jul 17, 2025 at 06:07:41AM +0530, Arun Menon wrote:
> This is an incremental step in converting vmstate loading
> code to report error via Error objects instead of directly
> printing it to console/monitor.
> It is ensured that qemu_loadvm_state_main() must report an error
> in errp, in case of failure.
> loadvm_process_command also sets the errp object explicitly.
>
> Signed-off-by: Arun Menon <arme...@redhat.com>
> ---
> migration/colo.c | 5 +++--
> migration/savevm.c | 23 +++++++++++++----------
> migration/savevm.h | 3 ++-
> 3 files changed, 18 insertions(+), 13 deletions(-)
Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
>
> diff --git a/migration/colo.c b/migration/colo.c
> index
> e0f713c837f5da25d67afbd02ceb6c54024ca3af..ddc628cab4194b3cb82388c5e878286c820004b2
> 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -686,11 +686,12 @@ static void
> colo_incoming_process_checkpoint(MigrationIncomingState *mis,
>
> bql_lock();
> cpu_synchronize_all_states();
> - ret = qemu_loadvm_state_main(mis->from_src_file, mis);
> + ret = qemu_loadvm_state_main(mis->from_src_file, mis, &local_err);
Although earlier code in this method is using "&local_err", that
it because those methods are "void" return, so checking 'local_err'
is needed to detect failure.
In this case, however, you use 'ret' to detect failure so passing
"errp" to this method directly is the correct pattern....
> bql_unlock();
>
> if (ret < 0) {
> - error_setg(errp, "Load VM's live state (ram) error");
> + error_propagate_prepend(errp, local_err,
> + "Load VM's live state (ram) error");
..and just error_prepend here.
> @@ -3055,7 +3055,8 @@ static bool
> postcopy_pause_incoming(MigrationIncomingState *mis)
> return true;
> }
>
> -int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
> +int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
> + Error **errp)
> {
> uint8_t section_type;
> int ret = 0;
> @@ -3066,6 +3067,8 @@ retry:
>
> ret = qemu_file_get_error_obj_any(f, mis->postcopy_qemufile_dst,
> NULL);
> if (ret) {
> + error_setg(errp, "Failed to load device state section ID : %d",
> + ret);
Pass "errp" to qemu_file_get_error_obj_any() instead of NULL, to get the
real pre-existing error message.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
