Re: [Qemu-devel] [PATCH] block: add the support to drain throttled requests

[Stefan Hajnoczi]

On Tue, Mar 20, 2012 at 10:58:10AM +0100, Kevin Wolf wrote:
> Am 20.03.2012 10:47, schrieb Paolo Bonzini:
> > Il 20/03/2012 10:40, Zhi Yong Wu ha scritto:
> >> HI, Kevin,
> >>
> >> We hope that I/O throttling can be shipped without known issue in QEMU
> >> 1.1, so if you are available, can you give this patch some love?
> > 
> > I'm sorry to say this, but I think I/O throttling is impossible to save.
> >  As it is implemented now, it just cannot work in the presence of
> > synchronous I/O, except at the cost of busy waiting with the global
> > mutex taken.  See the message from Stefan yesterday.
> 
> qemu_aio_flush() is busy waiting with the global mutex taken anyway, so
> it doesn't change that much.

Yesterday I only posted an analysis of the bug but here are some
thoughts on how to move forward.  Throttling itself is not the problem.
We've known that synchronous operations in the vcpu thread are a problem
long before throttling.  This is just another reason to convert device
emulation to use asynchronous interfaces.

Here is the list of device models that perform synchronous block I/O:
hw/fdc.c
hw/ide/atapi.c
hw/ide/core.c
hw/nand.c
hw/onenand.c
hw/pflash_cfi01.c
hw/pflash_cfi02.c
hw/sd.c

Zhi Hui Li is working on hw/fdc.c and recently sent a patch.

I think it's too close to QEMU 1.1 to convert all the remaining devices
and test them properly before the soft-freeze.  But it's probably
possible to convert IDE before the soft-freeze.

In the meantime we could add this to bdrv_rw_co():

if (bs->io_limits_enabled) {
    fprintf(stderr, "Disabling I/O throttling on '%s' due "
            "to synchronous I/O\n", bdrv_get_device_name(bs));
    bdrv_io_limits_disable(bs);
}

It's not pretty but tells the user there is an issue and avoids
deadlocking.

Stefan