Re: [Qemu-devel] [PATCH v2 11/25] nfs: implement .bdrv_detach/attach_aio_context()

[Peter Lieven]

On 07.05.2014 12:27, Stefan Hajnoczi wrote:
> Drop the assumption that we're using the main AioContext.  The following
> functions need to be converted:
>   * qemu_bh_new() -> aio_bh_new()
>   * qemu_aio_set_fd_handler() -> aio_set_fd_handler()
>   * qemu_aio_wait() -> aio_poll()
>
> The .bdrv_detach/attach_aio_context() interfaces also need to be
> implemented to move the fd handler from the old to the new AioContext.
>
> Cc: Peter Lieven <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>   block/nfs.c | 80 ++++++++++++++++++++++++++++++++++++++++++-------------------
>   1 file changed, 56 insertions(+), 24 deletions(-)
>
> diff --git a/block/nfs.c b/block/nfs.c
> index 9fa831f..33f198b 100644
> --- a/block/nfs.c
> +++ b/block/nfs.c
> @@ -40,6 +40,7 @@ typedef struct NFSClient {
>       struct nfsfh *fh;
>       int events;
>       bool has_zero_init;
> +    AioContext *aio_context;
>   } NFSClient;
>   
>   typedef struct NFSRPC {
> @@ -49,6 +50,7 @@ typedef struct NFSRPC {
>       struct stat *st;
>       Coroutine *co;
>       QEMUBH *bh;
> +    AioContext *aio_context;
>   } NFSRPC;

If you need to respin for some reason, I would copy the pointer
to the whole NFSClient struct here just in case we need any other
member of that struct in the future.

>   
>   static void nfs_process_read(void *arg);
> @@ -58,10 +60,11 @@ static void nfs_set_events(NFSClient *client)
>   {
>       int ev = nfs_which_events(client->context);
>       if (ev != client->events) {
> -        qemu_aio_set_fd_handler(nfs_get_fd(client->context),
> -                      (ev & POLLIN) ? nfs_process_read : NULL,
> -                      (ev & POLLOUT) ? nfs_process_write : NULL,
> -                      client);
> +        aio_set_fd_handler(client->aio_context,
> +                           nfs_get_fd(client->context),
> +                           (ev & POLLIN) ? nfs_process_read : NULL,
> +                           (ev & POLLOUT) ? nfs_process_write : NULL,
> +                           client);
>   
>       }
>       client->events = ev;
> @@ -84,7 +87,8 @@ static void nfs_process_write(void *arg)
>   static void nfs_co_init_task(NFSClient *client, NFSRPC *task)
>   {
>       *task = (NFSRPC) {
> -        .co         = qemu_coroutine_self(),
> +        .co             = qemu_coroutine_self(),
> +        .aio_context    = client->aio_context,
>       };
>   }
>   
> @@ -116,7 +120,7 @@ nfs_co_generic_cb(int ret, struct nfs_context *nfs, void *data,
>           error_report("NFS Error: %s", nfs_get_error(nfs));
>       }
>       if (task->co) {
> -        task->bh = qemu_bh_new(nfs_co_generic_bh_cb, task);
> +        task->bh = aio_bh_new(task->aio_context, nfs_co_generic_bh_cb, task);
>           qemu_bh_schedule(task->bh);
>       }
>   }
> @@ -224,13 +228,34 @@ static QemuOptsList runtime_opts = {
>       },
>   };
>   
> +static void nfs_detach_aio_context(BlockDriverState *bs)
> +{
> +    NFSClient *client = bs->opaque;
> +
> +    aio_set_fd_handler(client->aio_context,
> +                       nfs_get_fd(client->context),
> +                       NULL, NULL, NULL);
> +    client->events = 0;
> +}
> +
> +static void nfs_attach_aio_context(BlockDriverState *bs,
> +                                   AioContext *new_context)
> +{
> +    NFSClient *client = bs->opaque;
> +
> +    client->aio_context = new_context;
> +    nfs_set_events(client);
> +}
> +
>   static void nfs_client_close(NFSClient *client)
>   {
>       if (client->context) {
>           if (client->fh) {
>               nfs_close(client->context, client->fh);
>           }
> -        qemu_aio_set_fd_handler(nfs_get_fd(client->context), NULL, NULL, NULL);
> +        aio_set_fd_handler(client->aio_context,
> +                           nfs_get_fd(client->context),
> +                           NULL, NULL, NULL);
>           nfs_destroy_context(client->context);
>       }
>       memset(client, 0, sizeof(NFSClient));
> @@ -341,6 +366,8 @@ static int nfs_file_open(BlockDriverState *bs, QDict 
> *options, int flags,
>       QemuOpts *opts;
>       Error *local_err = NULL;
>   
> +    client->aio_context = bdrv_get_aio_context(bs);
> +
>       opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
>       qemu_opts_absorb_qdict(opts, options, &local_err);
>       if (local_err) {
> @@ -364,6 +391,8 @@ static int nfs_file_create(const char *url, 
> QEMUOptionParameter *options,
>       int64_t total_size = 0;
>       NFSClient *client = g_malloc0(sizeof(NFSClient));
>   
> +    client->aio_context = qemu_get_aio_context();
> +
>       /* Read out options */
>       while (options && options->name) {
>           if (!strcmp(options->name, "size")) {
> @@ -403,7 +432,7 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState 
> *bs)
>   
>       while (!task.complete) {
>           nfs_set_events(client);
> -        qemu_aio_wait();
> +        aio_poll(bdrv_get_aio_context(bs), true);
>       }

Why not

aio_poll(client->aio_context, true);

?

Peter
>   
>       return (task.ret < 0 ? task.ret : st.st_blocks * st.st_blksize);
> @@ -416,22 +445,25 @@ static int nfs_file_truncate(BlockDriverState *bs, 
> int64_t offset)
>   }
>   
>   static BlockDriver bdrv_nfs = {
> -    .format_name     = "nfs",
> -    .protocol_name   = "nfs",
> -
> -    .instance_size   = sizeof(NFSClient),
> -    .bdrv_needs_filename = true,
> -    .bdrv_has_zero_init = nfs_has_zero_init,
> -    .bdrv_get_allocated_file_size = nfs_get_allocated_file_size,
> -    .bdrv_truncate = nfs_file_truncate,
> -
> -    .bdrv_file_open  = nfs_file_open,
> -    .bdrv_close      = nfs_file_close,
> -    .bdrv_create     = nfs_file_create,
> -
> -    .bdrv_co_readv         = nfs_co_readv,
> -    .bdrv_co_writev        = nfs_co_writev,
> -    .bdrv_co_flush_to_disk = nfs_co_flush,
> +    .format_name                    = "nfs",
> +    .protocol_name                  = "nfs",
> +
> +    .instance_size                  = sizeof(NFSClient),
> +    .bdrv_needs_filename            = true,
> +    .bdrv_has_zero_init             = nfs_has_zero_init,
> +    .bdrv_get_allocated_file_size   = nfs_get_allocated_file_size,
> +    .bdrv_truncate                  = nfs_file_truncate,
> +
> +    .bdrv_file_open                 = nfs_file_open,
> +    .bdrv_close                     = nfs_file_close,
> +    .bdrv_create                    = nfs_file_create,
> +
> +    .bdrv_co_readv                  = nfs_co_readv,
> +    .bdrv_co_writev                 = nfs_co_writev,
> +    .bdrv_co_flush_to_disk          = nfs_co_flush,
> +
> +    .bdrv_detach_aio_context        = nfs_detach_aio_context,
> +    .bdrv_attach_aio_context        = nfs_attach_aio_context,
>   };
>   
>   static void nfs_block_init(void)