On Fri, 08/28 12:53, Stefan Hajnoczi wrote: > On Wed, Jul 29, 2015 at 12:42:03PM +0800, Fam Zheng wrote: > > v2: Switch to disable/enable model. [Paolo] > > > > Most existing nested aio_poll()'s in block layer are inconsiderate of > > dispatching potential new r/w requests from ioeventfds and nbd exports, > > which > > might result in responsiveness issues (e.g. bdrv_drain_all will not return > > when > > new requests keep coming), or even wrong semantics (e.g. qmp_transaction > > cannot > > enforce atomicity due to aio_poll in bdrv_drain_all). > > > > Previous attampts to address this issue include new op blocker[1], > > bdrv_lock[2] > > and nested AioContext (patches not posted to qemu-devel). > > > > This approach is based on the idea proposed by Paolo Bonzini. The original > > idea > > is introducing "aio_context_disable_client / aio_context_enable_client to > > filter AioContext handlers according to the "client", e.g. > > AIO_CLIENT_DATAPLANE (ioeventfd), AIO_CLIENT_PROTOCOL, > > AIO_CLIENT_NBD_SERVER, > > AIO_CLIENT_CONTEXT, ... Extend aio_set_{event_notifier,fd}_handler to pass a > > client (type)." > > > > After this series, block layer aio_poll() will only process those "protocol" > > fds that are used in block I/O, plus the ctx->notifier for aio_notify(); > > other > > aio_poll()'s keep unchanged. > > > > The biggest advantage over approaches [1] and [2] is, no change is needed in > > virtio-{blk,scsi}-dataplane code, also this doesn't depend on converting > > QMP to > > coroutines. > > > > The windows implementation is not tested except for compiling. > > > > [1]: https://lists.gnu.org/archive/html/qemu-block/2015-05/msg00800.html > > [2]: http://lists.nongnu.org/archive/html/qemu-block/2015-06/msg00027.html > > > > > > Fam Zheng (11): > > aio: Introduce "type" in aio_set_fd_handler and aio_set_event_notifier > > aio: Save type to AioHandler > > block: Mark fd handlers as "protocol" > > nbd: Mark fd handlers client type as "nbd server" > > aio: Mark ctx->notifier's client type as "context" > > dataplane: Mark host notifiers' client type as "dataplane" > > aio-posix: introduce aio_{disable,enable}_clients > > aio-win32: Implement aio_{disable,enable}_clients > > block: Introduce bdrv_aio_poll > > block: Replace nested aio_poll with bdrv_aio_poll > > block: Only poll block layer fds in bdrv_aio_poll > > > > aio-posix.c | 23 ++++++++++++++-- > > aio-win32.c | 22 +++++++++++++++- > > async.c | 3 ++- > > block.c | 2 +- > > block/curl.c | 16 +++++++----- > > block/io.c | 28 +++++++++++++------- > > block/iscsi.c | 9 +++---- > > block/linux-aio.c | 5 ++-- > > block/nbd-client.c | 10 ++++--- > > block/nfs.c | 19 ++++++-------- > > block/qed-table.c | 8 +++--- > > block/sheepdog.c | 32 +++++++++++++++-------- > > block/ssh.c | 5 ++-- > > block/win32-aio.c | 5 ++-- > > blockjob.c | 2 +- > > hw/block/dataplane/virtio-blk.c | 6 +++-- > > hw/scsi/virtio-scsi-dataplane.c | 24 +++++++++++------ > > include/block/aio.h | 33 +++++++++++++++++++++++ > > include/block/block.h | 2 ++ > > nbd.c | 4 ++- > > qemu-img.c | 2 +- > > qemu-io-cmds.c | 4 +-- > > tests/test-aio.c | 58 > > +++++++++++++++++++++++------------------ > > 23 files changed, 219 insertions(+), 103 deletions(-) > > What is the status of this series? > > Paolo's points about Patch 11 & 12 stand. It seems the fd type concept > is good but the bdrv_aio_poll() mechanism requires changes to fully > solve the problem.
Let's drop bdrv_aio_poll(), introduce bdrv_quiesce() and bdrv_unquiesce() to protect nested aio_poll(). For example in QMP, add them between prepare and commit. That needs more changes, like splitting drive_backup_prepare() and put the starting of coroutine and installing of "before_write" notifier to BdrvActionOps.commit. Fam