Re: [Qemu-devel] [PATCH for-1.4 stable] block: handle spurious coroutine entries

[Paolo Bonzini]

Il 09/02/2013 17:44, Stefan Hajnoczi ha scritto:
> bdrv_co_io_em(), bdrv_co_flush(), and bdrv_co_discard() yield the
> coroutine when waiting for aio to complete.  They do not check that the
> request has actually finished.
> 
> In simple cases this works, but it returns early when we get spurious
> wake-ups due to qemu_coroutine_enter() being called from other sources.
> One such example is block-migration.c:process_incoming_migration().
> 
> This patch fixes a segfault on incoming block migration.
> 
> Reported-by: David Pravec <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>

I would prefer to have ret initialized to -EINPROGRESS, but since we're
close to release it's easier this way.

Reviewed-by: Paolo Bonzini <address@hidden>

> ---
> This patch is partially fixes block migration.  There is another issue that
> causes block migration to fail with "Unknown flags" at 99%.
> 
>  block.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 50dab8e..f2f3b80 100644
> --- a/block.c
> +++ b/block.c
> @@ -3961,6 +3961,7 @@ void qemu_aio_release(void *p)
>  typedef struct CoroutineIOCompletion {
>      Coroutine *coroutine;
>      int ret;
> +    bool done;
>  } CoroutineIOCompletion;
>  
>  static void bdrv_co_io_em_complete(void *opaque, int ret)
> @@ -3968,6 +3969,7 @@ static void bdrv_co_io_em_complete(void *opaque, int 
> ret)
>      CoroutineIOCompletion *co = opaque;
>  
>      co->ret = ret;
> +    co->done = true;
>      qemu_coroutine_enter(co->coroutine, NULL);
>  }
>  
> @@ -3992,7 +3994,9 @@ static int coroutine_fn bdrv_co_io_em(BlockDriverState 
> *bs, int64_t sector_num,
>      if (!acb) {
>          return -EIO;
>      }
> -    qemu_coroutine_yield();
> +    while (!co.done) {
> +        qemu_coroutine_yield();
> +    }
>  
>      return co.ret;
>  }
> @@ -4051,7 +4055,9 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
>          if (acb == NULL) {
>              ret = -EIO;
>          } else {
> -            qemu_coroutine_yield();
> +            while (!co.done) {
> +                qemu_coroutine_yield();
> +            }
>              ret = co.ret;
>          }
>      } else {
> @@ -4161,7 +4167,9 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, 
> int64_t sector_num,
>          if (acb == NULL) {
>              return -EIO;
>          } else {
> -            qemu_coroutine_yield();
> +            while (!co.done) {
> +                qemu_coroutine_yield();
> +            }
>              return co.ret;
>          }
>      } else {
>