Re: [PATCH 2/3] block-backend: Update ctx immediately after root

2022-10-07 Thread Kevin Wolf
Am 23.09.2022 um 14:52 hat Hanna Reitz geschrieben:
> blk_get_aio_context() asserts that blk->ctx is always equal to the root
> BDS's context (if there is a root BDS).  Therefore,
> blk_do_set_aio_context() must update blk->ctx immediately after the root
> BDS's context has changed.
> 
> Without this patch, the next patch would break iotest 238, because
> bdrv_drained_begin() (called by blk_do_set_aio_context()) may then
> invoke bdrv_child_get_parent_aio_context() on the root child, i.e.
> blk_get_aio_context().  However, by this point, blk->ctx would not have
> been updated and thus differ from the root node's context.  This patch
> fixes that.
> 
> Signed-off-by: Hanna Reitz 
> ---
>  block/block-backend.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/block/block-backend.c b/block/block-backend.c
> index d4a5df2ac2..abdb5ff5af 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -2156,6 +2156,7 @@ static int blk_do_set_aio_context(BlockBackend *blk, 
> AioContext *new_context,
>  return ret;
>  }
>  }
> +blk->ctx = new_context;
>  if (tgm->throttle_state) {
>  bdrv_drained_begin(bs);
>  throttle_group_detach_aio_context(tgm);
> @@ -2164,9 +2165,10 @@ static int blk_do_set_aio_context(BlockBackend *blk, 
> AioContext *new_context,
>  }
>  
>  bdrv_unref(bs);
> +} else {
> +blk->ctx = new_context;
>  }
>  
> -blk->ctx = new_context;
>  return 0;
>  }

Makes sense. Maybe in the first branch a comment wouldn't hurt (like
"make blk->ctx consistent with the root node again before any other
operations like drain").

Reviewed-by: Kevin Wolf 




[PATCH 2/3] block-backend: Update ctx immediately after root

2022-09-23 Thread Hanna Reitz
blk_get_aio_context() asserts that blk->ctx is always equal to the root
BDS's context (if there is a root BDS).  Therefore,
blk_do_set_aio_context() must update blk->ctx immediately after the root
BDS's context has changed.

Without this patch, the next patch would break iotest 238, because
bdrv_drained_begin() (called by blk_do_set_aio_context()) may then
invoke bdrv_child_get_parent_aio_context() on the root child, i.e.
blk_get_aio_context().  However, by this point, blk->ctx would not have
been updated and thus differ from the root node's context.  This patch
fixes that.

Signed-off-by: Hanna Reitz 
---
 block/block-backend.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index d4a5df2ac2..abdb5ff5af 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2156,6 +2156,7 @@ static int blk_do_set_aio_context(BlockBackend *blk, 
AioContext *new_context,
 return ret;
 }
 }
+blk->ctx = new_context;
 if (tgm->throttle_state) {
 bdrv_drained_begin(bs);
 throttle_group_detach_aio_context(tgm);
@@ -2164,9 +2165,10 @@ static int blk_do_set_aio_context(BlockBackend *blk, 
AioContext *new_context,
 }
 
 bdrv_unref(bs);
+} else {
+blk->ctx = new_context;
 }
 
-blk->ctx = new_context;
 return 0;
 }
 
-- 
2.36.1