Re: [PATCH v3 20/20] aio: remove aio_disable_external() API

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 5303 bytes --]

On Thu, Apr 20, 2023 at 03:44:06PM +0200, Philippe Mathieu-Daudé wrote:
> On 20/4/23 13:37, Stefan Hajnoczi wrote:
> > All callers now pass is_external=false to aio_set_fd_handler() and
> > aio_set_event_notifier(). The aio_disable_external() API that
> > temporarily disables fd handlers that were registered is_external=true
> > is therefore dead code.
> > 
> > Remove aio_disable_external(), aio_enable_external(), and the
> > is_external arguments to aio_set_fd_handler() and
> > aio_set_event_notifier().
> > 
> > The entire test-fdmon-epoll test is removed because its sole purpose was
> > testing aio_disable_external().
> > 
> > Parts of this patch were generated using the following coccinelle
> > (https://coccinelle.lip6.fr/) semantic patch:
> > 
> >    @@
> >    expression ctx, fd, is_external, io_read, io_write, io_poll, io_poll_ready, opaque;
> >    @@
> >    - aio_set_fd_handler(ctx, fd, is_external, io_read, io_write, io_poll, io_poll_ready, opaque)
> >    + aio_set_fd_handler(ctx, fd, io_read, io_write, io_poll, io_poll_ready, opaque)
> > 
> >    @@
> >    expression ctx, notifier, is_external, io_read, io_poll, io_poll_ready;
> >    @@
> >    - aio_set_event_notifier(ctx, notifier, is_external, io_read, io_poll, io_poll_ready)
> >    + aio_set_event_notifier(ctx, notifier, io_read, io_poll, io_poll_ready)
> > 
> > Reviewed-by: Juan Quintela <quintela@redhat.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >   include/block/aio.h           | 55 --------------------------
> >   util/aio-posix.h              |  1 -
> >   block.c                       |  7 ----
> >   block/blkio.c                 | 15 +++----
> >   block/curl.c                  | 10 ++---
> >   block/export/fuse.c           |  8 ++--
> >   block/export/vduse-blk.c      | 10 ++---
> >   block/io.c                    |  2 -
> >   block/io_uring.c              |  4 +-
> >   block/iscsi.c                 |  3 +-
> >   block/linux-aio.c             |  4 +-
> >   block/nfs.c                   |  5 +--
> >   block/nvme.c                  |  8 ++--
> >   block/ssh.c                   |  4 +-
> >   block/win32-aio.c             |  6 +--
> >   hw/i386/kvm/xen_xenstore.c    |  2 +-
> >   hw/virtio/virtio.c            |  6 +--
> >   hw/xen/xen-bus.c              |  8 ++--
> >   io/channel-command.c          |  6 +--
> >   io/channel-file.c             |  3 +-
> >   io/channel-socket.c           |  3 +-
> >   migration/rdma.c              | 16 ++++----
> >   tests/unit/test-aio.c         | 27 +------------
> >   tests/unit/test-fdmon-epoll.c | 73 -----------------------------------
> >   util/aio-posix.c              | 20 +++-------
> >   util/aio-win32.c              |  8 +---
> >   util/async.c                  |  3 +-
> >   util/fdmon-epoll.c            | 10 -----
> >   util/fdmon-io_uring.c         |  8 +---
> >   util/fdmon-poll.c             |  3 +-
> >   util/main-loop.c              |  7 ++--
> >   util/qemu-coroutine-io.c      |  7 ++--
> >   util/vhost-user-server.c      | 11 +++---
> >   tests/unit/meson.build        |  3 --
> >   34 files changed, 76 insertions(+), 290 deletions(-)
> >   delete mode 100644 tests/unit/test-fdmon-epoll.c
> 
> 
> > -/**
> > - * aio_disable_external:
> > - * @ctx: the aio context
> > - *
> > - * Disable the further processing of external clients.
> > - */
> > -static inline void aio_disable_external(AioContext *ctx)
> > -{
> > -    qatomic_inc(&ctx->external_disable_cnt);
> > -}
> > -
> > -/**
> > - * aio_enable_external:
> > - * @ctx: the aio context
> > - *
> > - * Enable the processing of external clients.
> > - */
> > -static inline void aio_enable_external(AioContext *ctx)
> > -{
> > -    int old;
> > -
> > -    old = qatomic_fetch_dec(&ctx->external_disable_cnt);
> > -    assert(old > 0);
> > -    if (old == 1) {
> > -        /* Kick event loop so it re-arms file descriptors */
> > -        aio_notify(ctx);
> > -    }
> > -}
> > -
> > -/**
> > - * aio_external_disabled:
> > - * @ctx: the aio context
> > - *
> > - * Return true if the external clients are disabled.
> > - */
> > -static inline bool aio_external_disabled(AioContext *ctx)
> > -{
> > -    return qatomic_read(&ctx->external_disable_cnt);
> > -}
> 
> Missing:
> 
> -- >8 --
> diff --git a/include/block/aio.h b/include/block/aio.h
> index d4ce01ea08..266be26f8e 100644
> --- a/include/block/aio.h
> +++ b/include/block/aio.h
> @@ -224,6 +224,4 @@ struct AioContext {
>      QEMUTimerListGroup tlg;
> 
> -    int external_disable_cnt;
> -
>      /* Number of AioHandlers without .io_poll() */
>      int poll_disable_cnt;
> diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
> index d9d3807062..5c89169e46 100644
> --- a/tests/unit/test-bdrv-drain.c
> +++ b/tests/unit/test-bdrv-drain.c
> @@ -436,5 +436,4 @@ static void test_graph_change_drain_all(void)
>      g_assert_cmpint(bs_b->quiesce_counter, ==, 0);
>      g_assert_cmpint(b_s->drain_count, ==, 0);
> -    g_assert_cmpint(qemu_get_aio_context()->external_disable_cnt, ==, 0);
> 
>      bdrv_unref(bs_b);
> ---
> 
> Once cleaned:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Oh, yes! Thank you.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]