Re: [Qemu-devel] [PATCH v4 2/7] qcow2: Discard/zero clusters by byte count

[Eric Blake]

On 01/31/2017 07:54 PM, Max Reitz wrote:

>>> Another thing: I'm not quite positive whether qcow2_cluster_zeroize()
>>> can actually handle a byte count greater than INT_MAX. If you could pass
>>> such a number to it, the multiplication "ret * s->cluster_size" might
>>> overflow. It's only caller qcow2_co_pwrite_zeroes() will never exceed
>>> that limit, but maybe this should be made clear somewhere -- either by
>>> making the parameter an int instead of a uint64_t or by asserting it in
>>> the function.
>>
>> I'd lean towards an assertion, especially since it would be nice to
>> someday unify all the internal block interfaces to get to a 64-bit
>> interface wherever possible
> 
> Kevin once said "We should write code that makes sense today, not code
> that will make sense some day in the future." Or something like that. :-)

Hmm. I looked a bit further.  The calculation of "ret * s->cluster_size"
is bounded by the maximum value of the earlier "ret = zero_single_l2()",
which is limited to the number of clusters in a single L2 table.  But
when you have an image with 2M clusters, a single L2 page thus covers
2M/8 (or 262144) clusters, but that is in turn much larger than INT_MAX.
 My v5 will fix things to track the int return value separately from the
64-bit crawl through zero_single_l2() (in other words, quit overloading
'ret' to serve two purposes).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature