Re: [Qemu-devel] [PATCH 0/3] throttle: use AioContext for dataplane support

[Stefan Hajnoczi <stefanha@redhat.com>]

On Wed, May 14, 2014 at 07:40:18PM +0200, Benoît Canet wrote:
> The Wednesday 14 May 2014 à 16:22:44 (+0200), Stefan Hajnoczi wrote :
> > This series applies on top of my "dataplane: use QEMU block layer" series.
> > 
> > Now that the dataplane code path is using the QEMU block layer we should make
> > I/O throttling limits safe to use.  When the block_set_io_throttle monitor
> > command is executed, the BlockDriverState's AioContext must be acquired in
> > order to prevent race conditions with the IOThread that is processing requests
> > from the guest.
> > 
> > The new block layer AioContext detach/attach mechanism needs to be extended to
> > move the throttling timer to a new AioContext.  This makes throttling work
> > across bdrv_set_aio_context() calls.
> > 
> > The result of this series is that I/O throttling works with dataplane and
> > limits may be changed at runtime using the monitor.
> > 
> > Stefan Hajnoczi (3):
> >   throttle: add throttle_detach/attach_aio_context()
> >   throttle: add detach/attach test case
> >   blockdev: acquire AioContext in block_set_io_throttle
> > 
> >  block.c                 |  7 +++++++
> >  blockdev.c              |  6 ++++++
> >  include/qemu/throttle.h | 10 ++++++++++
> >  tests/test-throttle.c   | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
> >  util/throttle.c         | 27 +++++++++++++++++++++++----
> >  5 files changed, 90 insertions(+), 9 deletions(-)
> > 
> > -- 
> > 1.9.0
> > 
> 
> I find One thing chocking is this series.
> I carefully decloupled the throttling code from the block layer so anyone could reuse it.
> I am under the impression that this series couples it back.

The coupling is just due to the current header file layout.  It is not a
real coupling to the block layer.

Throttling has a dependency on timers.  Timers are part of an event loop
(either AioContext or main loop).

The AioContext prototypes happen to be in block/aio.h but they are not a
dependency on block.h or BlockDriverState.  This means we could extract
them and move them to qemu/aiocontext.h in a separate series.

Hope this explains the block/aio.h header and shows it's not true
coupling.

Stefan