Re: [Qemu-devel] [PATCH 04/18] savevm: set right return value for qemu_file_rate_limit

[Lei Li]

On 08/23/2013 01:34 PM, Paolo Bonzini wrote:
> > Say, In ram_save_iterate(), the current logic is:
> >
> > ret = qemu_file_rate_limit();
> > while(ret == 0) {
> >       save RAM blocks until no more to send.
> > }
> > if (ret < 0) {
> >       return ret;
> > }
> > ...
> >
> > And in savevm layer, qemu_savevm_state_iterate() set an error if the return
> > value of ram_save_iterate < 0.
> But that is to report errors *not in the QEMUFile*.  Errors in the
> QEMUFile are already reported by qemu_file_get_error(), and
> qemu_savevm_state_iterate() will not overwrite them.

Sorry, a little confusion.
In qemu_savevm_state_iterate(), if the return of ram_save_iterate < 0,
it will set error in QEMUFile by qemu_file_set_error(f, ret), the error
reported by qemu_file_get_error() in the QEMUFile is got from
qemu_file_set_error() by reading the f->last_error, right?

And now as qemu_file_rate_limit() never return negative value, what's the
meaning for the check: if (qemu_file_rate_limit(f) < 0) in ram_save_iterate()?

> qemu_file_rate_limit() returning 1 is enough to exit the loop,
> which is all that is needed.
> > Obviously the return value of qemu_file_rate_limit() should have an negative
> > value for there has been an error. Otherwise we need to modify the logic
> > above.
> It is not obvious to me... what is, again, the bug that you're observing?
> I think it is happening only because you're modifying the migration thread's
> body.  If you use the normal code of the migration thread, it will just work.
>
> Paolo


--
Lei