Re: Bug? qemu-img convert to preallocated image makes it sparse

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3970 bytes --]

On 16.01.20 15:13, Richard W.M. Jones wrote:
> I'm not necessarily saying this is a bug, but a change in behaviour in
> qemu has caused virt-v2v to fail.  The reproducer is quite simple.
> 
> Create sparse and preallocated qcow2 files of the same size:
> 
>   $ qemu-img create -f qcow2 sparse.qcow2 50M
>   Formatting 'sparse.qcow2', fmt=qcow2 size=52428800 cluster_size=65536 lazy_refcounts=off refcount_bits=16
> 
>   $ qemu-img create -f qcow2 prealloc.qcow2 50M -o preallocation=falloc,compat=1.1
>   Formatting 'prealloc.qcow2', fmt=qcow2 size=52428800 compat=1.1 cluster_size=65536 preallocation=falloc lazy_refcounts=off refcount_bits=16
> 
>   $ du -m sparse.qcow2 prealloc.qcow2 
>   1 sparse.qcow2
>   51	prealloc.qcow2
> 
> Now copy the sparse file into the preallocated file using the -n
> option so qemu-img doesn't create the target:
> 
>   $ qemu-img convert -p -n -f qcow2 -O qcow2 sparse.qcow2 prealloc.qcow2
>       (100.00/100%)
> 
> In new qemu that makes the target file sparse:
> 
>   $ du -m sparse.qcow2 prealloc.qcow2 
>   1 sparse.qcow2
>   1 prealloc.qcow2         <-- should still be 51
> 
> In old qemu the target file remained preallocated, which is what
> I and virt-v2v are expecting.
> 
> I bisected this to the following commit:
> 
> 4d7c487eac1652dfe4498fe84f32900ad461d61b is the first bad commit
> commit 4d7c487eac1652dfe4498fe84f32900ad461d61b
> Author: Max Reitz <mreitz@redhat.com>
> Date:   Wed Jul 24 19:12:29 2019 +0200
> 
>     qemu-img: Fix bdrv_has_zero_init() use in convert
>     
>     bdrv_has_zero_init() only has meaning for newly created images or image
>     areas.  If qemu-img convert did not create the image itself, it cannot
>     rely on bdrv_has_zero_init()'s result to carry any meaning.
>     
>     Signed-off-by: Max Reitz <mreitz@redhat.com>
>     Message-id: 20190724171239.8764-2-mreitz@redhat.com
>     Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
>     Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>     Signed-off-by: Max Reitz <mreitz@redhat.com>
> 
>  qemu-img.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> Reverting this commit on the current master branch restores the
> expected behaviour.

So what this commit changed was that when you take an existing image as
the destination, you can’t assume anything about its contents.  Before
this commit, we assumed it’s zero.  That’s clearly wrong, because it can
be anything.

So when you convert to the target image, you have to make sure all areas
that are zero in the source are zero in the target, too.  The way we do
that is to write zeroes to the target.  The problem is that this
operation disregards the previous preallocation and discards the
preallocated space.

As for fixing the bug...  Can we fix it in qemu(-img)?

We could try to detect whether areas that are zero in the source are
zero in the (preallocated) target image, too.  But doing so what require
reading the data from those areas and comparing it to zero.  That would
take time and it isn’t trivial.  So that’s something I’d rather avoid.

Off the top of my head, the only thing that comes to my mind would be to
add a flag to qemu-img convert with which you can let it know that you
guarantee the target image is zero.  I suppose we could document it also
to imply that given this flag, areas that are zero in the source will
then not be changed in the target image; i.e. that preallocation stays
intact in those areas.


OTOH, can it be fixed in virt-v2v?  Is there already a safe way to call
qemu-img convert -n and keeping the target’s preallocation intact?
Unfortunately, I don’t think so.  I don’t think we ever guaranteed it
would, and well, now it broke.


So would you be OK with a --target-is-zero flag?  (I think we could let
this flag guarantee that your use case works, so it should be future-safe.)

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]