Hi
On Fri, Jul 25, 2025 at 5:46 PM Marc-André Lureau <
marcandre.lur...@redhat.com> wrote:
>
>
> On Fri, Jul 25, 2025 at 4:19 PM Arun Menon <arme...@redhat.com> 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 vmstate_subsection_load() must report an error
>> in errp, in case of failure.
>>
>> Signed-off-by: Arun Menon <arme...@redhat.com>
>> ---
>> migration/vmstate.c | 11 ++++++++---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/migration/vmstate.c b/migration/vmstate.c
>> index
>> 5feaa3244d259874f03048326b2497e7db32e47c..aeffeafaa4fa7582076a4f2747906ddf9aca891b
>> 100644
>> --- a/migration/vmstate.c
>> +++ b/migration/vmstate.c
>> @@ -25,7 +25,7 @@ static int vmstate_subsection_save(QEMUFile *f, const
>> VMStateDescription *vmsd,
>> void *opaque, JSONWriter *vmdesc,
>> Error **errp);
>> static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription
>> *vmsd,
>> - void *opaque);
>> + void *opaque, Error **errp);
>>
>> /* Whether this field should exist for either save or load the VM? */
>> static bool
>> @@ -225,7 +225,7 @@ int vmstate_load_state(QEMUFile *f, const
>> VMStateDescription *vmsd,
>> field++;
>> }
>> assert(field->flags == VMS_END);
>> - ret = vmstate_subsection_load(f, vmsd, opaque);
>> + ret = vmstate_subsection_load(f, vmsd, opaque, NULL);
>> if (ret != 0) {
>> qemu_file_set_error(f, ret);
>> return ret;
>> @@ -566,7 +566,7 @@ vmstate_get_subsection(const VMStateDescription *
>> const *sub,
>> }
>>
>> static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription
>> *vmsd,
>> - void *opaque)
>> + void *opaque, Error **errp)
>> {
>> trace_vmstate_subsection_load(vmsd->name);
>>
>> @@ -598,6 +598,8 @@ static int vmstate_subsection_load(QEMUFile *f, const
>> VMStateDescription *vmsd,
>> sub_vmsd = vmstate_get_subsection(vmsd->subsections, idstr);
>> if (sub_vmsd == NULL) {
>> trace_vmstate_subsection_load_bad(vmsd->name, idstr,
>> "(lookup)");
>> + error_setg(errp, "VM subsection '%s' in '%s' does not exist",
>> + idstr, vmsd->name);
>> return -ENOENT;
>> }
>> qemu_file_skip(f, 1); /* subsection */
>> @@ -608,6 +610,9 @@ static int vmstate_subsection_load(QEMUFile *f, const
>> VMStateDescription *vmsd,
>> ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
>> if (ret) {
>> trace_vmstate_subsection_load_bad(vmsd->name, idstr,
>> "(child)");
>> + error_setg(errp,
>> + "Loading VM subsection '%s' in '%s' failed : %d",
>>
>
> extra space before ":"
>
> other than that
> Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
>
Actually, almost systematically when you introduce an extra **errp
argument, you should ERRP_GUARD() in the function (see include/qapi/error.h
doc). Was this discussed before? Can you update the following patches too?
>
>> + idstr, vmsd->name, ret);
>> return ret;
>> }
>> }
>>
>> --
>> 2.50.0
>>
>>
