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

2014-06-06 Thread Markus Armbruster 
Fam Zheng f...@redhat.com writes:

 On Thu, 06/05 14:15, Markus Armbruster wrote:
 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 its invalid, reporting the error to the

 s/its/it's/ ?

Of course.  Perhaps it can be fixed on commit.

 Anyway,

 Reviewed-by: Fam Zheng f...@redhat.com

Thanks!

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

2014-06-05 Thread Markus Armbruster 
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 its 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 arm...@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 f2b4dca..2c68d0d 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -300,15 +300,16 @@ static void virtio_blk_handle_write(VirtIOBlockReq *req, 
MultiReqBuffer *mrb)
 
 sector = ldq_p(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);
 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);
 }
@@ -330,14 +331,15 @@ static void virtio_blk_handle_read(VirtIOBlockReq *req)
 
 sector = ldq_p(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);
 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);
-- 
1.9.3

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

2014-06-05 Thread Fam Zheng 
On Thu, 06/05 14:15, Markus Armbruster wrote:
 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 its invalid, reporting the error to the

s/its/it's/ ?

Anyway,

Reviewed-by: Fam Zheng f...@redhat.com

 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 arm...@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 f2b4dca..2c68d0d 100644
 --- a/hw/block/virtio-blk.c
 +++ b/hw/block/virtio-blk.c
 @@ -300,15 +300,16 @@ static void virtio_blk_handle_write(VirtIOBlockReq 
 *req, MultiReqBuffer *mrb)
  
  sector = ldq_p(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);
  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);
  }
 @@ -330,14 +331,15 @@ static void virtio_blk_handle_read(VirtIOBlockReq *req)
  
  sector = ldq_p(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);
  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);
 -- 
 1.9.3

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

2014-06-05 Thread Eric Blake 
On 06/05/2014 09:18 PM, Fam Zheng wrote:
 On Thu, 06/05 14:15, Markus Armbruster wrote:
 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 its invalid, reporting the error to the
 
 s/its/it's/ ?

Yes, this is a case where you are contracting it is into it's

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature