Re: Bug? qemu-img convert to preallocated image makes it sparse
On Thursday, 2020-01-16 at 15:37:22 +01, Max Reitz wrote: > So I suppose the best idea I can come up with is indeed a > --target-is-zero flag for qemu-img convert -n. Would that work for you? I was looking at adding this for a slightly different reason (converting sparse images to newly provisioned LUNs). Followup is a naive patch (I'm new to hacking on qemu and the block layer seems complex due to maturity) that works in my tests. Feedback much appreciated. The patch specifically targets the initial blanking of the image rather than any other attempt to write zeroes, as I couldn't convince myself that there was no control flow where qemu-img convert might need to overwrite (with zeroes) data that it itself had previously written. dme. -- Stop the music and go home.
Re: Bug? qemu-img convert to preallocated image makes it sparse
On 16.01.20 17:00, Richard W.M. Jones wrote: > On Thu, Jan 16, 2020 at 05:38:03PM +0200, Maxim Levitsky wrote: >> How about doing write zeros without discard only in this particular case >> (convert to existing image) >> Basically omitting the BDRV_REQ_MAY_UNMAP flag to blk_co_pwrite_zeroes. >> It will be slow, but maybe for this particular case, it is acceptable? > > I should probably say that we don't want to break the other case > (which is likely more important) where we write a sparse source to a > sparse target and want the target to contain only the union of the two > sparse maps, not fully allocated :-) > > It would be fine, I think, to have a new "make this disk fully > allocated" operation. qemu-img resize could almost do it with a > request to add 0 extra bytes, but the --preallocation flag only > applies to the new space. Well, that works with -S 0, as Kevin has said. But that will take time because it will write actual zeroes wherever the source is zero. Max signature.asc Description: OpenPGP digital signature
Re: Bug? qemu-img convert to preallocated image makes it sparse
On Thu, Jan 16, 2020 at 05:38:03PM +0200, Maxim Levitsky wrote: > How about doing write zeros without discard only in this particular case > (convert to existing image) > Basically omitting the BDRV_REQ_MAY_UNMAP flag to blk_co_pwrite_zeroes. > It will be slow, but maybe for this particular case, it is acceptable? I should probably say that we don't want to break the other case (which is likely more important) where we write a sparse source to a sparse target and want the target to contain only the union of the two sparse maps, not fully allocated :-) It would be fine, I think, to have a new "make this disk fully allocated" operation. qemu-img resize could almost do it with a request to add 0 extra bytes, but the --preallocation flag only applies to the new space. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Re: Bug? qemu-img convert to preallocated image makes it sparse
On 16.01.20 16:38, Maxim Levitsky wrote: > On Thu, 2020-01-16 at 15:55 +0100, Max Reitz wrote: >> On 16.01.20 15:50, Kevin Wolf wrote: >>> Am 16.01.2020 um 15:37 hat Max Reitz geschrieben: 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 > 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 > Message-id: 20190724171239.8764-2-mre...@redhat.com > Reviewed-by: Maxim Levitsky > Reviewed-by: Stefano Garzarella > Signed-off-by: Max Reitz > > qemu-img.c | 11 --- > 1 file changed, 8 insertions(+), 3 deletions(-) > > Reverting this commit on the current master branch restores the > expected behaviour. The commit changed the behavior because now qemu-img realizes that it cannot skip writing to areas that are supposed to be zero when it converts to an existing image (because we have no idea what data that existing image contains). So that’s a bug fix, and I don’t think we can undo it without being wrong. The problem is that qemu-img will try to be quickthat about making these areas zero, and so it does zero writes (actually, it even zeroes the whole image) and in the process it will of course discard all preallocation. Now, about fixing the problem I’m not so sure. >>> >>> Wouldn't just passing -S 0 solve the problem? It should tell qemu-img >>> convert that you don't want it to sparsify anything. >> >> But it would also convert the falloc preallocation to a full one. >> >> (I had a section to this effect in my first draft, but then I >> accidentally deleted it and forgot it in my second version...) >> >> Max >> > > How about doing write zeros without discard only in this particular case > (convert to existing image) > Basically omitting the BDRV_REQ_MAY_UNMAP flag to blk_co_pwrite_zeroes. > It will be slow, but maybe for this particular case, it is acceptable? I don’t think so, because AFAIA one relatively common case is to convert to an existing block device, and we want to zero that quickly. Or if the target image actually stores some data, I can imagine that people actually want a discarding unmap so the image doesn’t use more space than necessary. (Also, if we resolve this without any new kind of flag, I’m not sure whether we can guarantee to keep the desired behavior in the future, because maybe something else will force us to forego existing preallocation unless we know that the user really wants to keep it. I think there are just different use cases that convert -n is used for and it makes sense to add a flag to distinguish between them.) Max signature.asc Description: OpenPGP digital signature
Re: Bug? qemu-img convert to preallocated image makes it sparse
On Thu, 2020-01-16 at 15:55 +0100, Max Reitz wrote: > On 16.01.20 15:50, Kevin Wolf wrote: > > Am 16.01.2020 um 15:37 hat Max Reitz geschrieben: > > > 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 > > > > 51prealloc.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 > > > > 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 > > > > Message-id: 20190724171239.8764-2-mre...@redhat.com > > > > Reviewed-by: Maxim Levitsky > > > > Reviewed-by: Stefano Garzarella > > > > Signed-off-by: Max Reitz > > > > > > > > qemu-img.c | 11 --- > > > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > > > > > Reverting this commit on the current master branch restores the > > > > expected behaviour. > > > > > > The commit changed the behavior because now qemu-img realizes that it > > > cannot skip writing to areas that are supposed to be zero when it > > > converts to an existing image (because we have no idea what data that > > > existing image contains). So that’s a bug fix, and I don’t think we can > > > undo it without being wrong. > > > > > > The problem is that qemu-img will try to be quickthat about making these > > > areas zero, and so it does zero writes (actually, it even zeroes the > > > whole image) and in the process it will of course discard all > > > preallocation. > > > > > > Now, about fixing the problem I’m not so sure. > > > > Wouldn't just passing -S 0 solve the problem? It should tell qemu-img > > convert that you don't want it to sparsify anything. > > But it would also convert the falloc preallocation to a full one. > > (I had a section to this effect in my first draft, but then I > accidentally deleted it and forgot it in my second version...) > > Max > How about doing write zeros without discard only in this particular case (convert to existing image) Basically omitting the BDRV_REQ_MAY_UNMAP flag to blk_co_pwrite_zeroes. It will be slow, but maybe for this particular case, it is acceptable? Best regards, Maxim Levitsky
Re: Bug? qemu-img convert to preallocated image makes it sparse
On 16.01.20 15:57, Eric Blake wrote: > On 1/16/20 8:47 AM, Max Reitz wrote: > >> 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. > > Can't we also use block status queries on the destination, as that is > likely to be faster than actual reads and comparison to zero? It might work for falloc preallocation, but I don’t know whether we are really guaranteed that fallocated() areas return holes through lseek(). It won’t work for full preallocation, though. Yes, you might get around that with -S 0 then, but I think overall the simplest thing would be a --target-is-zero flag. (I don’t know, it seems useful to me in general. For example, when you convert to a new block device. The biggest drawback I see is that people might use it blindly and then complain that their image contains garbage...) Max signature.asc Description: OpenPGP digital signature
Re: Bug? qemu-img convert to preallocated image makes it sparse
On 1/16/20 8:47 AM, Max Reitz wrote: 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. Can't we also use block status queries on the destination, as that is likely to be faster than actual reads and comparison to zero? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: Bug? qemu-img convert to preallocated image makes it sparse
On 16.01.20 15:50, Kevin Wolf wrote: > Am 16.01.2020 um 15:37 hat Max Reitz geschrieben: >> 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 >>> 51prealloc.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 >>> 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 >>> Message-id: 20190724171239.8764-2-mre...@redhat.com >>> Reviewed-by: Maxim Levitsky >>> Reviewed-by: Stefano Garzarella >>> Signed-off-by: Max Reitz >>> >>> qemu-img.c | 11 --- >>> 1 file changed, 8 insertions(+), 3 deletions(-) >>> >>> Reverting this commit on the current master branch restores the >>> expected behaviour. >> >> The commit changed the behavior because now qemu-img realizes that it >> cannot skip writing to areas that are supposed to be zero when it >> converts to an existing image (because we have no idea what data that >> existing image contains). So that’s a bug fix, and I don’t think we can >> undo it without being wrong. >> >> The problem is that qemu-img will try to be quickthat about making these >> areas zero, and so it does zero writes (actually, it even zeroes the >> whole image) and in the process it will of course discard all preallocation. >> >> Now, about fixing the problem I’m not so sure. > > Wouldn't just passing -S 0 solve the problem? It should tell qemu-img > convert that you don't want it to sparsify anything. But it would also convert the falloc preallocation to a full one. (I had a section to this effect in my first draft, but then I accidentally deleted it and forgot it in my second version...) Max signature.asc Description: OpenPGP digital signature
Re: Bug? qemu-img convert to preallocated image makes it sparse
On Thu, Jan 16, 2020 at 03:47:30PM +0100, Max Reitz wrote: > 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 > > 51prealloc.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 > > 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 > > Message-id: 20190724171239.8764-2-mre...@redhat.com > > Reviewed-by: Maxim Levitsky > > Reviewed-by: Stefano Garzarella > > Signed-off-by: Max Reitz > > > > 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. >From the fixing virt-v2v point of view, it's a bit tricky since the code has to deal with all kinds of output targets. (For example we sometimes qemu-img convert into an NBD target.) However we do know when the target contains zeroes - in fact it always contains zeroes, so: > 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.) this one should work. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Re: Bug? qemu-img convert to preallocated image makes it sparse
Am 16.01.2020 um 15:37 hat Max Reitz geschrieben: > 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 > > 51prealloc.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 > > 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 > > Message-id: 20190724171239.8764-2-mre...@redhat.com > > Reviewed-by: Maxim Levitsky > > Reviewed-by: Stefano Garzarella > > Signed-off-by: Max Reitz > > > > qemu-img.c | 11 --- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > Reverting this commit on the current master branch restores the > > expected behaviour. > > The commit changed the behavior because now qemu-img realizes that it > cannot skip writing to areas that are supposed to be zero when it > converts to an existing image (because we have no idea what data that > existing image contains). So that’s a bug fix, and I don’t think we can > undo it without being wrong. > > The problem is that qemu-img will try to be quickthat about making these > areas zero, and so it does zero writes (actually, it even zeroes the > whole image) and in the process it will of course discard all preallocation. > > Now, about fixing the problem I’m not so sure. Wouldn't just passing -S 0 solve the problem? It should tell qemu-img convert that you don't want it to sparsify anything. Kevin signature.asc Description: PGP signature
Re: Bug? qemu-img convert to preallocated image makes it sparse
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 > 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 > Message-id: 20190724171239.8764-2-mre...@redhat.com > Reviewed-by: Maxim Levitsky > Reviewed-by: Stefano Garzarella > Signed-off-by: Max Reitz > > 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 signature.asc Description: OpenPGP digital signature
Re: Bug? qemu-img convert to preallocated image makes it sparse
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 > 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 > Message-id: 20190724171239.8764-2-mre...@redhat.com > Reviewed-by: Maxim Levitsky > Reviewed-by: Stefano Garzarella > Signed-off-by: Max Reitz > > qemu-img.c | 11 --- > 1 file changed, 8 insertions(+), 3 deletions(-) > > Reverting this commit on the current master branch restores the > expected behaviour. The commit changed the behavior because now qemu-img realizes that it cannot skip writing to areas that are supposed to be zero when it converts to an existing image (because we have no idea what data that existing image contains). So that’s a bug fix, and I don’t think we can undo it without being wrong. The problem is that qemu-img will try to be quickthat about making these areas zero, and so it does zero writes (actually, it even zeroes the whole image) and in the process it will of course discard all preallocation. Now, about fixing the problem I’m not so sure. The problem is that it isn’t easy for qemu-img or the qcow2 driver to determine whether a preallocated area is zero. It needs to read the data from disk and compare it to zero. That would be slow and not trivial. So off top of my head, the only thing that comes to my mind would be a new flag for convert that lets you guarantee the image is zero and qemu doesn’t need to zero it. The problem with this is that I don’t think we ever guaranteed that preallocated images stay preallocated when written to, and so even if we assume the image is fully zero and thus restore the old behavior for your case, we might break it in the future again. So are there any ways to safely convert an image to an existing one and keeping the destinations preallocation intact? Sadly, I don’t think there is. Well, you can always use -S 0, but that will change your falloc preallocation into a full one. So I suppose the best idea I can come up with is indeed a --target-is-zero flag for qemu-img convert -n. Would that work for you? Max signature.asc Description: OpenPGP digital signature
Bug? qemu-img convert to preallocated image makes it sparse
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 51prealloc.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 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 Message-id: 20190724171239.8764-2-mre...@redhat.com Reviewed-by: Maxim Levitsky Reviewed-by: Stefano Garzarella Signed-off-by: Max Reitz qemu-img.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) Reverting this commit on the current master branch restores the expected behaviour. Thoughts? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org