Hi Fabiano,
Thanks for the review.
On Fri, Aug 15, 2025 at 03:58:50PM -0300, Fabiano Rosas wrote:
> Arun Menon <arme...@redhat.com> writes:
>
> > 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_load_device_state() must report an error
> > in errp, in case of failure.
> >
> > Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> > Signed-off-by: Arun Menon <arme...@redhat.com>
> > ---
> > migration/colo.c | 4 ++--
> > migration/savevm.c | 5 +++--
> > migration/savevm.h | 2 +-
> > 3 files changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/migration/colo.c b/migration/colo.c
> > index
> > e0f713c837f5da25d67afbd02ceb6c54024ca3af..0ba22ee76a13e313793f653f43a728e3c433bbc1
> > 100644
> > --- a/migration/colo.c
> > +++ b/migration/colo.c
> > @@ -729,9 +729,9 @@ static void
> > colo_incoming_process_checkpoint(MigrationIncomingState *mis,
> > bql_lock();
> > vmstate_loading = true;
> > colo_flush_ram_cache();
> > - ret = qemu_load_device_state(fb);
> > + ret = qemu_load_device_state(fb, errp);
> > if (ret < 0) {
> > - error_setg(errp, "COLO: load device state failed");
> > + error_prepend(errp, "COLO: load device state failed: ");
>
> This will say: "COLO: load device state failed: Failed to load device
> state: %d"
>
> Just remove it.
sure, will do.
>
> > vmstate_loading = false;
> > bql_unlock();
> > return;
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index
> > 05dc392bdf4e19f340bc72bf66ba0543d56868a5..70e021597d884030c4a0dc2a7bc27d42a7371797
> > 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -3263,15 +3263,16 @@ int qemu_loadvm_state(QEMUFile *f, Error **errp)
> > return ret;
> > }
> >
> > -int qemu_load_device_state(QEMUFile *f)
> > +int qemu_load_device_state(QEMUFile *f, Error **errp)
> > {
> > + ERRP_GUARD();
>
> Is this needed here?
Not needed. Will remove, as we are not using error_prepend()
>
> > MigrationIncomingState *mis = migration_incoming_get_current();
> > int ret;
> >
> > /* Load QEMU_VM_SECTION_FULL section */
> > ret = qemu_loadvm_state_main(f, mis);
> > if (ret < 0) {
> > - error_report("Failed to load device state: %d", ret);
> > + error_setg(errp, "Failed to load device state: %d", ret);
> > return ret;
> > }
> >
> > diff --git a/migration/savevm.h b/migration/savevm.h
> > index
> > b80770b7461a60e2ad6ba5e24a7baeae73d90955..b12681839f0b1afa3255e45215d99c13a224b19f
> > 100644
> > --- a/migration/savevm.h
> > +++ b/migration/savevm.h
> > @@ -67,7 +67,7 @@ int qemu_save_device_state(QEMUFile *f);
> > int qemu_loadvm_state(QEMUFile *f, Error **errp);
> > void qemu_loadvm_state_cleanup(MigrationIncomingState *mis);
> > int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
> > -int qemu_load_device_state(QEMUFile *f);
> > +int qemu_load_device_state(QEMUFile *f, Error **errp);
> > int qemu_loadvm_approve_switchover(void);
> > int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> > bool in_postcopy);
>
Regards,
Arun
