Re: [Qemu-devel] [PATCH v6 14/18] qcow2: Switch qcow2_measure() to byte-based iteration
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-devel] [PATCH v6 14/18] qcow2: Switch qcow2_measure() to byte-based iteration
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-devel] [PATCH v6 14/18] qcow2: Switch qcow2_measure() to byte-based iteration
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, &pnum, &file); if (ret < 0) { error_setg_errno(&local_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