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 <ebl...@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 <jc...@redhat.com>
