Re: [PATCH V2 4/7] block/rbd: add bdrv_attach_aio_context

[Peter Lieven]

Am 15.02.21 um 11:20 schrieb Kevin Wolf:

Am 26.01.2021 um 12:25 hat Peter Lieven geschrieben:

Signed-off-by: Peter Lieven 
---
  block/rbd.c | 15 +--
  1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index f68ebcf240..7abd0252c9 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -91,6 +91,7 @@ typedef struct BDRVRBDState {
  char *namespace;
  uint64_t image_size;
  uint64_t object_size;
+AioContext *aio_context;
  } BDRVRBDState;

A commit message explaining the why would be helpful here.

This is already stored in BlockDriverState, which should be available
everywhere. Keeping redundant information needs a good justification,
which seems unlikely when BlockDriverState and BDRVRBDState are already
connected through the BlockDriverState.opaque pointer.

The rest of the series doesn't seem to make more use of it either.



You are right. I was not aware that the aio_context is already there.

We keep a local copy of aio_context in iscsi and nfs driver as well. That

is where I got it from. I will change it if we don't drop the series completely.


Peter

Re: [PATCH V2 4/7] block/rbd: add bdrv_attach_aio_context

[Kevin Wolf]

Am 26.01.2021 um 12:25 hat Peter Lieven geschrieben:
> Signed-off-by: Peter Lieven 
> ---
>  block/rbd.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/block/rbd.c b/block/rbd.c
> index f68ebcf240..7abd0252c9 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -91,6 +91,7 @@ typedef struct BDRVRBDState {
>  char *namespace;
>  uint64_t image_size;
>  uint64_t object_size;
> +AioContext *aio_context;
>  } BDRVRBDState;

A commit message explaining the why would be helpful here.

This is already stored in BlockDriverState, which should be available
everywhere. Keeping redundant information needs a good justification,
which seems unlikely when BlockDriverState and BDRVRBDState are already
connected through the BlockDriverState.opaque pointer.

The rest of the series doesn't seem to make more use of it either.

Kevin

[PATCH V2 4/7] block/rbd: add bdrv_attach_aio_context

[Peter Lieven]

Signed-off-by: Peter Lieven 
---
 block/rbd.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index f68ebcf240..7abd0252c9 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -91,6 +91,7 @@ typedef struct BDRVRBDState {
 char *namespace;
 uint64_t image_size;
 uint64_t object_size;
+AioContext *aio_context;
 } BDRVRBDState;
 
 static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx,
@@ -749,6 +750,8 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 }
 }
 
+s->aio_context = bdrv_get_aio_context(bs);
+
 /* When extending regular files, we get zeros from the OS */
 bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
 
@@ -839,8 +842,7 @@ static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB 
*rcb)
 rcb->ret = rbd_aio_get_return_value(c);
 rbd_aio_release(c);
 
-replay_bh_schedule_oneshot_event(bdrv_get_aio_context(acb->common.bs),
- rbd_finish_bh, rcb);
+replay_bh_schedule_oneshot_event(acb->s->aio_context, rbd_finish_bh, rcb);
 }
 
 static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
@@ -1160,6 +1162,13 @@ static const char *const qemu_rbd_strong_runtime_opts[] 
= {
 NULL
 };
 
+static void qemu_rbd_attach_aio_context(BlockDriverState *bs,
+   AioContext *new_context)
+{
+BDRVRBDState *s = bs->opaque;
+s->aio_context = new_context;
+}
+
 static BlockDriver bdrv_rbd = {
 .format_name= "rbd",
 .instance_size  = sizeof(BDRVRBDState),
@@ -1189,6 +1198,8 @@ static BlockDriver bdrv_rbd = {
 .bdrv_snapshot_goto = qemu_rbd_snap_rollback,
 .bdrv_co_invalidate_cache = qemu_rbd_co_invalidate_cache,
 
+.bdrv_attach_aio_context  = qemu_rbd_attach_aio_context,
+
 .strong_runtime_opts= qemu_rbd_strong_runtime_opts,
 };
 
-- 
2.17.1