Re: [Qemu-devel] [PATCH v2 2.1 2/4] virtio-blk: Bypass error action and I/O accounting on invalid r/w

[Kevin Wolf <kwolf@redhat.com>]

Am 04.07.2014 um 15:31 hat Markus Armbruster geschrieben:
> When a device model's I/O operation fails, we execute the error
> action.  This lets layers above QEMU implement thin provisioning, or
> attempt to correct errors before they reach the guest.  But when the
> I/O operation fails because it's invalid, reporting the error to the
> guest is the only sensible action.
> 
> If the guest's read or write asks for an invalid sector range, fail
> the request right away, without considering the error action.  No
> change with error action BDRV_ACTION_REPORT.
> 
> Furthermore, bypass I/O accounting, because we want to track only I/O
> that actually reaches the block layer.
> 
> The next commit will extend "invalid sector range" to cover attempts
> to read/write beyond the end of the medium.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Fam Zheng <famz@redhat.com>
> ---
>  hw/block/virtio-blk.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index d946fa9..c8952c0 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -307,15 +307,16 @@ static void virtio_blk_handle_write(VirtIOBlockReq *req, MultiReqBuffer *mrb)
>  
>      sector = virtio_ldq_p(VIRTIO_DEVICE(req->dev), &req->out.sector);
>  
> -    bdrv_acct_start(req->dev->bs, &req->acct, req->qiov.size, BDRV_ACCT_WRITE);
> -
>      trace_virtio_blk_handle_write(req, sector, req->qiov.size / 512);
>  
>      if (!virtio_blk_sect_range_ok(req->dev, sector, req->qiov.size)) {
> -        virtio_blk_rw_complete(req, -EIO);
> +        virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
> +        g_free(req);

This should probably be virtio_blk_free_request() now, which was only
recently introduced.

>          return;
>      }
>  
> +    bdrv_acct_start(req->dev->bs, &req->acct, req->qiov.size, BDRV_ACCT_WRITE);
> +
>      if (mrb->num_writes == 32) {
>          virtio_submit_multiwrite(req->dev->bs, mrb);
>      }
> @@ -337,14 +338,15 @@ static void virtio_blk_handle_read(VirtIOBlockReq *req)
>  
>      sector = virtio_ldq_p(VIRTIO_DEVICE(req->dev), &req->out.sector);
>  
> -    bdrv_acct_start(req->dev->bs, &req->acct, req->qiov.size, BDRV_ACCT_READ);
> -
>      trace_virtio_blk_handle_read(req, sector, req->qiov.size / 512);
>  
>      if (!virtio_blk_sect_range_ok(req->dev, sector, req->qiov.size)) {
> -        virtio_blk_rw_complete(req, -EIO);
> +        virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
> +        g_free(req);

Same here.

>          return;
>      }
> +
> +    bdrv_acct_start(req->dev->bs, &req->acct, req->qiov.size, BDRV_ACCT_READ);
>      bdrv_aio_readv(req->dev->bs, sector, &req->qiov,
>                     req->qiov.size / BDRV_SECTOR_SIZE,
>                     virtio_blk_rw_complete, req);

Kevin