On 29/09/2016 14:50, Paolo Bonzini wrote:
>
>
> On 29/09/2016 14:48, Kevin Wolf wrote:
>> Am 29.09.2016 um 14:14 hat Paolo Bonzini geschrieben:
>>> On 29/09/2016 12:39, Kevin Wolf wrote:
>>>>>> Because there is another check of pass-discard-request value in
>>>>>> update_refcount:
>>>>>>
>>>>>> if (refcount == 0 && s->discard_passthrough[type]) {
>>>>>> update_refcount_discard(bs, cluster_offset, s->cluster_size);
>>>>>> }
>>>> What I mean is that in the second case, you're still uselessly
>>>> deallocating the cluster on the qcow2 level while you can't reclaim it
>>>> on the filesystem level. So it would be better to leave it allocated in
>>>> qcow2, too, so that you don't get an expensive reallocation the next
>>>> time you write to it.
>>>
>>> But if you do a qemu-img convert, the deallocated cluster wouldn't be in
>>> the destination.
>>
>> Right. I still think that there has to be an option to keep the image
>> fully allocated. Perhaps what we really need to check is BDRV_O_UNMAP.
>
> Duh, of course it is.
... and it's handled in bdrv_co_pwrite_zeroes, so Fam's patch should be
okay:
if (!(child->bs->open_flags & BDRV_O_UNMAP)) {
flags &= ~BDRV_REQ_MAY_UNMAP;
}
return bdrv_co_pwritev(child, offset, count, NULL,
BDRV_REQ_ZERO_WRITE | flags);
Paolo
