Il 11/09/2013 13:06, Juan Quintela ha scritto:
And I think that the right solution is make qemu_get_rate_limit() to
return -1 in case of error (or the error, I don't care).
You might do both things, it would avoid the useless g_usleep you
pointed out below. But Lei's patch is good, because an error could
happen exactly during the qemu_put_be64 that writes RAM_SAVE_FLAG_EOS.
Caller checks also. This is the reason I wanted qemu_file_* callers to
return an error. It has some advantages and some disadvantages. We
don't agree on which ones are bigger O:-)
I think the disadvantages are bigger. It litters the code with error
handling, hides where things actually happen, and doesn't even simplify
QEMUFile itself. Checking only at the toplevel is simpler, all we need
to do is ensure that we get there every now and then (and that's what
qemu_file_rate_limit does).
savevm.c: qemu_savevm_state_iterate()
if (qemu_file_rate_limit(f)) {
return 0;
}
check is incorrect again, we should return an error if there is one
error.
Nothing cares if qemu_savevm_state_iterate returns 0 or negative, so
changing qemu_savevm_state_iterate to only return 0/1 would make sense too.
In this case, 0 means:
please, call us again
when what we mean is:
don't care about calling us again, there is an error. Handle the error.
Or alternatively, 0 means:
we haven't finished the work
when what we mean is:
we haven't finished the work (BTW, please check if there is an error)
Notice that qemu_save_iterate() already returns errors in other code
paths
Yes that's also unnecessary.
If we change th ereturn value for qemu_file_rate_limit() the change that
cames with this patch is not needed, that was my point.
This is what an earlier patch from Lei did. I told him (or her?) to