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

[Paolo Bonzini <pbonzini@redhat.com>]

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