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

[Lei Li <lilei@linux.vnet.ibm.com>]

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