Re: [PATCH] qcow2: add discard-no-unref option

[Hanna Czenczek <hreitz@redhat.com>]

On 15.05.23 09:36, Jean-Louis Dupond wrote:
> When we for example have a sparse qcow2 image and discard: unmap is enabled,
> there can be a lot of fragmentation in the image after some time. Surely on VM's
> that do a lot of writes/deletes.
> This causes the qcow2 image to grow even over 110% of its virtual size,
> because the free gaps in the image get to small to allocate new
> continuous clusters. So it allocates new space as the end of the image.
>
> Disabling discard is not an option, as discard is needed to keep the
> incremental backup size as low as possible. Without discard, the
> incremental backups would become large, as qemu thinks it's just dirty
> blocks but it doesn't know the blocks are empty/useless.
> So we need to avoid fragmentation but also 'empty' the useless blocks in
> the image to have a small incremental backup.
>
> Next to that we also want to send the discards futher down the stack, so
> the underlying blocks are still discarded.
>
> Therefor we introduce a new qcow2 option "discard-no-unref". When
> setting this option to true (defaults to false), the discard requests
> will still be executed, but it will keep the offset of the cluster. And
> it will also pass the discard request further down the stack (if
> discard:unmap is enabled).
> This will avoid fragmentation and for example on a fully preallocated
> qcow2 image, this will make sure the image is perfectly continuous.

I don’t follow how this patch is cleaner than the “block: Add zeroes 
discard option” patch.  That patch’s diff stat is +14/-5, this one’s is 
120+/57-.

As for better, I don’t want to discount that, but at the same time I 
don’t know the reasoning for it.  As far as I understand, this patch 
makes qcow2 retain the cluster mapping as-is, and only discards on the 
lower layer.  So effectively, instead of marking the cluster as zero on 
the qcow2 level, we do so on the filesystem level.  I’m not sure I see 
all the implications of that difference.

The advantage I see is that this free up disk space, which the 
discard=zeroes wouldn’t do.  I wonder whether that couldn’t be solved 
with an orthogonal qcow2-only option, though, which would have the qcow2 
driver always discard zeroed clusters.

On the other hand, one thing to note is that we’ve had performance 
problems in the past with holes in the filesystem level (on tmpfs at 
least).  qemu generally can get the information on which clusters are 
zero quicker from qcow2 than from the filesystem, which suggests that 
even if we want to discard something on the filesystem layer, we 
probably also want to mark it as zero on the qcow2 layer.

I also have a small concern about fragmentation on the filesystem layer 
– if you use these options to prevent fragmentation, with this patch, 
you’re only doing so on the qcow2 layer.  Because the cluster is then 
discarded in the filesystem, it’s possible to get fragmentation there, 
which might not be desirable.  I don’t think that’s too important, but I 
think it’d just be nice to have a configuration in which the guest can 
tell qemu what areas it doesn’t care about, qemu marks these as zero, so 
that backups are more efficient; but at the same time, everything stays 
allocated, so no fragmentatoin is introduced.

Hanna