Re: [PATCH v7 13/13] block: Convert 'block_resize' to coroutine

[Stefan Hajnoczi]

On Wed, Sep 09, 2020 at 05:11:49PM +0200, Kevin Wolf wrote:
> @@ -2456,8 +2456,7 @@ void qmp_block_resize(bool has_device, const char 
> *device,
>          return;
>      }
>  
> -    aio_context = bdrv_get_aio_context(bs);
> -    aio_context_acquire(aio_context);
> +    old_ctx = bdrv_co_move_to_aio_context(bs);
>  
>      if (size < 0) {
>          error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 size");

Is it safe to call blk_new() outside the BQL since it mutates global state?

In other words, could another thread race with us?

> @@ -2479,8 +2478,8 @@ void qmp_block_resize(bool has_device, const char 
> *device,
>      bdrv_drained_end(bs);
>  
>  out:
> +    aio_co_reschedule_self(old_ctx);
>      blk_unref(blk);
> -    aio_context_release(aio_context);

The following precondition is violated by the blk_unref -> bdrv_drain ->
AIO_WAIT_WHILE() call if blk->refcnt is 1 here:

 * The caller's thread must be the IOThread that owns @ctx or the main loop
 * thread (with @ctx acquired exactly once).

blk_unref() is called from the main loop thread without having acquired
blk's AioContext.

Normally blk->refcnt will be > 1 so bdrv_drain() won't be called, but
I'm not sure if that can be guaranteed.

The following seems safer although it's uglier:

  aio_context = bdrv_get_aio_context(bs);
  aio_context_acquire(aio_context);
  blk_unref(blk);
  aio_context_release(aio_context);

signature.asc
Description: PGP signature