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

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(-)

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