Re: [RFC PATCH 14/22] block/export: Move AioContext from NBDExport to BlockExport

From: Max Reitz <mreitz@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>, qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org
Subject: Re: [RFC PATCH 14/22] block/export: Move AioContext from NBDExport to BlockExport
Date: Mon, 17 Aug 2020 16:56:02 +0200	[thread overview]
Message-ID: <2851aef1-176c-1b81-4941-dc4b08b6bfec@redhat.com> (raw)
In-Reply-To: <20200813162935.210070-15-kwolf@redhat.com>

[-- Attachment #1.1: Type: text/plain, Size: 2054 bytes --]

On 13.08.20 18:29, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  include/block/export.h |  6 ++++++
>  nbd/server.c           | 26 +++++++++++++-------------
>  2 files changed, 19 insertions(+), 13 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

> diff --git a/include/block/export.h b/include/block/export.h
> index f44290a4a2..5459f79469 100644
> --- a/include/block/export.h
> +++ b/include/block/export.h
> @@ -33,6 +33,12 @@ struct BlockExport {
>       * the export.
>       */
>      int refcount;
> +
> +    /*
> +     * The AioContex whose lock needs to be held while calling

*AioContext

> +     * BlockExportDriver callbacks.

Hm.  But other blk_exp_* functions (i.e. the refcount manipulation
functions) are fair game?

> +     */
> +    AioContext *ctx;
>  };
>  
>  extern const BlockExportDriver blk_exp_nbd;
> diff --git a/nbd/server.c b/nbd/server.c
> index 2bf30bb731..b735a68429 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c

[...]

> @@ -1466,7 +1464,7 @@ static void blk_aio_attached(AioContext *ctx, void *opaque)
>  
>      trace_nbd_blk_aio_attached(exp->name, ctx);
>  
> -    exp->ctx = ctx;
> +    exp->common.ctx = ctx;

(Not sure if Ḯ’m missing anything to that regard), but perhaps after
patch 21 we can move this part to the common block export code, and
maybe make it call a BlockExportDriver callback (that handles the rest
of this function).

>      QTAILQ_FOREACH(client, &exp->clients, next) {
>          qio_channel_attach_aio_context(client->ioc, ctx);
> @@ -1484,13 +1482,13 @@ static void blk_aio_detach(void *opaque)
>      NBDExport *exp = opaque;
>      NBDClient *client;
>  
> -    trace_nbd_blk_aio_detach(exp->name, exp->ctx);
> +    trace_nbd_blk_aio_detach(exp->name, exp->common.ctx);
>  
>      QTAILQ_FOREACH(client, &exp->clients, next) {
>          qio_channel_detach_aio_context(client->ioc);
>      }
>  
> -    exp->ctx = NULL;
> +    exp->common.ctx = NULL;

(same here)

Max

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