* Juan Quintela (quint...@redhat.com) wrote:
> Juan Quintela <quint...@redhat.com> wrote:
> > "Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote:
> >> * Juan Quintela (quint...@redhat.com) 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 <quint...@redhat.com>
> >>> ---
> >>> 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 / dgilb...@redhat.com / Manchester, UK
