Paolo Bonzini <pbonz...@redhat.com> writes:
> This lets AioContexts be used (optionally) with a glib main loop.
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> aio-posix.c | 4 ++++
> aio-win32.c | 4 ++++
> async.c | 65
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> qemu-aio.h | 23 ++++++++++++++++++++++
> 4 file modificati, 95 inserzioni(+). 1 rimozione(-)
>
> diff --git a/aio-posix.c b/aio-posix.c
> index c848a9f..e29ece9 100644
> --- a/aio-posix.c
> +++ b/aio-posix.c
> @@ -56,6 +56,8 @@ void aio_set_fd_handler(AioContext *ctx,
> /* Are we deleting the fd handler? */
> if (!io_read && !io_write) {
> if (node) {
> + g_source_remove_poll(&ctx->source, &node->pfd);
> +
> /* If the lock is held, just mark the node as deleted */
> if (ctx->walking_handlers) {
> node->deleted = 1;
> @@ -75,6 +77,8 @@ void aio_set_fd_handler(AioContext *ctx,
> node = g_malloc0(sizeof(AioHandler));
> node->pfd.fd = fd;
> QLIST_INSERT_HEAD(&ctx->aio_handlers, node, node);
> +
> + g_source_add_poll(&ctx->source, &node->pfd);
> }
> /* Update handler with latest information */
> node->io_read = io_read;
> diff --git a/aio-win32.c b/aio-win32.c
> index c46dfb2..5057371 100644
> --- a/aio-win32.c
> +++ b/aio-win32.c
> @@ -45,6 +45,8 @@ void aio_set_event_notifier(AioContext *ctx,
> /* Are we deleting the fd handler? */
> if (!io_notify) {
> if (node) {
> + g_source_remove_poll(&ctx->source, &node->pfd);
> +
Why remove vs. setting events = 0?
add_poll/remove_poll also comes with an event loop notify which I don't
think is strictly necessary here.
> /* If the lock is held, just mark the node as deleted */
> if (ctx->walking_handlers) {
> node->deleted = 1;
> @@ -66,6 +68,8 @@ void aio_set_event_notifier(AioContext *ctx,
> node->pfd.fd = (uintptr_t)event_notifier_get_handle(e);
> node->pfd.events = G_IO_IN;
> QLIST_INSERT_HEAD(&ctx->aio_handlers, node, node);
> +
> + g_source_add_poll(&ctx->source, &node->pfd);
> }
> /* Update handler with latest information */
> node->io_notify = io_notify;
> diff --git a/async.c b/async.c
> index 513bdd7..ed2bd3f 100644
> --- a/async.c
> +++ b/async.c
> @@ -136,10 +136,73 @@ void aio_bh_update_timeout(AioContext *ctx, uint32_t
> *timeout)
> }
> }
>
> +static gboolean
> +aio_ctx_prepare(GSource *source, gint *timeout)
> +{
> + AioContext *ctx = (AioContext *) source;
> + uint32_t wait = -1;
> + aio_bh_update_timeout(ctx, &wait);
> +
> + if (wait != -1) {
> + *timeout = MIN(*timeout, wait);
> + return wait == 0;
> + }
> +
> + return FALSE;
> +}
> +
> +static gboolean
> +aio_ctx_check(GSource *source)
> +{
> + AioContext *ctx = (AioContext *) source;
> + QEMUBH *bh;
> +
> + for (bh = ctx->first_bh; bh; bh = bh->next) {
> + if (!bh->deleted && bh->scheduled) {
> + return true;
> + }
> + }
> + return aio_pending(ctx);
> +}
Think you've got some copy/paste leftover glib coding style. Probably
should use TRUE/true consistently too. I think using TRUE/FALSE for
gboolean and true/false for bool is reasonable.
> +
> +static gboolean
> +aio_ctx_dispatch(GSource *source,
> + GSourceFunc callback,
> + gpointer user_data)
> +{
> + AioContext *ctx = (AioContext *) source;
> +
> + assert(callback == NULL);
> + aio_poll(ctx, false);
> + return TRUE;
> +}
> +
> +static GSourceFuncs aio_source_funcs = {
> + aio_ctx_prepare,
> + aio_ctx_check,
> + aio_ctx_dispatch,
> + NULL
> +};
> +
> +GSource *aio_get_g_source(AioContext *ctx)
> +{
> + g_source_ref(&ctx->source);
> + return &ctx->source;
> +}
>
> AioContext *aio_context_new(void)
> {
> - return g_new0(AioContext, 1);
> + return (AioContext *) g_source_new(&aio_source_funcs,
> sizeof(AioContext));
> +}
> +
> +void aio_context_ref(AioContext *ctx)
> +{
> + g_source_ref(&ctx->source);
> +}
> +
> +void aio_context_unref(AioContext *ctx)
> +{
> + g_source_unref(&ctx->source);
> }
>
> void aio_flush(AioContext *ctx)
> diff --git a/qemu-aio.h b/qemu-aio.h
> index ac24896..aedf66c 100644
> --- a/qemu-aio.h
> +++ b/qemu-aio.h
> @@ -44,6 +44,8 @@ typedef void QEMUBHFunc(void *opaque);
> typedef void IOHandler(void *opaque);
>
> typedef struct AioContext {
> + GSource source;
> +
> /* The list of registered AIO handlers */
> QLIST_HEAD(, AioHandler) aio_handlers;
>
> @@ -75,6 +77,22 @@ typedef int (AioFlushEventNotifierHandler)(EventNotifier
> *e);
> AioContext *aio_context_new(void);
>
> /**
> + * aio_context_ref:
> + * @ctx: The AioContext to operate on.
> + *
> + * Add a reference to an AioContext.
> + */
> +void aio_context_ref(AioContext *ctx);
> +
> +/**
> + * aio_context_unref:
> + * @ctx: The AioContext to operate on.
> + *
> + * Drop a reference to an AioContext.
> + */
> +void aio_context_unref(AioContext *ctx);
> +
> +/**
> * aio_bh_new: Allocate a new bottom half structure.
> *
> * Bottom halves are lightweight callbacks whose invocation is guaranteed
> @@ -188,6 +206,11 @@ void aio_set_event_notifier(AioContext *ctx,
> EventNotifierHandler *io_read,
> AioFlushEventNotifierHandler *io_flush);
>
> +/* Return a GSource that lets the main loop poll the file descriptors
> attached
> + * to this AioContext.
> + */
> +GSource *aio_get_g_source(AioContext *ctx);
> +
> /* Functions to operate on the main QEMU AioContext. */
>
> void qemu_aio_flush(void);
> --
> 1.7.12
I kind of dislike the fact that we've got a single source for all bottom
halves but this is definitely a good starting point.
The GSource implementation looks right to me.
Regards,
Anthony Liguori
