On 13/03/2015 09:27, Fam Zheng wrote: > On Fri, 03/13 09:08, Paolo Bonzini wrote: >> >> >> On 13/03/2015 07:35, Fam Zheng wrote: >>> Throttle timers won't make any progress when VCPU is not running, which >>> is prone to stall the request queue in cases like utils, qtest, >>> suspending, and live migration, unless carefully handled. What we do now >>> is crude. For example in bdrv_drain_all, requests are resumed >>> immediately without consulting throttling timer. Unfortunately >>> bdrv_drain_all is so widely used that there may be too many holes that >>> guest could bypass throttling. >>> >>> If we use the host clock, we can just trust the nested poll when waiting >>> for requests. >>> >>> Signed-off-by: Fam Zheng <f...@redhat.com> >>> --- >>> block.c | 2 +- >>> tests/test-throttle.c | 14 +++++++------- >> >> I think test-throttle.c should use the vm_clock. At some point it was >> managing the clock manually (by overriding cpu_get_clock from >> libqemustub.a), and that's only possible with QEMU_CLOCK_VIRTUAL. > > Ah! That is in iotests 093 (hint: authord by Fam Zheng :-/), which WILL be > complicated if block.c switches away from QEMU_CLOCK_VIRTUAL. But I'll do the > work if we decide to make this change. > > As to tests/test-throttle.c, I don't see its dependency on clock type, so > either way should work and I don't mind keeping it as-is at all.
If there's another way to do the same thing, I'd prefer it. For example, can we call bdrv_drain_all() at the beginning of do_vm_stop, before pausing the VCPUs? >> As to block.c, I'll leave the review to the block folks. But I think >> QEMU_CLOCK_REALTIME is preferrable. > > Real time clock should be fine, but we should review that the code handles > clock reversing. QEMU_CLOCK_HOST is the one that follows the wall clock; QEMU_CLOCK_REALTIME is monotonic. :) Paolo
