On Fri, Apr 01, 2016 at 03:19:53PM +0200, Paolo Bonzini wrote:
> Eliminating the reentrancy is actually a nice thing that we can do
> with the API that Michael proposed, so let's make it first class.
> This also hides the complex assign/set_handler conventions from
> callers of virtio_queue_aio_set_host_notifier_handler, which in
> fact was always called with assign=true.
>
> Reviewed-by: Cornelia Huck <cornelia.h...@de.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> hw/block/dataplane/virtio-blk.c | 7 +++----
> hw/scsi/virtio-scsi-dataplane.c | 12 ++++--------
> hw/virtio/virtio.c | 19 ++++---------------
> include/hw/virtio/virtio.h | 6 ++----
> 4 files changed, 13 insertions(+), 31 deletions(-)
>
> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> index fd06726..74c6d37 100644
> --- a/hw/block/dataplane/virtio-blk.c
> +++ b/hw/block/dataplane/virtio-blk.c
> @@ -237,8 +237,8 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>
> /* Get this show started by hooking up our callbacks */
> aio_context_acquire(s->ctx);
> - virtio_set_queue_aio(s->vq, virtio_blk_data_plane_handle_output);
> - virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, true, true);
> + virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx,
> +
> virtio_blk_data_plane_handle_output);
> aio_context_release(s->ctx);
> return;
>
> @@ -273,8 +273,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
> aio_context_acquire(s->ctx);
>
> /* Stop notifications for new requests from guest */
> - virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, true, false);
> - virtio_set_queue_aio(s->vq, NULL);
> + virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, NULL);
>
> /* Drain and switch bs back to the QEMU main loop */
> blk_set_aio_context(s->conf->conf.blk, qemu_get_aio_context());
> diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
> index a497a2c..5494dcc 100644
> --- a/hw/scsi/virtio-scsi-dataplane.c
> +++ b/hw/scsi/virtio-scsi-dataplane.c
> @@ -81,8 +81,7 @@ static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue
> *vq, int n,
> return rc;
> }
>
> - virtio_queue_aio_set_host_notifier_handler(vq, s->ctx, true, true);
> - virtio_set_queue_aio(vq, fn);
> + virtio_queue_aio_set_host_notifier_handler(vq, s->ctx, fn);
> return 0;
> }
>
> @@ -99,13 +98,10 @@ static void virtio_scsi_clear_aio(VirtIOSCSI *s)
> VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
> int i;
>
> - virtio_queue_aio_set_host_notifier_handler(vs->ctrl_vq, s->ctx, true,
> false);
> - virtio_set_queue_aio(vs->ctrl_vq, NULL);
> - virtio_queue_aio_set_host_notifier_handler(vs->event_vq, s->ctx, true,
> false);
> - virtio_set_queue_aio(vs->event_vq, NULL);
> + virtio_queue_aio_set_host_notifier_handler(vs->ctrl_vq, s->ctx, NULL);
> + virtio_queue_aio_set_host_notifier_handler(vs->event_vq, s->ctx, NULL);
> for (i = 0; i < vs->conf.num_queues; i++) {
> - virtio_queue_aio_set_host_notifier_handler(vs->cmd_vqs[i], s->ctx,
> true, false);
> - virtio_set_queue_aio(vs->cmd_vqs[i], NULL);
> + virtio_queue_aio_set_host_notifier_handler(vs->cmd_vqs[i], s->ctx,
> NULL);
> }
> }
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index eb04ac0..7fcfc24f 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1159,14 +1159,6 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int
> queue_size,
> return &vdev->vq[i];
> }
>
> -void virtio_set_queue_aio(VirtQueue *vq,
> - void (*handle_output)(VirtIODevice *, VirtQueue *))
> -{
> - assert(vq->handle_output);
> -
> - vq->handle_aio_output = handle_output;
> -}
> -
> void virtio_del_queue(VirtIODevice *vdev, int n)
> {
> if (n < 0 || n >= VIRTIO_QUEUE_MAX) {
> @@ -1809,19 +1801,16 @@ static void
> virtio_queue_host_notifier_aio_read(EventNotifier *n)
> }
>
> void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext
> *ctx,
> - bool assign, bool
> set_handler)
> + void
> (*handle_output)(VirtIODevice *,
> + VirtQueue *))
> {
> - if (assign && set_handler) {
> + vq->handle_aio_output = handle_output;
> + if (handle_output) {
> aio_set_event_notifier(ctx, &vq->host_notifier, true,
> virtio_queue_host_notifier_aio_read);
> } else {
> aio_set_event_notifier(ctx, &vq->host_notifier, true, NULL);
> }
> - if (!assign) {
> - /* Test and clear notifier before after disabling event,
> - * in case poll callback didn't have time to run. */
> - virtio_queue_host_notifier_aio_read(&vq->host_notifier);
> - }
> }
>
> static void virtio_queue_host_notifier_read(EventNotifier *n)
This means that caller is now responsible for invoking the
handler after it sets handle_output = NULL.
I think it's cleaner to invoke it internally.
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index fa3f93b..6a37065 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -142,9 +142,6 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int
> queue_size,
> void (*handle_output)(VirtIODevice *,
> VirtQueue *));
>
> -void virtio_set_queue_aio(VirtQueue *vq,
> - void (*handle_output)(VirtIODevice *, VirtQueue
> *));
> -
> void virtio_del_queue(VirtIODevice *vdev, int n);
>
> void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_num);
> @@ -254,7 +251,8 @@ EventNotifier *virtio_queue_get_host_notifier(VirtQueue
> *vq);
> void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
> bool set_handler);
> void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext
> *ctx,
> - bool assign, bool
> set_handler);
> + void (*fn)(VirtIODevice *,
> + VirtQueue *));
> void virtio_irq(VirtQueue *vq);
> VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t vector);
> VirtQueue *virtio_vector_next_queue(VirtQueue *vq);
> --
> 1.8.3.1
>
