[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v4 2/7] qcow2: Discard/zero clusters by byte cou
Re: [Qemu-block] [PATCH v4 2/7] qcow2: Discard/zero clusters by byte count
Thu, 9 Feb 2017 15:05:14 -0600
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0
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
Description: OpenPGP digital signature
|[Prev in Thread]
||[Next in Thread]|
- Re: [Qemu-block] [PATCH v4 2/7] qcow2: Discard/zero clusters by byte count,
Eric Blake <=