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

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

[Vladimir Sementsov-Ogievskiy]

This allows effectively free in_use bitmap clusters including bitmap
table without loss of meaningful data.

Now it is possible only to free end-point clusters and zero-out (not
free) bitmap table

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---

Hi all!

Here is one small but significant addition to specification of bitmaps in qcow2.

Can we apply it just like this or I'll have to inroduce new incompatible feature flag?

If there is existing implementation of the format, it may break image, saved by
software, using extended spec. But is there are any implementations except not
finished my one?


 docs/specs/qcow2.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 80cdfd0..dd07a82 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -435,6 +435,8 @@ 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.
-- 
1.8.3.1

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

[Vladimir Sementsov-Ogievskiy]

On 29.06.2016 15:22, Vladimir Sementsov-Ogievskiy wrote:
> This allows effectively free in_use bitmap clusters including bitmap
> table without loss of meaningful data.
>
> Now it is possible only to free end-point clusters and zero-out (not
> free) bitmap table
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>
> Hi all!
>
> Here is one small but significant addition to specification of bitmaps in qcow2.
>
> Can we apply it just like this or I'll have to inroduce new incompatible feature flag?
>
> If there is existing implementation of the format, it may break image, saved by
> software, using extended spec. But is there are any implementations except not
> finished my one?
>
>
>   docs/specs/qcow2.txt | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
> index 80cdfd0..dd07a82 100644
> --- a/docs/specs/qcow2.txt
> +++ b/docs/specs/qcow2.txt
> @@ -435,6 +435,8 @@ 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.

+               bitmap_table_size must be zero if bitmap_table_size is zero.



-- 
Best regards,
Vladimir

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

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 2117 bytes --]

On 29.06.2016 15:24, Vladimir Sementsov-Ogievskiy wrote:
> On 29.06.2016 15:22, Vladimir Sementsov-Ogievskiy wrote:
>> This allows effectively free in_use bitmap clusters including bitmap
>> table without loss of meaningful data.

I'm afraid I fail to understand this sentence.

>>
>> Now it is possible only to free end-point clusters and zero-out (not
>> free) bitmap table

OK, so the idea is not having to store a bitmap table if the bitmap is
fully zeroed?

>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>
>> Hi all!
>>
>> Here is one small but significant addition to specification of bitmaps
>> in qcow2.
>>
>> Can we apply it just like this or I'll have to inroduce new
>> incompatible feature flag?

I think it should be fine without a flag.

>>
>> If there is existing implementation of the format, it may break image,
>> saved by
>> software, using extended spec. But is there are any implementations
>> except not
>> finished my one?
>>
>>
>>   docs/specs/qcow2.txt | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
>> index 80cdfd0..dd07a82 100644
>> --- a/docs/specs/qcow2.txt
>> +++ b/docs/specs/qcow2.txt
>> @@ -435,6 +435,8 @@ 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.
> 
> +               bitmap_table_size must be zero if bitmap_table_size is
> zero.

The second bitmap_table_size should probably be bitmap_table_offset.

The idea looks good to me, but I'd rather understand the commit message
before giving an R-b. :-)

Max


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

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

[John Snow]

On 06/29/2016 08:22 AM, Vladimir Sementsov-Ogievskiy wrote:
> This allows effectively free in_use bitmap clusters including bitmap
> table without loss of meaningful data.
> 
> Now it is possible only to free end-point clusters and zero-out (not
> free) bitmap table
> 

Same comment as Max, the commit message needs to be a little better.

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> 
> Hi all!
> 
> Here is one small but significant addition to specification of bitmaps in qcow2.
> 
> Can we apply it just like this or I'll have to inroduce new incompatible feature flag?
> 
> If there is existing implementation of the format, it may break image, saved by
> software, using extended spec. But is there are any implementations except not
> finished my one?
> 

In pure, actual fact... your WIP reference implementation is almost
certainly the only implementation in existence.

I think this change is probably fine, as a zeroed entry here before
would have meant literally the qcow2 header itself which is quite
obviously wrong/invalid.

So it's probably a fairly easy extension to identify
dynamically/on-the-fly and won't collide with prior versions.

> 
>  docs/specs/qcow2.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
> index 80cdfd0..dd07a82 100644
> --- a/docs/specs/qcow2.txt
> +++ b/docs/specs/qcow2.txt
> @@ -435,6 +435,8 @@ 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.
>