Re: [Qemu-devel] [PATCH 007/124] vmstate: Return error in case of error

[Dr. David Alan Gilbert]

* Juan Quintela (address@hidden) wrote:
> Juan Quintela <address@hidden> wrote:
> > "Dr. David Alan Gilbert" <address@hidden> wrote:
> >> * Juan Quintela (address@hidden) wrote:
> >>> If there is an error while loading a field, we should stop reading and
> >>> not continue with the rest of fields.  And we should also set an error
> >>> in qemu_file.
> >>> 
> >>> Signed-off-by: Juan Quintela <address@hidden>
> >>> ---
> >>>  vmstate.c | 6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>> 
> >>> diff --git a/vmstate.c b/vmstate.c
> >>> index bfa34cc..bcf1cde 100644
> >>> --- a/vmstate.c
> >>> +++ b/vmstate.c
> >>> @@ -74,7 +74,13 @@ int vmstate_load_state(QEMUFile *f, const 
> >>> VMStateDescription *vmsd,
> >>>                      ret = field->info->get(f, addr, size);
> >>> 
> >>>                  }
> >>> +                if (ret >= 0) {
> >>> +                    ret = qemu_file_get_error(f);
> >>> +                }
> >>>                  if (ret < 0) {
> >>> +                    if (!qemu_file_get_error(f)) {
> >>> +                        qemu_file_set_error(f, ret);
> >>> +                    }
> >>
> >> qemu_file_set_error already checks for an existing error, so
> >> you don't need that check.
> >
> > ret could already be less than zero and qemu_file error not being set
> > yet.  Problem found during testing.  That is the reason that I have to
> > "improve" the previous patch.
> >
> > Later, Juan.
> 
> qemu_file_set_error() already do the check.
> 
> I stand corrected.
> 
> if ((ret < 0) || qemu_file_get_error(f) {
>    qemu_file_set_error(f, ret);
>    return ret;
> }
> 
> sounds better?

If ret >=0  but qemu_file_get_error has an existing error
then that would return the good value from ret - is
that your intent?

Or do you want:

if (ret >= 0) {
    ret = qemu_file_get_error(f);
}

if (ret < 0) {
    qemu_file_set_error(f, ret);
    trace_vmstate_load_field_error(field->name, ret);
    return ret;
}

Dave

> 
> Thanks, Juan.
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK