Am 25.04.2018 um 20:32 hat Eric Blake geschrieben:
> 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 repurposing index_in_cluster and n
> to be 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>
> ---
> block/qcow.c | 39 +++++++++++++++++++--------------------
> 1 file changed, 19 insertions(+), 20 deletions(-)
>
> diff --git a/block/qcow.c b/block/qcow.c
> index 32730a8dd91..bf9d80fd227 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -623,6 +623,8 @@ static coroutine_fn int qcow_co_readv(BlockDriverState
> *bs, int64_t sector_num,
> QEMUIOVector hd_qiov;
> uint8_t *buf;
> void *orig_buf;
> + int64_t offset = sector_num << BDRV_SECTOR_BITS;
> + int64_t bytes = nb_sectors << BDRV_SECTOR_BITS;
This should be okay because nb_sectors is limited to INT_MAX / 512, but
I wouldn't be surprised if Coverity complained about a possible
truncation here. Multiplying with BDRV_SECTOR_SIZE instead wouldn't be
any less readable and would avoid the problem.
> if (qiov->niov > 1) {
> buf = orig_buf = qemu_try_blockalign(bs, qiov->size);
> @@ -636,36 +638,36 @@ 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,
> + ret = get_cluster_offset(bs, offset,
> 0, 0, 0, 0, &cluster_offset);
This surely fits in a single line now?
> 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;
> + index_in_cluster = offset & (s->cluster_size - 1);
> + n = s->cluster_size - index_in_cluster;
> + if (n > bytes) {
> + n = bytes;
> }
"index" suggests that it refers to an object larger than a byte. qcow2
renamed the variable to offset_in_cluster when the same change was made.
A new name for a unit change would also make review a bit easier.
The logic looks correct, though.
Kevin
