[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2] block: Switch to host monotonic clock for IO

From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH v2] block: Switch to host monotonic clock for IO throttling
Date: Mon, 23 Mar 2015 14:28:46 +0000

On Mon, Mar 23, 2015 at 1:58 PM, Paolo Bonzini <address@hidden> wrote:
> On 23/03/2015 14:56, Stefan Hajnoczi wrote:
>> On Mon, Mar 23, 2015 at 01:24:15PM +0800, Fam Zheng wrote:
>>> Currently, throttle timers won't make any progress when VCPU is not
>>> running, which would stall the request queue in utils, qtest, vm
>>> suspending, and live migration without special handling.
>>> For example in bdrv_drain_all, all requests are resumed immediately
>>> without taking throttling limit into account. This means whenever it is
>>> called, IO throttling goes ineffective (examples: system reset,
>>> migration and many block job operations.).
>>> This might be some loophole that guest could exploit.
>>> If we use the host clock, we can later just trust the nested poll when
>>> waiting for requests.
>>> Note that for qemu-iotests case 093, which sets up qtest when running
>>> QEMU, we still use vm clock so the script can control the clock stepping
>>> in order to be deterministic.
>>> Signed-off-by: Fam Zheng <address@hidden>
>>> ---
>>> v2: Don't break qemu-iotests 093.
>>> ---
>>>  block.c | 9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>> Should we make an exception for live migration to reduce downtime?
>> I'm concerned that now vm_stop() can take even longer since we'll wait
>> for throttling.
> What would have prevented the wait before?  (In fact I'm not sure why we
> would have even terminated bdrv_drain_all, since we run it when
> QEMU_CLOCK_VIRTUAL is not advancing anymore!).

Hang on, I just noticed that bdrv_drain_one() disables throttling:

    bs->io_limits_enabled = false;

    for (i = 0; i < 2; i++) {
        while (qemu_co_enter_next(&bs->throttled_reqs[i])) {
            drained = true;

    bs->io_limits_enabled = enabled;

So this patch does not make bdrv_drain_all() honor I/O throttling
limits.  I find the commit description confusing when it mentions

What this patch really does is to all throttled I/O requests after
vm_stop() or when the CPU was never running in the first place.

The reason why the virtual clock was chosen for throttling is to
guarantee that we never violate the assumption that device backends
are stopped when vcpus are stopped.

I'd like to know what problems exactly this patch fixes.  The test
case for throttling actually relies on the virtual clock so it can be
stepped deterministically.  What other legitimate cases are there
where throttling is used outside a running qemu-system process?


reply via email to

[Prev in Thread] Current Thread [Next in Thread]