Re: [Qemu-block] [PATCH] Discard old bitmap directories in QCOW2 image

[Max Reitz]

On 27.02.19 14:16, Denis Lunev wrote:
> On 2/27/19 4:00 PM, Max Reitz wrote:
>> On 18.02.19 16:36, Vladimir Sementsov-Ogievskiy wrote:
>>> 12.02.2019 15:35, Andrey Shinkevich wrote:
>>>> Clean QCOW2 image from bitmap obsolete directory when a new one
>>>> is allocated and stored. It slows down the image growth a little bit.
>>>> The flag QCOW2_DISCARD_ALWAYS allows a call to raw_co_pdiscard()
>>>> that does the actual cleaning of the image on disk.
>>>> With the flag QCOW2_DISCARD_OTHER, a reference count of the cluster
>>>> is updated only.
>>>>
>>>> Signed-off-by: Andrey Shinkevich <address@hidden>
>>> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>>
>>> side question: should not we change 
>>> discard_passthrough[QCOW2_DISCARD_OTHER] to
>>> true or at least flags&BDRV_O_UNMAP by default? What is the reason of not 
>>> discarding
>>> things in qcow2-cluster?
>> As far as I remember the reason is that whenever you clean up something
>> its cluster is probably going to be reused rather soon.  So cleaning up
>> takes longer, repopulating that cluster takes longer, and you save only
>> rather little space.
>>
>> This is also why I don't know whether this patch makes much sense.
>>
>> Max
>>
> This depands upon the amount of actually free clusters in the image.
> If there a lot of them at the moment, this one could be refilled
> any time later on.

Well, it's a trade-off.  For small structures like the bitmap directory,
I don't see the point in forcefully unmapping them.  We don't do it for
the old L1 table either when that has to be resized.

Discarding a whole bitmap (like in clear_bitmap_table()) is something
else, though.  So I was too quick to dismiss the whole patch; parts of
it do make sense.

For bitmap tables...  I don't know exactly.  I don't think a bitmap
table is ever going to be larger than the L1 table, so it should
probably be handled the same way -- which is to keep QCOW2_DISCARD_OTHER.

Max

signature.asc
Description: OpenPGP digital signature