Re: [RFC PATCH 7/8] block: use the new _change_ API instead of _can_set_ and _set_

[Paolo Bonzini]

On 7/12/22 23:19, Emanuele Giuseppe Esposito wrote:
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 674eaaa2bf..6e90ac3a6a 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -2184,8 +2184,12 @@ static int blk_do_set_aio_context(BlockBackend *blk, 
> AioContext *new_context,
>           bdrv_ref(bs);
>   
>           if (update_root_node) {
> -            ret = bdrv_child_try_set_aio_context(bs, new_context, blk->root,
> -                                                 errp);
> +            /*
> +             * update_root_node MUST be false for 
> blk_root_set_aio_ctx_commit(),
> +             * as we are already in the commit function of a transaction.
> +             */
> +            ret = bdrv_child_try_change_aio_context(bs, new_context, blk->root,
> +                                                    errp);
>               if (ret < 0) {
>                   bdrv_unref(bs);
>                   return ret;


Looking further at blk_do_set_aio_context:

        if (tgm->throttle_state) {
            bdrv_drained_begin(bs);
            throttle_group_detach_aio_context(tgm);
            throttle_group_attach_aio_context(tgm, new_context);
            bdrv_drained_end(bs);
        }

Perhaps the drained_begin/drained_end pair can be moved to 
blk_set_aio_context?  It shouldn't be needed from the change_aio_ctx 
callback, because bs is already drained.  If so, blk_do_set_aio_context 
would become just:

     if (tgm->throttle_state) {
         throttle_group_detach_aio_context(tgm);
         throttle_group_attach_aio_context(tgm, new_context);
     }
     blk->ctx = new_context;

and blk_set_aio_context would be something like:

    if (bs) {
        bdrv_ref(bs);
        ret = bdrv_child_try_set_aio_context(bs, new_context, blk->root,
                                             errp);
        if (ret < 0) {
            goto out_no_drain;
        }
        bdrv_drained_begin(bs);
    }
    ret = blk_do_set_aio_context(blk, new_context, errp);
    if (bs) {
        bdrv_drained_end(bs);
out_no_drain;
        bdrv_unref(bs);
    }
    return ret;

Paolo