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 <andrey.shinkev...@virtuozzo.com> >>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >>> >>> 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
