Re: [Qemu-block] [PATCH v6 14/18] qcow2: Switch qcow2_measure() to byte-based iteration

[Eric Blake]

On 09/08/2017 08:15 AM, Kevin Wolf wrote:
> Am 30.08.2017 um 23:05 hat Eric Blake geschrieben:
>> This is new code, but it is easier to read if it makes passes over
>> the image using bytes rather than sectors (and will get easier in
>> the future when bdrv_get_block_status is converted to byte-based).
>>
>> Signed-off-by: Eric Blake 
>> Reviewed-by: John Snow 
>>

>> -int nb_sectors = MIN(ssize / BDRV_SECTOR_SIZE - sector_num,
>> - BDRV_REQUEST_MAX_SECTORS);
>> +for (offset = 0; offset < ssize;
>> + offset += pnum * BDRV_SECTOR_SIZE) {
>> +int nb_sectors = MIN(ssize - offset,
>> + INT_MAX) / BDRV_SECTOR_SIZE;
> 
> Shouldn't this be BDRV_REQUEST_MAX_BYTES? (Which is close to INT_MAX,
> but rounded down to sector alignment.)

The division rounds down to sector alignment after the MIN(), for the
same result in nb_sectors either way.  But you are correct that an
absolutely literal translation of the pre-patch version would feed
BDRV_REQUEST_MAX_BYTES instead of INT_MAX into the MIN().

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH v6 14/18] qcow2: Switch qcow2_measure() to byte-based iteration

[Kevin Wolf]

Am 30.08.2017 um 23:05 hat Eric Blake geschrieben:
> This is new code, but it is easier to read if it makes passes over
> the image using bytes rather than sectors (and will get easier in
> the future when bdrv_get_block_status is converted to byte-based).
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: John Snow 
> 
> ---
> v6: separate bug fix to earlier patch
> v5: new patch
> ---
>  block/qcow2.c | 22 ++
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 40ba26c111..57e3c5e7d5 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3666,20 +3666,19 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts 
> *opts, BlockDriverState *in_bs,
>   */
>  required = virtual_size;
>  } else {
> -int cluster_sectors = cluster_size / BDRV_SECTOR_SIZE;
> -int64_t sector_num;
> +int64_t offset;
>  int pnum = 0;
> 
> -for (sector_num = 0;
> - sector_num < ssize / BDRV_SECTOR_SIZE;
> - sector_num += pnum) {
> -int nb_sectors = MIN(ssize / BDRV_SECTOR_SIZE - sector_num,
> - BDRV_REQUEST_MAX_SECTORS);
> +for (offset = 0; offset < ssize;
> + offset += pnum * BDRV_SECTOR_SIZE) {
> +int nb_sectors = MIN(ssize - offset,
> + INT_MAX) / BDRV_SECTOR_SIZE;

Shouldn't this be BDRV_REQUEST_MAX_BYTES? (Which is close to INT_MAX,
but rounded down to sector alignment.)

Kevin

[Qemu-block] [PATCH v6 14/18] qcow2: Switch qcow2_measure() to byte-based iteration

[Eric Blake]

This is new code, but it is easier to read if it makes passes over
the image using bytes rather than sectors (and will get easier in
the future when bdrv_get_block_status is converted to byte-based).

Signed-off-by: Eric Blake 
Reviewed-by: John Snow 

---
v6: separate bug fix to earlier patch
v5: new patch
---
 block/qcow2.c | 22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 40ba26c111..57e3c5e7d5 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3666,20 +3666,19 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, 
BlockDriverState *in_bs,
  */
 required = virtual_size;
 } else {
-int cluster_sectors = cluster_size / BDRV_SECTOR_SIZE;
-int64_t sector_num;
+int64_t offset;
 int pnum = 0;

-for (sector_num = 0;
- sector_num < ssize / BDRV_SECTOR_SIZE;
- sector_num += pnum) {
-int nb_sectors = MIN(ssize / BDRV_SECTOR_SIZE - sector_num,
- BDRV_REQUEST_MAX_SECTORS);
+for (offset = 0; offset < ssize;
+ offset += pnum * BDRV_SECTOR_SIZE) {
+int nb_sectors = MIN(ssize - offset,
+ INT_MAX) / BDRV_SECTOR_SIZE;
 BlockDriverState *file;
 int64_t ret;

 ret = bdrv_get_block_status_above(in_bs, NULL,
-  sector_num, nb_sectors,
+  offset >> BDRV_SECTOR_BITS,
+  nb_sectors,
   , );
 if (ret < 0) {
 error_setg_errno(_err, -ret,
@@ -3692,12 +3691,11 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, 
BlockDriverState *in_bs,
 } else if ((ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) ==
(BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) {
 /* Extend pnum to end of cluster for next iteration */
-pnum = ROUND_UP(sector_num + pnum, cluster_sectors) -
-   sector_num;
+pnum = (ROUND_UP(offset + pnum * BDRV_SECTOR_SIZE,
+ cluster_size) - offset) >> BDRV_SECTOR_BITS;

 /* Count clusters we've seen */
-required += (sector_num % cluster_sectors + pnum) *
-BDRV_SECTOR_SIZE;
+required += offset % cluster_size + pnum * 
BDRV_SECTOR_SIZE;
 }
 }
 }
-- 
2.13.5