Message ID | 20171004020048.26379-15-eblake@redhat.com |
---|---|
State | New |
Headers | show |
Series | make bdrv_get_block_status byte-based | expand |
On 10/03/2017 09:00 PM, Eric Blake wrote: > Compare the following images with all-zero contents: > $ truncate --size 1M A > $ qemu-img create -f qcow2 -o preallocation=off B 1G > $ qemu-img create -f qcow2 -o preallocation=metadata C 1G > > Signed-off-by: Eric Blake <eblake@redhat.com> > Reviewed-by: John Snow <jsnow@redhat.com> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > > +++ b/qemu-img.c > @@ -1481,11 +1481,11 @@ static int img_compare(int argc, char **argv) > while (sector_num < progress_base) { > int64_t count; > > - ret = bdrv_is_allocated_above(blk_bs(blk_over), NULL, > + ret = bdrv_block_status_above(blk_bs(blk_over), NULL, Just now noticing: in this function, ret is 32-bit, but bdrv_block_status_above() returns 64-bit values... > sector_num * BDRV_SECTOR_SIZE, > (progress_base - sector_num) * > BDRV_SECTOR_SIZE, > - &count); > + &count, NULL); > if (ret < 0) { ...which could make for a false positive in a static checker (none of our implementations return a negative value beyond INT_MIN for wrapping to be a serious concern). So that's yet another reason why I am liking Kevin's proposal to split the returned offset to be a by-reference parameter rather than squashed into the return type, as it will let me use a 32-bit return type and avoid worrying about this corner case.
diff --git a/qemu-img.c b/qemu-img.c index abd289c0b5..43e3038894 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1481,11 +1481,11 @@ static int img_compare(int argc, char **argv) while (sector_num < progress_base) { int64_t count; - ret = bdrv_is_allocated_above(blk_bs(blk_over), NULL, + ret = bdrv_block_status_above(blk_bs(blk_over), NULL, sector_num * BDRV_SECTOR_SIZE, (progress_base - sector_num) * BDRV_SECTOR_SIZE, - &count); + &count, NULL); if (ret < 0) { ret = 3; error_report("Sector allocation test failed for %s", @@ -1493,11 +1493,11 @@ static int img_compare(int argc, char **argv) goto out; } - /* TODO relax this once bdrv_is_allocated_above does not enforce + /* TODO relax this once bdrv_block_status_above does not enforce * sector alignment */ assert(QEMU_IS_ALIGNED(count, BDRV_SECTOR_SIZE)); nb_sectors = count >> BDRV_SECTOR_BITS; - if (ret) { + if (ret & BDRV_BLOCK_ALLOCATED && !(ret & BDRV_BLOCK_ZERO)) { nb_sectors = MIN(nb_sectors, IO_BUF_SIZE >> BDRV_SECTOR_BITS); ret = check_empty_sectors(blk_over, sector_num, nb_sectors, filename_over, buf1, quiet);