[v2,06/17] bochs: Implement .bdrv_co_preadv() interface

Commit Message

Kevin Wolf April 28, 2016, 1:16 p.m. UTC

Signed-off-by: Kevin Wolf <[e-mail]>
---
 block/bochs.c | 51 +++++++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 18 deletions(-)

Comments

Eric Blake April 28, 2016, 3:50 p.m. UTC | #1

On 04/28/2016 07:16 AM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <[e-mail]>
> ---
>  block/bochs.c | 51 +++++++++++++++++++++++++++++++++------------------
>  1 file changed, 33 insertions(+), 18 deletions(-)
> 

> +    uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
> +    int nb_sectors = bytes >> BDRV_SECTOR_BITS;

Since we're using the named constant here (instead of /512 or >>9),...

> +    uint64_t bytes_done = 0;
> +    QEMUIOVector local_qiov;
>      int ret;
>  
> +    assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
> +    assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
> +
> +    qemu_iovec_init(&local_qiov, qiov->niov);
> +    qemu_co_mutex_lock(&s->lock);
> +
>      while (nb_sectors > 0) {
>          int64_t block_offset = seek_to_sector(bs, sector_num);
>          if (block_offset < 0) {
> -            return block_offset;
> -        } else if (block_offset > 0) {
> -            ret = bdrv_pread(bs->file->bs, block_offset, buf, 512);
> +            ret = block_offset;
> +            goto fail;
> +        }
> +
> +        qemu_iovec_reset(&local_qiov);
> +        qemu_iovec_concat(&local_qiov, qiov, bytes_done, 512);

should we also use the named constant BDRV_SECTOR_SIZE here instead of 512?

I don't care strongly enough for a respin, though, particularly since it
would affect line wrapping.

Reviewed-by: Eric Blake <[e-mail]>

> +
> +        if (block_offset > 0) {
> +            ret = bdrv_co_preadv(bs->file->bs, block_offset, 512,
> +                                 &local_qiov, 0);
>              if (ret < 0) {
> -                return ret;
> +                goto fail;
>              }
>          } else {
> -            memset(buf, 0, 512);
> +            qemu_iovec_memset(&local_qiov, 0, 0, 512);
>          }
>          nb_sectors--;
>          sector_num++;
> -        buf += 512;
> +        bytes_done += 512;

More of the magic 512, if you care.

Patch

diff --git a/block/bochs.c b/block/bochs.c
index af8b7ab..f0e18c0 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -104,6 +104,7 @@  static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
     int ret;
 
     bs->read_only = 1; // no write support yet
+    bs->request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O supported */
 
     ret = bdrv_pread(bs->file->bs, 0, &bochs, sizeof(bochs));
     if (ret < 0) {
@@ -221,38 +222,52 @@  static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)
     return bitmap_offset + (512 * (s->bitmap_blocks + extent_offset));
 }
 
-static int bochs_read(BlockDriverState *bs, int64_t sector_num,
-                    uint8_t *buf, int nb_sectors)
+static int coroutine_fn
+bochs_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+                QEMUIOVector *qiov, int flags)
 {
+    BDRVBochsState *s = bs->opaque;
+    uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
+    int nb_sectors = bytes >> BDRV_SECTOR_BITS;
+    uint64_t bytes_done = 0;
+    QEMUIOVector local_qiov;
     int ret;
 
+    assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
+    assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+
+    qemu_iovec_init(&local_qiov, qiov->niov);
+    qemu_co_mutex_lock(&s->lock);
+
     while (nb_sectors > 0) {
         int64_t block_offset = seek_to_sector(bs, sector_num);
         if (block_offset < 0) {
-            return block_offset;
-        } else if (block_offset > 0) {
-            ret = bdrv_pread(bs->file->bs, block_offset, buf, 512);
+            ret = block_offset;
+            goto fail;
+        }
+
+        qemu_iovec_reset(&local_qiov);
+        qemu_iovec_concat(&local_qiov, qiov, bytes_done, 512);
+
+        if (block_offset > 0) {
+            ret = bdrv_co_preadv(bs->file->bs, block_offset, 512,
+                                 &local_qiov, 0);
             if (ret < 0) {
-                return ret;
+                goto fail;
             }
         } else {
-            memset(buf, 0, 512);
+            qemu_iovec_memset(&local_qiov, 0, 0, 512);
         }
         nb_sectors--;
         sector_num++;
-        buf += 512;
+        bytes_done += 512;
     }
-    return 0;
-}
 
-static coroutine_fn int bochs_co_read(BlockDriverState *bs, int64_t sector_num,
-                                      uint8_t *buf, int nb_sectors)
-{
-    int ret;
-    BDRVBochsState *s = bs->opaque;
-    qemu_co_mutex_lock(&s->lock);
-    ret = bochs_read(bs, sector_num, buf, nb_sectors);
+    ret = 0;
+fail:
     qemu_co_mutex_unlock(&s->lock);
+    qemu_iovec_destroy(&local_qiov);
+
     return ret;
 }
 
@@ -267,7 +282,7 @@  static BlockDriver bdrv_bochs = {
     .instance_size	= sizeof(BDRVBochsState),
     .bdrv_probe		= bochs_probe,
     .bdrv_open		= bochs_open,
-    .bdrv_read          = bochs_co_read,
+    .bdrv_co_preadv = bochs_co_preadv,
     .bdrv_close		= bochs_close,
 };