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

[Max Reitz <mreitz@redhat.com>]

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 <mreitz@redhat.com>
>> ---
>> +/*
>> + * 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).

>> +
>> +    /* the refcount order might be different in newer images - however, qemu
>> +     * doesn't support anything different than 4 anyway, so nothing to fix
>> +     * there */
> This sounds risky.  Wouldn't it be safer to error out if the image
> didn't have a refcount order of 4, than to just ignore it; on the
> grounds that if qemu DOES add support for non-4 refcount order, an error
> will at least alert someone to the fact that they need to add some
> (potentially complicated) code here?
>
Oh, yes, of course. I'll fix it.


Max