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 vmstate_load() 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/savevm.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index
> 0c445a957fc99f826e6753ed3795bcdd51f1e3f5..7f79461844105bf672314c3325caee9cdb654c27
> 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -963,14 +963,20 @@ void vmstate_unregister(VMStateIf *obj, const
> VMStateDescription *vmsd,
> }
> }
>
> -static int vmstate_load(QEMUFile *f, SaveStateEntry *se)
> +static int vmstate_load(QEMUFile *f, SaveStateEntry *se, Error **errp)
> {
> + int ret;
> trace_vmstate_load(se->idstr, se->vmsd ? se->vmsd->name : "(old)");
> if (!se->vmsd) { /* Old style */
> - return se->ops->load_state(f, se->opaque, se->load_version_id);
> + ret = se->ops->load_state(f, se->opaque, se->load_version_id);
> + if (ret < 0) {
> + error_setg(errp, "Failed to load VM version_id: %d, ret: %d",
"VM" is ambiguous. I'd use "vmstate".
> + se->load_version_id, ret);
> + }
> + return ret;
> }
> return vmstate_load_state(f, se->vmsd, se->opaque, se->load_version_id,
> - &error_fatal);
> + errp);
> }
>
> static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se,
> @@ -2692,6 +2698,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t
> type)
> SaveStateEntry *se;
> char idstr[256];
> int ret;
> + Error *local_err = NULL;
>
> /* Read section start */
> section_id = qemu_get_be32(f);
> @@ -2741,10 +2748,11 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t
> type)
> start_ts = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
> }
>
> - ret = vmstate_load(f, se);
> + ret = vmstate_load(f, se, &local_err);
> if (ret < 0) {
> error_report("error while loading state for instance 0x%"PRIx32" of"
> " device '%s'", instance_id, idstr);
> + warn_report_err(local_err);
I was about to ask why you're using a warning here, but I see you remove
it futher in the series. That's fine, but this kind of thing should be
mentioned in the commit message, otherwise it gets confusing to review.
> return ret;
> }
>
> @@ -2769,6 +2777,7 @@ qemu_loadvm_section_part_end(QEMUFile *f, uint8_t type)
> uint32_t section_id;
> SaveStateEntry *se;
> int ret;
> + Error *local_err = NULL;
>
> section_id = qemu_get_be32(f);
>
> @@ -2794,10 +2803,11 @@ qemu_loadvm_section_part_end(QEMUFile *f, uint8_t
> type)
> start_ts = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
> }
>
> - ret = vmstate_load(f, se);
> + ret = vmstate_load(f, se, &local_err);
> if (ret < 0) {
> error_report("error while loading state section id %d(%s)",
> section_id, se->idstr);
> + warn_report_err(local_err);
> return ret;
> }
