Re: [Qemu-devel] [PATCH] fix raw_aio_read\write error handling

[Anthony Liguori <aliguori@us.ibm.com>]

Stefano Stabellini wrote:
> Currently when qemu_paio_read or qemu_paio_write return an error we call
> qemu_aio_release without removing the request from the list.
> I know that in the current implementation qemu_paio_write\read don't return
> any error, but still the behavior is wrong, especially considering
> that the implementation of these two functions is likely to change in is
> the future.
> This patch fixes the problem adding a raw_aio_remove function that
> removes the callback from the queue and also calls qemu_aio_release.
> raw_aio_remove is called by raw_aio_read, raw_aio_write and
> raw_aio_cancel.
>   

Good catch.  Applied.  Thanks.

Regards,

Anthony Liguori

> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
>
> ---
>
> diff --git a/block-raw-posix.c b/block-raw-posix.c
> index 4404eb1..ac88fc3 100644
> --- a/block-raw-posix.c
> +++ b/block-raw-posix.c
> @@ -598,6 +598,24 @@ static void raw_aio_em_cb(void* opaque)
>      qemu_aio_release(acb);
>  }
>  
> +static void raw_aio_remove(RawAIOCB *acb)
> +{
> +    RawAIOCB **pacb;
> +
> +    /* remove the callback from the queue */
> +    pacb = &posix_aio_state->first_aio;
> +    for(;;) {
> +        if (*pacb == NULL) {
> +            break;
> +        } else if (*pacb == acb) {
> +            *pacb = acb->next;
> +            qemu_aio_release(acb);
> +            break;
> +        }
> +        pacb = &acb->next;
> +    }
> +}
> +
>  static BlockDriverAIOCB *raw_aio_read(BlockDriverState *bs,
>          int64_t sector_num, uint8_t *buf, int nb_sectors,
>          BlockDriverCompletionFunc *cb, void *opaque)
> @@ -623,7 +641,7 @@ static BlockDriverAIOCB *raw_aio_read(BlockDriverState *bs,
>      if (!acb)
>          return NULL;
>      if (qemu_paio_read(&acb->aiocb) < 0) {
> -        qemu_aio_release(acb);
> +        raw_aio_remove(acb);
>          return NULL;
>      }
>      return &acb->common;
> @@ -654,7 +672,7 @@ static BlockDriverAIOCB *raw_aio_write(BlockDriverState *bs,
>      if (!acb)
>          return NULL;
>      if (qemu_paio_write(&acb->aiocb) < 0) {
> -        qemu_aio_release(acb);
> +        raw_aio_remove(acb);
>          return NULL;
>      }
>      return &acb->common;
> @@ -664,7 +682,6 @@ static void raw_aio_cancel(BlockDriverAIOCB *blockacb)
>  {
>      int ret;
>      RawAIOCB *acb = (RawAIOCB *)blockacb;
> -    RawAIOCB **pacb;
>  
>      ret = qemu_paio_cancel(acb->aiocb.aio_fildes, &acb->aiocb);
>      if (ret == QEMU_PAIO_NOTCANCELED) {
> @@ -673,18 +690,7 @@ static void raw_aio_cancel(BlockDriverAIOCB *blockacb)
>          while (qemu_paio_error(&acb->aiocb) == EINPROGRESS);
>      }
>  
> -    /* remove the callback from the queue */
> -    pacb = &posix_aio_state->first_aio;
> -    for(;;) {
> -        if (*pacb == NULL) {
> -            break;
> -        } else if (*pacb == acb) {
> -            *pacb = acb->next;
> -            qemu_aio_release(acb);
> -            break;
> -        }
> -        pacb = &acb->next;
> -    }
> +    raw_aio_remove(acb);
>  }
>  #else /* CONFIG_AIO */
>  static int posix_aio_init(void)
>
>
>
>