Re: [Qemu-devel] [PATCH v2 2/3] qcow2: Implement bdrv_amend_options

[Kevin Wolf]

Am 29.08.2013 um 14:52 hat Max Reitz geschrieben:
> Am 29.08.2013 14:45, schrieb Eric Blake:
> >On 08/29/2013 05:20 AM, Max Reitz wrote:
> >>Implement bdrv_amend_options for compat, size, backing_file, backing_fmt
> >>and lazy_refcounts.
> >>
> >>Downgrading images from compat=1.1 to compat=0.10 is achieved through
> >>handling all incompatible flags accordingly, clearing all compatible and
> >>autoclear flags and expanding all zero clusters.
> >>
> >>Signed-off-by: Max Reitz <address@hidden>
> >>---
> >>+/*
> >>+ * Expands all zero clusters on the image; important for downgrading to a 
> >>qcow2
> >>+ * version which doesn't yet support metadata zero clusters.
> >Do we have to fully write 0 blocks into the image no matter what, or are
> >there cases where, when the file has a backing image and we know the
> >backing image has 0 bytes at the same offset, where we could flatten by
> >removing the cluster and letting the reference defer to the backing
> >file?  It's always safer to write 0 blocks into this image, but it may
> >be worth considering whether we need the (ability) to try the alternate
> >method as it results in a smaller file and potentially faster conversion.
> This seems non-trivial to optimize to me (at least doing that check
> fast), at least too non-trivial for implementing it solely for an
> image version downgrade (which nobody who is concerned about image
> size should do anyway, imho).
> 
> For non-backed images however, we could certainly just unallocate
> the blocks, I guess, since the spec explicitly states for that case
> that "if a cluster is unallocated, read requests […] shall read
> zeros for all parts that are not covered by the backing file" (also
> applies if there is no backing file at all).

Yup, checking for !bs->backing_hd is easy, so simple deallocating in
this case sounds like a good idea to do.

Reading from the backing file and checking if the buffer is zero isn't
_that_ complicated either, but at least the conversion speed won't be
improved by doing this. If we already had Paolo'sbdrv_get_block_status,
we could try that, but as it is today I don't think it's worth doing
anything else here.

Downgrading an image is an unusual operation anyway.

Kevin