On 05/22/2017 10:14 PM, Eric Blake wrote:
On 05/22/2017 02:12 PM, Eric Blake wrote:
+++ b/block/qcow2.c
@@ -2482,7 +2482,7 @@ static bool is_zero_sectors(BlockDriverState *bs, int64_t
start,
int64_t res;
if (start + count > bs->total_sectors) {
- count = bs->total_sectors - start;
+ count = start < bs->total_sectors ? bs->total_sectors - start : 0;
}
if (!count) {
@@ -2490,7 +2490,9 @@ static bool is_zero_sectors(BlockDriverState *bs, int64_t
start,
}
res = bdrv_get_block_status_above(bs, NULL, start, count,
&nr, &file);
- return res >= 0 && (res & BDRV_BLOCK_ZERO) && nr == count;
+ return res >= 0
+ && (((res & BDRV_BLOCK_ZERO) && nr == count)
+ || nr == 0);
The logic makes sense to me, although the formatting is unusual (we tend
to split && and || with the operator at the tail of the previous line,
not the head of the new line).
This quick check may make me revisit whether it is worth my my RFC
series about adding BDRV_BLOCK_EOF for more quickly tracking reads
beyond EOF (my solution in that series was able to make the same change
to test 154, but by doing it at the block layer instead of the qcow2.c
code). https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg01127.html
Actually, re-reading my RFC - I was able to change 6 instances in test
154, while your tweak only affected 2 instances (you still left four
instances that were allocating). So my approach may still be more
optimal in the long run.
Yes, looks like your approach is more general; let's drop this one then
/Anton
