We are gradually moving away from sector-based interfaces, towards
byte-based.  Make the change for the internals of the qcow
driver read function, by iterating over offset/bytes instead of
sector_num/nb_sectors, and with a rename of index_in_cluster and
repurposing of n to track bytes instead of sectors.

A later patch will then switch the qcow driver as a whole over
to byte-based operation.

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v2: prefer 64-bit * over 32-bit <<, rename variable for legibility [Kevin]
---
 block/qcow.c | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 42d1bbb7643..b90915218ff 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -617,13 +617,15 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
                          int nb_sectors, QEMUIOVector *qiov)
 {
     BDRVQcowState *s = bs->opaque;
-    int index_in_cluster;
+    int offset_in_cluster;
     int ret = 0, n;
     uint64_t cluster_offset;
     struct iovec hd_iov;
     QEMUIOVector hd_qiov;
     uint8_t *buf;
     void *orig_buf;
+    int64_t offset = sector_num * BDRV_SECTOR_SIZE;
+    int64_t bytes = nb_sectors * BDRV_SECTOR_SIZE;

     if (qiov->niov > 1) {
         buf = orig_buf = qemu_try_blockalign(bs, qiov->size);
@@ -637,36 +639,35 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,

     qemu_co_mutex_lock(&s->lock);

-    while (nb_sectors != 0) {
+    while (bytes != 0) {
         /* prepare next request */
-        ret = get_cluster_offset(bs, sector_num << 9,
-                                 0, 0, 0, 0, &cluster_offset);
+        ret = get_cluster_offset(bs, offset, 0, 0, 0, 0, &cluster_offset);
         if (ret < 0) {
             break;
         }
-        index_in_cluster = sector_num & (s->cluster_sectors - 1);
-        n = s->cluster_sectors - index_in_cluster;
-        if (n > nb_sectors) {
-            n = nb_sectors;
+        offset_in_cluster = offset & (s->cluster_size - 1);
+        n = s->cluster_size - offset_in_cluster;
+        if (n > bytes) {
+            n = bytes;
         }

         if (!cluster_offset) {
             if (bs->backing) {
                 /* read from the base image */
                 hd_iov.iov_base = (void *)buf;
-                hd_iov.iov_len = n * 512;
+                hd_iov.iov_len = n;
                 qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
                 qemu_co_mutex_unlock(&s->lock);
                 /* qcow2 emits this on bs->file instead of bs->backing */
                 BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
-                ret = bdrv_co_readv(bs->backing, sector_num, n, &hd_qiov);
+                ret = bdrv_co_preadv(bs->backing, offset, n, &hd_qiov, 0);
                 qemu_co_mutex_lock(&s->lock);
                 if (ret < 0) {
                     break;
                 }
             } else {
                 /* Note: in this case, no need to wait */
-                memset(buf, 0, 512 * n);
+                memset(buf, 0, n);
             }
         } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
             /* add AIO support for compressed blocks ? */
@@ -674,21 +675,19 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
                 ret = -EIO;
                 break;
             }
-            memcpy(buf,
-                   s->cluster_cache + index_in_cluster * 512, 512 * n);
+            memcpy(buf, s->cluster_cache + offset_in_cluster, n);
         } else {
             if ((cluster_offset & 511) != 0) {
                 ret = -EIO;
                 break;
             }
             hd_iov.iov_base = (void *)buf;
-            hd_iov.iov_len = n * 512;
+            hd_iov.iov_len = n;
             qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
             qemu_co_mutex_unlock(&s->lock);
             BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
-            ret = bdrv_co_readv(bs->file,
-                                (cluster_offset >> 9) + index_in_cluster,
-                                n, &hd_qiov);
+            ret = bdrv_co_preadv(bs->file, cluster_offset + offset_in_cluster,
+                                 n, &hd_qiov, 0);
             qemu_co_mutex_lock(&s->lock);
             if (ret < 0) {
                 break;
@@ -696,8 +695,7 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
             if (bs->encrypted) {
                 assert(s->crypto);
                 if (qcrypto_block_decrypt(s->crypto,
-                                          sector_num * BDRV_SECTOR_SIZE, buf,
-                                          n * BDRV_SECTOR_SIZE, NULL) < 0) {
+                                          offset, buf, n, NULL) < 0) {
                     ret = -EIO;
                     break;
                 }
@@ -705,9 +703,9 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
         }
         ret = 0;

-        nb_sectors -= n;
-        sector_num += n;
-        buf += n * 512;
+        bytes -= n;
+        offset += n;
+        buf += n;
     }

     qemu_co_mutex_unlock(&s->lock);
-- 
2.14.3

On Thu, May 31, 2018 at 03:50:41PM -0500, Eric Blake wrote:
> We are gradually moving away from sector-based interfaces, towards
> byte-based.  Make the change for the internals of the qcow
> driver read function, by iterating over offset/bytes instead of
> sector_num/nb_sectors, and with a rename of index_in_cluster and
> repurposing of n to track bytes instead of sectors.
> 
> A later patch will then switch the qcow driver as a whole over
> to byte-based operation.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> ---
> v2: prefer 64-bit * over 32-bit <<, rename variable for legibility [Kevin]
> ---
>  block/qcow.c | 42 ++++++++++++++++++++----------------------
>  1 file changed, 20 insertions(+), 22 deletions(-)
> 
> diff --git a/block/qcow.c b/block/qcow.c
> index 42d1bbb7643..b90915218ff 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -617,13 +617,15 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
>                           int nb_sectors, QEMUIOVector *qiov)
>  {
>      BDRVQcowState *s = bs->opaque;
> -    int index_in_cluster;
> +    int offset_in_cluster;
>      int ret = 0, n;
>      uint64_t cluster_offset;
>      struct iovec hd_iov;
>      QEMUIOVector hd_qiov;
>      uint8_t *buf;
>      void *orig_buf;
> +    int64_t offset = sector_num * BDRV_SECTOR_SIZE;
> +    int64_t bytes = nb_sectors * BDRV_SECTOR_SIZE;
> 
>      if (qiov->niov > 1) {
>          buf = orig_buf = qemu_try_blockalign(bs, qiov->size);
> @@ -637,36 +639,35 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
> 
>      qemu_co_mutex_lock(&s->lock);
> 
> -    while (nb_sectors != 0) {
> +    while (bytes != 0) {
>          /* prepare next request */
> -        ret = get_cluster_offset(bs, sector_num << 9,
> -                                 0, 0, 0, 0, &cluster_offset);
> +        ret = get_cluster_offset(bs, offset, 0, 0, 0, 0, &cluster_offset);
>          if (ret < 0) {
>              break;
>          }
> -        index_in_cluster = sector_num & (s->cluster_sectors - 1);
> -        n = s->cluster_sectors - index_in_cluster;
> -        if (n > nb_sectors) {
> -            n = nb_sectors;
> +        offset_in_cluster = offset & (s->cluster_size - 1);
> +        n = s->cluster_size - offset_in_cluster;
> +        if (n > bytes) {
> +            n = bytes;
>          }
> 
>          if (!cluster_offset) {
>              if (bs->backing) {
>                  /* read from the base image */
>                  hd_iov.iov_base = (void *)buf;
> -                hd_iov.iov_len = n * 512;
> +                hd_iov.iov_len = n;
>                  qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
>                  qemu_co_mutex_unlock(&s->lock);
>                  /* qcow2 emits this on bs->file instead of bs->backing */
>                  BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
> -                ret = bdrv_co_readv(bs->backing, sector_num, n, &hd_qiov);
> +                ret = bdrv_co_preadv(bs->backing, offset, n, &hd_qiov, 0);
>                  qemu_co_mutex_lock(&s->lock);
>                  if (ret < 0) {
>                      break;
>                  }
>              } else {
>                  /* Note: in this case, no need to wait */
> -                memset(buf, 0, 512 * n);
> +                memset(buf, 0, n);
>              }
>          } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
>              /* add AIO support for compressed blocks ? */
> @@ -674,21 +675,19 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
>                  ret = -EIO;
>                  break;
>              }
> -            memcpy(buf,
> -                   s->cluster_cache + index_in_cluster * 512, 512 * n);
> +            memcpy(buf, s->cluster_cache + offset_in_cluster, n);
>          } else {
>              if ((cluster_offset & 511) != 0) {
>                  ret = -EIO;
>                  break;
>              }
>              hd_iov.iov_base = (void *)buf;
> -            hd_iov.iov_len = n * 512;
> +            hd_iov.iov_len = n;
>              qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
>              qemu_co_mutex_unlock(&s->lock);
>              BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
> -            ret = bdrv_co_readv(bs->file,
> -                                (cluster_offset >> 9) + index_in_cluster,
> -                                n, &hd_qiov);
> +            ret = bdrv_co_preadv(bs->file, cluster_offset + offset_in_cluster,
> +                                 n, &hd_qiov, 0);
>              qemu_co_mutex_lock(&s->lock);
>              if (ret < 0) {
>                  break;
> @@ -696,8 +695,7 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
>              if (bs->encrypted) {
>                  assert(s->crypto);
>                  if (qcrypto_block_decrypt(s->crypto,
> -                                          sector_num * BDRV_SECTOR_SIZE, buf,
> -                                          n * BDRV_SECTOR_SIZE, NULL) < 0) {
> +                                          offset, buf, n, NULL) < 0) {
>                      ret = -EIO;
>                      break;
>                  }
> @@ -705,9 +703,9 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
>          }
>          ret = 0;
> 
> -        nb_sectors -= n;
> -        sector_num += n;
> -        buf += n * 512;
> +        bytes -= n;
> +        offset += n;
> +        buf += n;
>      }
> 
>      qemu_co_mutex_unlock(&s->lock);
> -- 
> 2.14.3
> 
> 

Reviewed-by: Jeff Cody <jcody@redhat.com>