On Thu, Jul 17, 2025 at 06:07:28AM +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_section_start_full() must report an error
> in errp, in case of failure.
>
> Signed-off-by: Arun Menon <arme...@redhat.com>
> ---
> migration/savevm.c | 34 ++++++++++++++++++----------------
> 1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index
> 1c1a46e4a01f812b559c451d278916426aa1cf40..63125971751394b72efc9bcaf7c4364715eef07f
> 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2684,7 +2684,7 @@ static bool check_section_footer(QEMUFile *f,
> SaveStateEntry *se)
> @@ -2705,8 +2705,8 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t
> type)
>
> ret = qemu_file_get_error(f);
> if (ret) {
> - error_report("%s: Failed to read instance/version ID: %d",
> - __func__, ret);
> + error_setg(errp, "%s: Failed to read instance/version ID: %d",
> + __func__, ret);
If we're going to use "__func__" in error messages, it should be done
universally, not as a one-off. IMHO, it is almost always not desirable
though, as error messages ought to be expressive enough to not need
the C function name included. IOW, just drop __func__, it is fine
without it here.
> @@ -2755,6 +2756,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t
> type)
> }
>
> if (!check_section_footer(f, se)) {
> + error_setg(errp, "Reading footer section failed");
Since we have them from earlier, can you include things like
idstr, instance_id, version_id as context.
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 :|
