Re: [PATCH] block/export: call blk_set_dev_ops(blk, NULL, NULL)

[Kevin Wolf <kwolf@redhat.com>]

Am 02.05.2023 um 23:11 hat Stefan Hajnoczi geschrieben:
> Most export types install BlockDeviceOps pointers. It is easy to forget
> to remove them because that happens automatically via the "drive" qdev
> property in hw/ but not block/export/.
> 
> Put blk_set_dev_ops(blk, NULL, NULL) calls in the core export.c code so
> the export types don't need to remember.
> 
> This fixes the nbd and vhost-user-blk export types.
> 
> Fixes: fd6afc501a01 ("nbd/server: Use drained block ops to quiesce the server")
> Fixes: ca858a5fe94c ("vhost-user-blk-server: notify client about disk resize")
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/export/export.c    | 2 ++
>  block/export/vduse-blk.c | 1 -
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/block/export/export.c b/block/export/export.c
> index e3fee60611..62c7c22d45 100644
> --- a/block/export/export.c
> +++ b/block/export/export.c
> @@ -192,6 +192,7 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
>      return exp;
>  
>  fail:
> +    blk_set_dev_ops(exp->blk, NULL, NULL);
>      blk_unref(blk);
>      aio_context_release(ctx);
>      if (exp) {

The last line of the context already shows that dereferencing exp
unconditionally is wrong. I'll fix it in my next series that tries to
address Fiona's concern that we need to take the graph lock even in the
main thread if we're in a coroutine.

Kevin