Re: [Qemu-devel] [PATCH] ide: fix invalid TRIM range abortion for macio

[John Snow <jsnow@redhat.com>]

On 03/02/2018 12:08 PM, Anton Nefedov wrote:
> commit 947858b0 "ide: abort TRIM operation for invalid range"
> is incorrect for macio; just ide_dma_error() without doing a callback
> is not enough for that errorpath.
> 
> Instead, pass -EINVAL to the callback and handle it there
> (see related motivation for read/write in 58ac32113).
> 
> It will however catch possible EINVAL from the block layer too.
> 
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> ---
>  hw/ide/core.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 257b429..54510d4 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -402,7 +402,6 @@ typedef struct TrimAIOCB {
>      QEMUIOVector *qiov;
>      BlockAIOCB *aiocb;
>      int i, j;
> -    bool is_invalid;
>  } TrimAIOCB;
>  
>  static void trim_aio_cancel(BlockAIOCB *acb)
> @@ -430,11 +429,8 @@ static void ide_trim_bh_cb(void *opaque)
>  {
>      TrimAIOCB *iocb = opaque;
>  
> -    if (iocb->is_invalid) {
> -        ide_dma_error(iocb->s);
> -    } else {
> -        iocb->common.cb(iocb->common.opaque, iocb->ret);
> -    }
> +    iocb->common.cb(iocb->common.opaque, iocb->ret);
> +
>      qemu_bh_delete(iocb->bh);
>      iocb->bh = NULL;
>      qemu_aio_unref(iocb);
> @@ -462,7 +458,7 @@ static void ide_issue_trim_cb(void *opaque, int ret)
>                  }
>  
>                  if (!ide_sect_range_ok(s, sector, count)) {
> -                    iocb->is_invalid = true;
> +                    iocb->ret = -EINVAL;
>                      goto done;
>                  }
>  
> @@ -502,7 +498,6 @@ BlockAIOCB *ide_issue_trim(
>      iocb->qiov = qiov;
>      iocb->i = -1;
>      iocb->j = 0;
> -    iocb->is_invalid = false;
>      ide_issue_trim_cb(iocb, 0);
>      return &iocb->common;
>  }
> @@ -848,6 +843,12 @@ static void ide_dma_cb(void *opaque, int ret)
>      if (ret == -ECANCELED) {
>          return;
>      }
> +
> +    if (ret == -EINVAL) {
> +        ide_dma_error(s);
> +        return;
> +    }
> +
>      if (ret < 0) {
>          if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
>              s->bus->dma->aiocb = NULL;
> 

Thanks, applied to my IDE tree:

https://github.com/jnsnow/qemu/commits/ide
https://github.com/jnsnow/qemu.git

--js