Re: [Qemu-block] [Qemu-devel] [PATCH v2] spec/qcow2: bitmaps: zero bitmap table offset

[John Snow]

On 06/30/2016 05:12 AM, Denis V. Lunev wrote:
> On 06/30/2016 10:34 AM, Vladimir Sementsov-Ogievskiy wrote:
>> After loading bitmap from image and setting IN_USE flag in it's header,
>> corresponding data (bitmap table and data clusters) becomes inconsistent
>> and is no longer needed. It is better to free bitmap table and
>> corresponding clusters from the image immediately after loading the
>> bitmap than free them when the bitmap is saved, or deleted or set
>> non-persistent.
>>
>> For now it is impossible to store only bitmap header without bitmap
>> table, as specification requires it. Storing zeroed bitmap table (one or
>> more clusters) is the only option to implement the behaviour similar to
>> specified above.
>>
>> The same problem is for just storing empty bitmaps.
>>
>> This patch allows storing only bitmap header for empty bitmaps.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> ---
>>
>> Additional note. Should we also allow here bitmap_table_offset = 1, like
>> in bitmap table, for the bitmap with all bits set? I am not sure that it
>> is needed at all, but just to keep the company..
>>
>>   docs/specs/qcow2.txt | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
>> index 80cdfd0..9826222 100644
>> --- a/docs/specs/qcow2.txt
>> +++ b/docs/specs/qcow2.txt
>> @@ -435,9 +435,12 @@ Structure of a bitmap directory entry:
>>                       Offset into the image file at which the bitmap
>> table
>>                       (described below) for the bitmap starts. Must be
>> aligned to
>>                       a cluster boundary.
>> +                    Zero value means that bitmap table is not
>> allocated and the
>> +                    bitmap should be considered as empty (all bits
>> are zero).
>>              8 - 11:    bitmap_table_size
>> -                    Number of entries in the bitmap table of the bitmap.
>> +                    Number of entries in the bitmap table of the
>> bitmap. It
>> +                    must be zero if bitmap_table_offset is zero.
>>             12 - 15:    flags
>>                       Bit
> NACK
> 
> no guys, we can not make this change at the moment.
> We do have QEMU available in the field which is working
> with the currently specified format.
> 
> Den
> 

But I think the new format is a /compatible/ change. Under the old spec,
I think this field is *NEVER* zero.

Am I wrong?