Re: [Qemu-devel] [PATCH v6 00/18] aio: drop io_flush()

[Stefan Hajnoczi]

On Thu, Jul 25, 2013 at 05:18:07PM +0200, Stefan Hajnoczi wrote:
> v6:
>  * Fix block/stream.c:close_unused_images() dangling pointer in Patch 2
>  * Rebase onto qemu.git/master
> 
> v5:
>  * Split out bdrv_delete() drain fix [bonzini]
>  * Fix commit message  [bonzini]
> 
> v4:
>  * Ensure pending BHs are processed in bdrv_drain_all() [bonzini]
> 
> v3:
>  * I forgot about this series, time to push it again!
>  * Rebase onto qemu.git/master
>  * Drop now-unused AioFlushHandler typedef [bonzini]
> 
> This series gets rid of aio's .io_flush() callback.  It's based on Paolo's
> insight that bdrv_drain_all() can be implemented using the 
> bs->tracked_requests
> list.  io_flush() is redundant since the block layer already knows if requests
> are pending.
> 
> The point of this effort is to simplify our event loop(s).  If we can reduce
> custom features like io_flush() it becomes possible to unify AioContext and
> main-loop.c, maybe even to replace it with glib's main loop.
> 
> This is also important to me for dataplane, since bdrv_drain_all() is one of
> the synchronization points between threads.  QEMU monitor commands invoke
> bdrv_drain_all() while the block device is accessed from a dataplane thread.
> 
> Background on io_flush() semantics:
> 
> The io_flush() handler must return 1 if this aio fd handler is active.  That
> is, requests are pending and we'd like to make progress by monitoring the fd.
> 
> If io_flush() returns 0, the aio event loop skips monitoring this fd.  This is
> critical for block drivers like iscsi where we have an idle TCP socket which 
> we
> want to block on *only* when there are pending requests.
> 
> The series works as follows:
> 
> Part 1 - stop relying on .io_flush()
> 
> The first patches change aio_poll() callers to check their termination
> condition themselves instead of relying on .io_flush():
> 
>   bcfa688 block: ensure bdrv_drain_all() works during bdrv_delete()
>   c1351f5 block: stop relying on io_flush() in bdrv_drain_all()
>   b75d9e5 dataplane/virtio-blk: check exit conditions before aio_poll()
>   18fc4d6 tests: adjust test-aio to new aio_poll() semantics
>   4c52a7c tests: adjust test-thread-pool to new aio_poll() semantics
> 
> Part 2 - stop calling .io_flush() from aio_poll()
> 
> The semantic change to aio_poll() is made here:
> 
>   bacae7a aio: stop using .io_flush()
> 
> Part 3 - drop io_flush() handlers, just pass NULL for the io_flush argument
> 
> Remove the now dead code from all .io_flush() users:
> 
>   720e0ad block/curl: drop curl_aio_flush()
>   2830cf3 block/gluster: drop qemu_gluster_aio_flush_cb()
>   c683e8e block/iscsi: drop iscsi_process_flush()
>   5019686 block/linux-aio: drop qemu_laio_completion_cb()
>   b862bcf block/nbd: drop nbd_have_request()
>   516e517 block/rbd: drop qemu_rbd_aio_flush_cb()
>   177090b block/sheepdog: drop have_co_req() and aio_flush_request()
>   e2e9e85 block/ssh: drop return_true()
>   7cc7bac dataplane/virtio-blk: drop flush_true() and flush_io()
>   db3cb18 thread-pool: drop thread_pool_active()
>   10a783b tests: drop event_active_cb()
> 
> Part 4 - remove io_flush arguments from aio functions
> 
> The big, mechanical patch that drops the io_flush argument:
> 
>   4a8c36b aio: drop io_flush argument
> 
> Note that I split Part 3 from Part 4 so it's easy to review individual block
> drivers.
> 
> Stefan Hajnoczi (18):
>   block: ensure bdrv_drain_all() works during bdrv_delete()
>   block: stop relying on io_flush() in bdrv_drain_all()
>   dataplane/virtio-blk: check exit conditions before aio_poll()
>   tests: adjust test-aio to new aio_poll() semantics
>   tests: adjust test-thread-pool to new aio_poll() semantics
>   aio: stop using .io_flush()
>   block/curl: drop curl_aio_flush()
>   block/gluster: drop qemu_gluster_aio_flush_cb()
>   block/iscsi: drop iscsi_process_flush()
>   block/linux-aio: drop qemu_laio_completion_cb()
>   block/nbd: drop nbd_have_request()
>   block/rbd: drop qemu_rbd_aio_flush_cb()
>   block/sheepdog: drop have_co_req() and aio_flush_request()
>   block/ssh: drop return_true()
>   dataplane/virtio-blk: drop flush_true() and flush_io()
>   thread-pool: drop thread_pool_active()
>   tests: drop event_active_cb()
>   aio: drop io_flush argument
> 
>  aio-posix.c                     | 36 ++++++------------
>  aio-win32.c                     | 37 ++++++++-----------
>  async.c                         |  4 +-
>  block.c                         | 50 ++++++++++++++++++-------
>  block/curl.c                    | 25 ++-----------
>  block/gluster.c                 | 21 ++---------
>  block/iscsi.c                   | 10 +----
>  block/linux-aio.c               | 18 +--------
>  block/nbd.c                     | 18 ++-------
>  block/rbd.c                     | 16 +-------
>  block/sheepdog.c                | 33 ++++-------------
>  block/ssh.c                     | 12 +-----
>  block/stream.c                  |  6 ++-
>  hw/block/dataplane/virtio-blk.c | 25 +++----------
>  include/block/aio.h             | 14 +------
>  main-loop.c                     |  9 ++---
>  tests/test-aio.c                | 82 
> +++++++++++++++++++++--------------------
>  tests/test-thread-pool.c        | 24 ++++++------
>  thread-pool.c                   | 11 +-----
>  19 files changed, 163 insertions(+), 288 deletions(-)

Ping?