diff mbox

[v6] spec: add qcow2 bitmaps extension specification

Message ID 1450892961-6495-1-git-send-email-vsementsov@virtuozzo.com
State New
Headers show

Commit Message

Vladimir Sementsov-Ogievskiy Dec. 23, 2015, 5:49 p.m. UTC
The new feature for qcow2: storing bitmaps.

This patch adds new header extension to qcow2 - Bitmaps Extension. It
provides an ability to store virtual disk related bitmaps in a qcow2
image. For now there is only one type of such bitmaps: Dirty Tracking
Bitmap, which just tracks virtual disk changes from some moment.

Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
should be stored in this qcow2 file. The size of each bitmap
(considering its granularity) is equal to virtual disk size.

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

v6:

- reword bitmap_directory_size description
- bitmap type: make 0 reserved
- extra_data_size: resize to 4bytes
  Also, I've marked this field as "must be zero". We can always change
  it, if we decide allowing managing app to specify any extra data, by
  defining some magic value as a top of user extra data.. So, for now
  non zeor extra_data_size should be considered as an error.
- swap name and extra_data to give good alignment to extra_data.


v5:

- 'Dirty bitmaps' renamed to 'Bitmaps', as we may have several types of
  bitmaps.
- rewordings
- move upper bounds to "Notes about Qemu limits"
- s/should/must somewhere. (but not everywhere)
- move name_size field closer to name itself in bitmap header
- add extra data area to bitmap header
- move bitmap data description to separate section

 docs/specs/qcow2.txt | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 160 insertions(+), 1 deletion(-)

Comments

Max Reitz Dec. 23, 2015, 10:31 p.m. UTC | #1
On 23.12.2015 18:49, Vladimir Sementsov-Ogievskiy wrote:
> The new feature for qcow2: storing bitmaps.
> 
> This patch adds new header extension to qcow2 - Bitmaps Extension. It
> provides an ability to store virtual disk related bitmaps in a qcow2
> image. For now there is only one type of such bitmaps: Dirty Tracking
> Bitmap, which just tracks virtual disk changes from some moment.
> 
> Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
> should be stored in this qcow2 file. The size of each bitmap
> (considering its granularity) is equal to virtual disk size.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

I see some issues regarding grammar and phrasing, but I still think
nagging about that should be left to native speakers and for the non-RFC
versions.
(Or someone can just fix it up on top.)

Semantically-reviewed-by: Max Reitz <mreitz@redhat.com>

Thanks!
Eric Blake Dec. 23, 2015, 11:41 p.m. UTC | #2
On 12/23/2015 10:49 AM, Vladimir Sementsov-Ogievskiy wrote:
> The new feature for qcow2: storing bitmaps.
> 
> This patch adds new header extension to qcow2 - Bitmaps Extension. It
> provides an ability to store virtual disk related bitmaps in a qcow2
> image. For now there is only one type of such bitmaps: Dirty Tracking
> Bitmap, which just tracks virtual disk changes from some moment.
> 
> Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
> should be stored in this qcow2 file. The size of each bitmap
> (considering its granularity) is equal to virtual disk size.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> 

> @@ -166,6 +179,34 @@ the header extension data. Each entry look like this:
>                      terminated if it has full length)
>  
>  
> +== Bitmaps extension ==
> +
> +Bitmaps extension is an optional header extension. It provides an ability to
> +store virtual disk related bitmaps in a qcow2 image. For now there is only one
> +type of such bitmaps: Dirty Tracking Bitmap, which just tracks virtual disk
> +changes from some moment.

This is already the qcow2 spec, so 'in a qcow2 image' may be redundant.
 Possible idea for nicer grammar:

It provides the ability to store bitmaps related to a virtual disk.  For
now, there is only one bitmap type: Dirty Tracking Bitmap, which tracks
virtual disk changes from some moment.


> +             17:    granularity_bits
> +                    Granularity bits. Valid values are: 0 - 63.

Elsewhere, the file has 'valid values: 0-63'; dropping 'are' would make
this more consistent.

> +
> +                    Note: Qemu currently doesn't support granularity_bits
> +                    greater than 31.
> +
> +                    Granularity is calculated as
> +                        granularity = 1 << granularity_bits
> +
> +                    Granularity of the bitmap is how many bytes of the image
> +                    accounts for one bit of the bitmap.
> +
> +        18 - 19:    name_size
> +                    Size of the bitmap name. Valid values: 1 - 1023.

Should this be more like:
Must be non-zero. Note: Qemu currently doesn't support values greater
than 1023.


> +=== Bitmap Data ===
> +
> +As noted above, bitmap data is stored in several (or may be one, exactly
> +bitmap_table_size) separate clusters, described by Bitmap Table.

bitmap_table_size was documented as "Number of entries in the Bitmap
Table of the bitmap", where each entry is 8 bytes.  But this sounds like
bitmap_table_size == 1 implies that the table is exactly 1 cluster (at
least 512 bytes).  I think you are trying to imply that the bitmap data
occupies ceil(cluster size / 8 / bitmap_tablesize) clusters.

I also wonder if you need more text to cover what happens when the
number of entries does not end on a cluster boundary.  Must the
remaining bits of the cluster containing the tail of the Bitmap be set
to all 0, or is it garbage that must be ignored regardless of content?
Vladimir Sementsov-Ogievskiy Dec. 24, 2015, 10 a.m. UTC | #3
On 24.12.2015 02:41, Eric Blake wrote:
> On 12/23/2015 10:49 AM, Vladimir Sementsov-Ogievskiy wrote:
>> The new feature for qcow2: storing bitmaps.
>>
>> This patch adds new header extension to qcow2 - Bitmaps Extension. It
>> provides an ability to store virtual disk related bitmaps in a qcow2
>> image. For now there is only one type of such bitmaps: Dirty Tracking
>> Bitmap, which just tracks virtual disk changes from some moment.
>>
>> Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
>> should be stored in this qcow2 file. The size of each bitmap
>> (considering its granularity) is equal to virtual disk size.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>
>> @@ -166,6 +179,34 @@ the header extension data. Each entry look like this:
>>                       terminated if it has full length)
>>   
>>   
>> +== Bitmaps extension ==
>> +
>> +Bitmaps extension is an optional header extension. It provides an ability to
>> +store virtual disk related bitmaps in a qcow2 image. For now there is only one
>> +type of such bitmaps: Dirty Tracking Bitmap, which just tracks virtual disk
>> +changes from some moment.
> This is already the qcow2 spec, so 'in a qcow2 image' may be redundant.
>   Possible idea for nicer grammar:
>
> It provides the ability to store bitmaps related to a virtual disk.  For
> now, there is only one bitmap type: Dirty Tracking Bitmap, which tracks
> virtual disk changes from some moment.
>
>
>> +             17:    granularity_bits
>> +                    Granularity bits. Valid values are: 0 - 63.
> Elsewhere, the file has 'valid values: 0-63'; dropping 'are' would make
> this more consistent.
>
>> +
>> +                    Note: Qemu currently doesn't support granularity_bits
>> +                    greater than 31.
>> +
>> +                    Granularity is calculated as
>> +                        granularity = 1 << granularity_bits
>> +
>> +                    Granularity of the bitmap is how many bytes of the image
>> +                    accounts for one bit of the bitmap.
>> +
>> +        18 - 19:    name_size
>> +                    Size of the bitmap name. Valid values: 1 - 1023.
> Should this be more like:
> Must be non-zero. Note: Qemu currently doesn't support values greater
> than 1023.
>
>
>> +=== Bitmap Data ===
>> +
>> +As noted above, bitmap data is stored in several (or may be one, exactly
>> +bitmap_table_size) separate clusters, described by Bitmap Table.
> bitmap_table_size was documented as "Number of entries in the Bitmap
> Table of the bitmap", where each entry is 8 bytes.  But this sounds like
> bitmap_table_size == 1 implies that the table is exactly 1 cluster (at
> least 512 bytes).  I think you are trying to imply that the bitmap data
> occupies ceil(cluster size / 8 / bitmap_tablesize) clusters.

I don't understand.. No. Bitmap data occupies bitmap_table_size 
clusters. The last one may have some meaningless remaining bits. If 
bitmap_table_size = 1, than bitmap data is stored in "exactly 1" 
cluster. Bitmap table is like page table.

>
> I also wonder if you need more text to cover what happens when the
> number of entries does not end on a cluster boundary.  Must the
> remaining bits of the cluster containing the tail of the Bitmap be set
> to all 0, or is it garbage that must be ignored regardless of content?
>
Vladimir Sementsov-Ogievskiy Dec. 24, 2015, 10:10 a.m. UTC | #4
On 24.12.2015 01:31, Max Reitz wrote:
> On 23.12.2015 18:49, Vladimir Sementsov-Ogievskiy wrote:
>> The new feature for qcow2: storing bitmaps.
>>
>> This patch adds new header extension to qcow2 - Bitmaps Extension. It
>> provides an ability to store virtual disk related bitmaps in a qcow2
>> image. For now there is only one type of such bitmaps: Dirty Tracking
>> Bitmap, which just tracks virtual disk changes from some moment.
>>
>> Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
>> should be stored in this qcow2 file. The size of each bitmap
>> (considering its granularity) is equal to virtual disk size.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> I see some issues regarding grammar and phrasing, but I still think
> nagging about that should be left to native speakers and for the non-RFC
> versions.

I've already removed 'RFC' mark)

> (Or someone can just fix it up on top.)
>
> Semantically-reviewed-by: Max Reitz <mreitz@redhat.com>
>
> Thanks!
>
Max Reitz Jan. 4, 2016, 10:21 p.m. UTC | #5
On 23.12.2015 18:49, Vladimir Sementsov-Ogievskiy wrote:
> The new feature for qcow2: storing bitmaps.
> 
> This patch adds new header extension to qcow2 - Bitmaps Extension. It
> provides an ability to store virtual disk related bitmaps in a qcow2
> image. For now there is only one type of such bitmaps: Dirty Tracking
> Bitmap, which just tracks virtual disk changes from some moment.
> 
> Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
> should be stored in this qcow2 file. The size of each bitmap
> (considering its granularity) is equal to virtual disk size.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---

OK, keeping my eyes open for grammar etc.; here goes nothing:

Generally, most of the qcow2 specification does not consider structure
names to be proper nouns, that is, they are generally written starting
with lower letters (e.g. "refcount table", "L2 table", "image header").

I'd prefer this spelling for all the structures presented herein (e.g.
"bitmaps extension", "bitmap directory", "dirty tracking bitmap", ...),
too, but that is probably a personal preference.

>  docs/specs/qcow2.txt | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 160 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
> index 121dfc8..b23966a 100644
> --- a/docs/specs/qcow2.txt
> +++ b/docs/specs/qcow2.txt
> @@ -103,7 +103,19 @@ in the description of a field.
>                      write to an image with unknown auto-clear features if it
>                      clears the respective bits from this field first.
>  
> -                    Bits 0-63:  Reserved (set to 0)
> +                    Bit 0:      Bitmaps extension bit.
> +                                This bit is responsible for Bitmaps extension
> +                                consistency.
> +
> +                                If it is set, but there is no Bitmaps
> +                                extension, this should be considered as an
> +                                error.

Maybe correct (this is why a native speaker should be doing this...),
but I'd omit the "as" (i.e. just "this should be considered an error").

> +
> +                                If it is not set, but there is a Bitmaps
> +                                extension, its data should be considered as
> +                                inconsistent.

Same here.

> +
> +                    Bits 1-63:  Reserved (set to 0)
>  
>           96 -  99:  refcount_order
>                      Describes the width of a reference count block entry (width
> @@ -123,6 +135,7 @@ be stored. Each extension has a structure like the following:
>                          0x00000000 - End of the header extension area
>                          0xE2792ACA - Backing file format name
>                          0x6803f857 - Feature name table
> +                        0x23852875 - Bitmaps extension
>                          other      - Unknown header extension, can be safely
>                                       ignored
>  
> @@ -166,6 +179,34 @@ the header extension data. Each entry look like this:
>                      terminated if it has full length)
>  
>  
> +== Bitmaps extension ==
> +
> +Bitmaps extension is an optional header extension. It provides an ability to

"The Bitmaps extension..."

Also, while it may be correct as it is, "provides the ability" sounds
better to me.

> +store virtual disk related bitmaps in a qcow2 image. For now there is only one
> +type of such bitmaps: Dirty Tracking Bitmap, which just tracks virtual disk

I'd prefer "The Dirty Tracking Bitmap" or "Dirty Tracking Bitmaps".

> +changes from some moment.

I think, "moment" is rather used for a time span in English. So this
should be "point in time" instead.

(Maybe even "a certain point in time" rather than "some point in time",
but that would be a semantic change.)

> +
> +The data of the extension should be considered as consistent only if

Again, the "as" can be dropped (and maybe it should be).

Also, it should be "...only if the corresponding..." ("the" missing).

> +corresponding auto-clear feature bit is set (see autoclear_features above).
> +
> +The fields of Bitmaps extension are:

"...of the Bitmaps..."

> +
> +          0 -  3:  nb_bitmaps
> +                   The number of bitmaps contained in the image. Must be
> +                   greater or equal to 1.
> +
> +                   Note: Qemu currently only supports up to 65535 bitmaps per
> +                   image.
> +
> +          4 -  7:  bitmap_directory_size
> +                   Size of the Bitmap Directory in bytes. It is a cumulative

"...It is the cumulative size..."

> +                   size of all (nb_bitmaps) bitmap headers.
> +
> +          8 - 15:  bitmap_directory_offset
> +                   Offset into the image file at which the Bitmap Directory
> +                   starts. Must be aligned to a cluster boundary.
> +
> +
>  == Host cluster management ==
>  
>  qcow2 manages the allocation of host clusters by maintaining a reference count
> @@ -360,3 +401,121 @@ Snapshot table entry:
>  
>          variable:   Padding to round up the snapshot table entry size to the
>                      next multiple of 8.
> +
> +
> +== Bitmaps ==
> +
> +The feature supports storing bitmaps in a qcow2 image. All bitmaps are related

Maybe "This feature supports...".

> +to the virtual disk, stored in this image.

The comma should be omitted.

> +
> +=== Bitmap Directory ===
> +
> +Each bitmap saved in the image is described in a Bitmap Directory entry. Bitmap
> +Directory is a contiguous area in the image file, whose starting offset and

"... The Bitmap Directory is..."

> +length are given by the header extension fields bitmap_directory_offset and

"bitmap header extension fields" may be clearer, but that can be parsed
both as "(bitmap (header extension)) fields" and "((bitmap header)
extension) fields"; the first is what we want (the qcow2 header
extension for bitmaps), the second would be an extension for a bitmap
directory entry (which are "bitmap headers"), sooo... Probably it'll be
best leaving this as it is.

> +bitmap_directory_size. The entries of the bitmap directory have variable
> +length, depending on the length of the bitmap name and extra data. These
> +entries are also called bitmap headers.
> +
> +Bitmap Directory Entry:

"entry" should be written starting with a lower letter, and a more
verbose "Structure of a Bitmap Directory entry" may be nicer.

> +
> +    Byte 0 -  7:    bitmap_table_offset
> +                    Offset into the image file at which the Bitmap Table
> +                    (described below) for the bitmap starts. Must be aligned to
> +                    a cluster boundary.
> +
> +         8 - 11:    bitmap_table_size
> +                    Number of entries in the Bitmap Table of the bitmap.
> +
> +        12 - 15:    flags
> +                    Bit
> +                      0: in_use
> +                         The bitmap was not saved correctly and may be
> +                         inconsistent.
> +
> +                      1: auto
> +                         The bitmap must reflect all changes of the virtual
> +                         disk by any application that would write to this qcow2
> +                         file (including writes, snapshot switching, etc.). The
> +                         type of this bitmap must be 'Dirty Tracking Bitmap'.
> +
> +                    Bits 2 - 31 are reserved and must be 0.
> +
> +             16:    type
> +                    This field describes the sort of the bitmap.
> +                    Values:
> +                      1: Dirty Tracking Bitmap
> +
> +                    Values 0, 2 - 255 are reserved.
> +
> +             17:    granularity_bits
> +                    Granularity bits. Valid values are: 0 - 63.
> +
> +                    Note: Qemu currently doesn't support granularity_bits
> +                    greater than 31.
> +
> +                    Granularity is calculated as
> +                        granularity = 1 << granularity_bits
> +
> +                    Granularity of the bitmap is how many bytes of the image
> +                    accounts for one bit of the bitmap.

I'd prefer "A bitmap's granularity" or at least "The granularity of the
bitmap".

> +
> +        18 - 19:    name_size
> +                    Size of the bitmap name. Valid values: 1 - 1023.
> +
> +        20 - 23:    extra_data_size
> +                    Size of type-specific extra data.
> +
> +                    For now, as no extra data is defined, extra_data_size is
> +                    reserved and must be zero.
> +
> +        variable:   Type-specific extra data for the bitmap.
> +
> +        variable:   The name of the bitmap (not null terminated). Must be
> +                    unique among all bitmap names within the Bitmaps extension.
> +
> +        variable:   Padding to round up the Bitmap Directory Entry size to the
> +                    next multiple of 8.

"...to round up this entry's size..." would be shorter. In any case,
"Entry" should be written with a lower letter, I think (but frankly, I
think that even "Bitmap Directory" should be written in lowercase, so
make of that what you wish).

> +
> +=== Bitmap Table ===
> +
> +Bitmaps are stored using a one-level (not two-level like refcounts and guest
> +clusters mapping) structure for the mapping of bitmaps data to host clusters.

The sentence in the parantheses may be correct, but it sounds weird.
Maybe "as opposed to two-level like for refcounts and..." would be
better (but the plain "two-level" still sounds strange).

Also, probably s/bitmaps data/bitmap data/.

> +It is called Bitmap Table.

I'd prefer either "...to host clusters, which is called a Bitmap Table."
or "This structure is called a Bitmap Table.".

> +
> +Each Bitmap Table has a variable size (stored in the Bitmap Directory Entry)
> +and may use multiple clusters, however it must be contiguous in the image file.

I'd put a comma after the "however", too.

> +
> +Bitmap Table entry:

Again, a more verbose "Structure of a Bitmap Table entry" might be
nicer, but this is completely optional.

> +
> +    Bit       0:    Reserved and must be zero if bits 9 - 55 are non-zero.
> +                    If bits 9 - 55 are zero:
> +                      0: Cluster should be read as all zeros.
> +                      1: Cluster should be read as all ones.
> +
> +         1 -  8:    Reserved and must be zero.
> +
> +         9 - 55:    Bits 9 - 55 of host cluster offset. Must be aligned to a

"...of the host cluster offset."

> +                    cluster boundary. If the offset is 0, the cluster is
> +                    unallocated, see bit 0 description.

"..., the custer is unallocated; in that case, see the description
of/for bit 0.",

or:

"..., the cluster is unallocated; in that case, bit 0 determines how
this cluster should be treated when read from."

> +
> +        56 - 63:    Reserved and must be zero.
> +
> +=== Bitmap Data ===

As you never use this as a proper noun anywhere, I think it's safe to
make this a "Bitmap data".

> +
> +As noted above, bitmap data is stored in several (or may be one, exactly

s/may be/maybe/

Alternatively, the common phrasing is: "...is stored in one or more
(bitmap_table_size) separate clusters...".

> +bitmap_table_size) separate clusters, described by Bitmap Table. Given an

"..., as described by the Bitmap Table."

> +offset (in bytes) into the bitmap data, the offset into the image file can be
> +obtained as follows:
> +
> +    image_offset =
> +        bitmap_table[bitmap_data_offset / cluster_size] +
> +            (bitmap_data_offset % cluster_size)
> +
> +Taking into account the granularity of the bitmap, an offset in bits into the
> +image file, corresponding to byte number byte_nr of the virtual disk can be
> +calculated like this:

Probably the following is better:

Given an offset byte_nr into the virtual disk and the bitmap's
granularity, the bit offset into the bitmap can be calculated like this:

> +
> +    bit_offset =
> +        image_offset(byte_nr / granularity / 8) * 8 +
> +            (byte_nr / granularity) % 8
> 

Max
John Snow Jan. 4, 2016, 10:34 p.m. UTC | #6
On 12/24/2015 05:00 AM, Vladimir Sementsov-Ogievskiy wrote:
> On 24.12.2015 02:41, Eric Blake wrote:
>> On 12/23/2015 10:49 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> The new feature for qcow2: storing bitmaps.
>>>
>>> This patch adds new header extension to qcow2 - Bitmaps Extension. It
>>> provides an ability to store virtual disk related bitmaps in a qcow2
>>> image. For now there is only one type of such bitmaps: Dirty Tracking
>>> Bitmap, which just tracks virtual disk changes from some moment.
>>>
>>> Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
>>> should be stored in this qcow2 file. The size of each bitmap
>>> (considering its granularity) is equal to virtual disk size.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>
>>> @@ -166,6 +179,34 @@ the header extension data. Each entry look like
>>> this:
>>>                       terminated if it has full length)
>>>     +== Bitmaps extension ==
>>> +
>>> +Bitmaps extension is an optional header extension. It provides an
>>> ability to
>>> +store virtual disk related bitmaps in a qcow2 image. For now there
>>> is only one
>>> +type of such bitmaps: Dirty Tracking Bitmap, which just tracks
>>> virtual disk
>>> +changes from some moment.
>> This is already the qcow2 spec, so 'in a qcow2 image' may be redundant.
>>   Possible idea for nicer grammar:
>>
>> It provides the ability to store bitmaps related to a virtual disk.  For
>> now, there is only one bitmap type: Dirty Tracking Bitmap, which tracks
>> virtual disk changes from some moment.
>>
>>
>>> +             17:    granularity_bits
>>> +                    Granularity bits. Valid values are: 0 - 63.
>> Elsewhere, the file has 'valid values: 0-63'; dropping 'are' would make
>> this more consistent.
>>
>>> +
>>> +                    Note: Qemu currently doesn't support
>>> granularity_bits
>>> +                    greater than 31.
>>> +
>>> +                    Granularity is calculated as
>>> +                        granularity = 1 << granularity_bits
>>> +
>>> +                    Granularity of the bitmap is how many bytes of
>>> the image
>>> +                    accounts for one bit of the bitmap.
>>> +
>>> +        18 - 19:    name_size
>>> +                    Size of the bitmap name. Valid values: 1 - 1023.
>> Should this be more like:
>> Must be non-zero. Note: Qemu currently doesn't support values greater
>> than 1023.
>>
>>
>>> +=== Bitmap Data ===
>>> +
>>> +As noted above, bitmap data is stored in several (or may be one,
>>> exactly
>>> +bitmap_table_size) separate clusters, described by Bitmap Table.
>> bitmap_table_size was documented as "Number of entries in the Bitmap
>> Table of the bitmap", where each entry is 8 bytes.  But this sounds like
>> bitmap_table_size == 1 implies that the table is exactly 1 cluster (at
>> least 512 bytes).  I think you are trying to imply that the bitmap data
>> occupies ceil(cluster size / 8 / bitmap_tablesize) clusters.
> 
> I don't understand.. No. Bitmap data occupies bitmap_table_size
> clusters. The last one may have some meaningless remaining bits. If
> bitmap_table_size = 1, than bitmap data is stored in "exactly 1"
> cluster. Bitmap table is like page table.
> 

Eric is referring to earlier in the spec where you state:

"bitmap_table_size" "Number of entries in the Bitmap Table of the bitmap."

But later on, it appears as if "bitmap_table_size" refers to a number of
clusters:

"As noted above, bitmap data is stored in several (or may be one,
exactly bitmap_table_size) separate clusters"

Here, one may read "bitmap_table_size" to be referring to a cluster
count -- which is only indirectly true.


I think what is meant is this:

- bitmap_table_size refers to the number of bitmap table entries.
- each bitmap table entry indicates a cluster's worth of data.
- "bitmap_table_size := 0x01" implies eight bytes for the header, but an
entire cluster for data.

So "indirectly," bitmap_table_size refers to the number of clusters that
contain bitmap data, but to be accurately precise, it actually refers to
the number of bitmap table entries.

Correct?

>>
>> I also wonder if you need more text to cover what happens when the
>> number of entries does not end on a cluster boundary.  Must the
>> remaining bits of the cluster containing the tail of the Bitmap be set
>> to all 0, or is it garbage that must be ignored regardless of content?
>>
> 
>
John Snow Jan. 4, 2016, 11:16 p.m. UTC | #7
Since Max didn't offer a grammatical review, here's my attempt at some
suggestions.

On 12/23/2015 12:49 PM, Vladimir Sementsov-Ogievskiy wrote:
> The new feature for qcow2: storing bitmaps.
> 
> This patch adds new header extension to qcow2 - Bitmaps Extension. It
> provides an ability to store virtual disk related bitmaps in a qcow2
> image. For now there is only one type of such bitmaps: Dirty Tracking
> Bitmap, which just tracks virtual disk changes from some moment.
> 
> Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
> should be stored in this qcow2 file. The size of each bitmap
> (considering its granularity) is equal to virtual disk size.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> 
> v6:
> 
> - reword bitmap_directory_size description
> - bitmap type: make 0 reserved
> - extra_data_size: resize to 4bytes
>   Also, I've marked this field as "must be zero". We can always change
>   it, if we decide allowing managing app to specify any extra data, by
>   defining some magic value as a top of user extra data.. So, for now
>   non zeor extra_data_size should be considered as an error.
> - swap name and extra_data to give good alignment to extra_data.
> 
> 
> v5:
> 
> - 'Dirty bitmaps' renamed to 'Bitmaps', as we may have several types of
>   bitmaps.
> - rewordings
> - move upper bounds to "Notes about Qemu limits"
> - s/should/must somewhere. (but not everywhere)
> - move name_size field closer to name itself in bitmap header
> - add extra data area to bitmap header
> - move bitmap data description to separate section
> 
>  docs/specs/qcow2.txt | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 160 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
> index 121dfc8..b23966a 100644
> --- a/docs/specs/qcow2.txt
> +++ b/docs/specs/qcow2.txt
> @@ -103,7 +103,19 @@ in the description of a field.
>                      write to an image with unknown auto-clear features if it
>                      clears the respective bits from this field first.
>  
> -                    Bits 0-63:  Reserved (set to 0)
> +                    Bit 0:      Bitmaps extension bit.

For consistency, no period after this.

> +                                This bit is responsible for Bitmaps extension
> +                                consistency.
> +

I might phrase it as: "This bit indicates consistency for the Bitmaps
extension data."

> +                                If it is set, but there is no Bitmaps
> +                                extension, this should be considered as an
> +                                error.
> +

"This should be considered as an error" can be shortened to just "This
is an error." This makes the sentence top-heavy though, so how about:

"It is an error if this bit is set without the Bitmaps extension present."

> +                                If it is not set, but there is a Bitmaps
> +                                extension, its data should be considered as
> +                                inconsistent.
> +

Let's remove "considered" here. The data *is* inconsistent if this has
happened.

"If the Bitmaps extension is present but this bit is unset, the Bitmaps
extension data is inconsistent."

> +                    Bits 1-63:  Reserved (set to 0)
>  
>           96 -  99:  refcount_order
>                      Describes the width of a reference count block entry (width
> @@ -123,6 +135,7 @@ be stored. Each extension has a structure like the following:
>                          0x00000000 - End of the header extension area
>                          0xE2792ACA - Backing file format name
>                          0x6803f857 - Feature name table
> +                        0x23852875 - Bitmaps extension
>                          other      - Unknown header extension, can be safely
>                                       ignored
>  
> @@ -166,6 +179,34 @@ the header extension data. Each entry look like this:
>                      terminated if it has full length)
>  
>  
> +== Bitmaps extension ==
> +
> +Bitmaps extension is an optional header extension. It provides an ability to
> +store virtual disk related bitmaps in a qcow2 image. For now there is only one
> +type of such bitmaps: Dirty Tracking Bitmap, which just tracks virtual disk
> +changes from some moment.
> +

I think "Bitmaps extension" is awkward without "The" leading it, so:

"The Bitmaps extension is an optional header extension."

And from Eric's suggestion:

"It provides the ability to store bitmaps related to a virtual disk.
For now, there is only one bitmap type: Dirty Tracking Bitmap, which
tracks virtual disk changes from some moment."

> +The data of the extension should be considered as consistent only if
> +corresponding auto-clear feature bit is set (see autoclear_features above).
> +

I might remove the parenthetical.

"The data in this extension should be considered consistent only if the
corresponding auto-clear feature bit is set, see autoclear_features above."

> +The fields of Bitmaps extension are:
> +

Adding 'the' again:

"The fields of the Bitmaps extension are:"

> +          0 -  3:  nb_bitmaps
> +                   The number of bitmaps contained in the image. Must be
> +                   greater or equal to 1.
> +

"Greater than or equal to" is a common phrasing -- adding the 'than'.

> +                   Note: Qemu currently only supports up to 65535 bitmaps per
> +                   image.
> +
> +          4 -  7:  bitmap_directory_size
> +                   Size of the Bitmap Directory in bytes. It is a cumulative
> +                   size of all (nb_bitmaps) bitmap headers.
> +
> +          8 - 15:  bitmap_directory_offset
> +                   Offset into the image file at which the Bitmap Directory
> +                   starts. Must be aligned to a cluster boundary.
> +
> +
>  == Host cluster management ==
>  
>  qcow2 manages the allocation of host clusters by maintaining a reference count
> @@ -360,3 +401,121 @@ Snapshot table entry:
>  
>          variable:   Padding to round up the snapshot table entry size to the
>                      next multiple of 8.
> +
> +
> +== Bitmaps ==
> +
> +The feature supports storing bitmaps in a qcow2 image. All bitmaps are related
> +to the virtual disk, stored in this image.
> +

Maybe like Eric's earlier suggestion, we can avoid "in a qcow2 image"
here. We can probably just repeat some of the earlier description here.

> +=== Bitmap Directory ===
> +
> +Each bitmap saved in the image is described in a Bitmap Directory entry. Bitmap

"The Bitmap Directory"

> +Directory is a contiguous area in the image file, whose starting offset and
> +length are given by the header extension fields bitmap_directory_offset and
> +bitmap_directory_size. The entries of the bitmap directory have variable
> +length, depending on the length of the bitmap name and extra data. These
> +entries are also called bitmap headers.
> +
> +Bitmap Directory Entry:
> +
> +    Byte 0 -  7:    bitmap_table_offset
> +                    Offset into the image file at which the Bitmap Table
> +                    (described below) for the bitmap starts. Must be aligned to
> +                    a cluster boundary.
> +
> +         8 - 11:    bitmap_table_size
> +                    Number of entries in the Bitmap Table of the bitmap.
> +
> +        12 - 15:    flags
> +                    Bit
> +                      0: in_use
> +                         The bitmap was not saved correctly and may be
> +                         inconsistent.
> +
> +                      1: auto
> +                         The bitmap must reflect all changes of the virtual
> +                         disk by any application that would write to this qcow2
> +                         file (including writes, snapshot switching, etc.). The
> +                         type of this bitmap must be 'Dirty Tracking Bitmap'.
> +
> +                    Bits 2 - 31 are reserved and must be 0.
> +
> +             16:    type
> +                    This field describes the sort of the bitmap.
> +                    Values:
> +                      1: Dirty Tracking Bitmap
> +
> +                    Values 0, 2 - 255 are reserved.
> +
> +             17:    granularity_bits
> +                    Granularity bits. Valid values are: 0 - 63.
> +
> +                    Note: Qemu currently doesn't support granularity_bits
> +                    greater than 31.
> +
> +                    Granularity is calculated as
> +                        granularity = 1 << granularity_bits
> +
> +                    Granularity of the bitmap is how many bytes of the image
> +                    accounts for one bit of the bitmap.
> +
> +        18 - 19:    name_size
> +                    Size of the bitmap name. Valid values: 1 - 1023.
> +
> +        20 - 23:    extra_data_size
> +                    Size of type-specific extra data.
> +
> +                    For now, as no extra data is defined, extra_data_size is
> +                    reserved and must be zero.
> +
> +        variable:   Type-specific extra data for the bitmap.
> +

And I guess we don't have any definitions for the "Dirty Tracking" type,
so this is zero.

Should we add a table of the bitmap types and their corresponding
definitions for the "Type-specific extra data"?

e.g.

=== Bitmap Types ===

0: Reserved
   extra_data_size: 0

   This bitmap type is invalid and must not be used.

1: Dirty Tracking Bitmap
   extra_data_size: 0

   This bitmap type represents data changed since a point in time
   and has no extra data.

2: Lorem Ipsum Bitmap
   extra_data_size: 4

   Lorem ipsum, dolor sit amet...
   This bitmap type has four byte of extra data:

   0-1: Identifier
   2-3: Padding

> +        variable:   The name of the bitmap (not null terminated). Must be
> +                    unique among all bitmap names within the Bitmaps extension.
> +
> +        variable:   Padding to round up the Bitmap Directory Entry size to the
> +                    next multiple of 8.
> +
> +=== Bitmap Table ===
> +
> +Bitmaps are stored using a one-level (not two-level like refcounts and guest
> +clusters mapping) structure for the mapping of bitmaps data to host clusters.
> +It is called Bitmap Table.
> +

I might add "the" before Bitmap Table above.

> +Each Bitmap Table has a variable size (stored in the Bitmap Directory Entry)
> +and may use multiple clusters, however it must be contiguous in the image file.
> +
> +Bitmap Table entry:
> +
> +    Bit       0:    Reserved and must be zero if bits 9 - 55 are non-zero.
> +                    If bits 9 - 55 are zero:
> +                      0: Cluster should be read as all zeros.
> +                      1: Cluster should be read as all ones.
> +
> +         1 -  8:    Reserved and must be zero.
> +
> +         9 - 55:    Bits 9 - 55 of host cluster offset. Must be aligned to a
> +                    cluster boundary. If the offset is 0, the cluster is
> +                    unallocated, see bit 0 description.
> +
> +        56 - 63:    Reserved and must be zero.
> +
> +=== Bitmap Data ===
> +
> +As noted above, bitmap data is stored in several (or may be one, exactly
> +bitmap_table_size) separate clusters, described by Bitmap Table. Given an
> +offset (in bytes) into the bitmap data, the offset into the image file can be
> +obtained as follows:
> +
> +    image_offset =
> +        bitmap_table[bitmap_data_offset / cluster_size] +
> +            (bitmap_data_offset % cluster_size)
> +
> +Taking into account the granularity of the bitmap, an offset in bits into the
> +image file, corresponding to byte number byte_nr of the virtual disk can be
> +calculated like this:
> +
> +    bit_offset =
> +        image_offset(byte_nr / granularity / 8) * 8 +
> +            (byte_nr / granularity) % 8
> 

That's all the language commentary I have on this. Hopefully not long
before consensus.

Thank you,
--John
Vladimir Sementsov-Ogievskiy Jan. 11, 2016, 12:20 p.m. UTC | #8
On 05.01.2016 02:16, John Snow wrote:
> Since Max didn't offer a grammatical review, here's my attempt at some
> suggestions.
>
> On 12/23/2015 12:49 PM, Vladimir Sementsov-Ogievskiy wrote:
>> The new feature for qcow2: storing bitmaps.
>>
>> This patch adds new header extension to qcow2 - Bitmaps Extension. It
>> provides an ability to store virtual disk related bitmaps in a qcow2
>> image. For now there is only one type of such bitmaps: Dirty Tracking
>> Bitmap, which just tracks virtual disk changes from some moment.
>>
>> Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
>> should be stored in this qcow2 file. The size of each bitmap
>> (considering its granularity) is equal to virtual disk size.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>
>> v6:
>>
>> - reword bitmap_directory_size description
>> - bitmap type: make 0 reserved
>> - extra_data_size: resize to 4bytes
>>    Also, I've marked this field as "must be zero". We can always change
>>    it, if we decide allowing managing app to specify any extra data, by
>>    defining some magic value as a top of user extra data.. So, for now
>>    non zeor extra_data_size should be considered as an error.
>> - swap name and extra_data to give good alignment to extra_data.
>>
>>
>> v5:
>>
>> - 'Dirty bitmaps' renamed to 'Bitmaps', as we may have several types of
>>    bitmaps.
>> - rewordings
>> - move upper bounds to "Notes about Qemu limits"
>> - s/should/must somewhere. (but not everywhere)
>> - move name_size field closer to name itself in bitmap header
>> - add extra data area to bitmap header
>> - move bitmap data description to separate section
>>
>>   docs/specs/qcow2.txt | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 160 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
>> index 121dfc8..b23966a 100644
>> --- a/docs/specs/qcow2.txt
>> +++ b/docs/specs/qcow2.txt
>> @@ -103,7 +103,19 @@ in the description of a field.
>>                       write to an image with unknown auto-clear features if it
>>                       clears the respective bits from this field first.
>>   
>> -                    Bits 0-63:  Reserved (set to 0)
>> +                    Bit 0:      Bitmaps extension bit.
> For consistency, no period after this.
>
>> +                                This bit is responsible for Bitmaps extension
>> +                                consistency.
>> +
> I might phrase it as: "This bit indicates consistency for the Bitmaps
> extension data."
>
>> +                                If it is set, but there is no Bitmaps
>> +                                extension, this should be considered as an
>> +                                error.
>> +
> "This should be considered as an error" can be shortened to just "This
> is an error." This makes the sentence top-heavy though, so how about:
>
> "It is an error if this bit is set without the Bitmaps extension present."
>
>> +                                If it is not set, but there is a Bitmaps
>> +                                extension, its data should be considered as
>> +                                inconsistent.
>> +
> Let's remove "considered" here. The data *is* inconsistent if this has
> happened.
>
> "If the Bitmaps extension is present but this bit is unset, the Bitmaps
> extension data is inconsistent."
>
>> +                    Bits 1-63:  Reserved (set to 0)
>>   
>>            96 -  99:  refcount_order
>>                       Describes the width of a reference count block entry (width
>> @@ -123,6 +135,7 @@ be stored. Each extension has a structure like the following:
>>                           0x00000000 - End of the header extension area
>>                           0xE2792ACA - Backing file format name
>>                           0x6803f857 - Feature name table
>> +                        0x23852875 - Bitmaps extension
>>                           other      - Unknown header extension, can be safely
>>                                        ignored
>>   
>> @@ -166,6 +179,34 @@ the header extension data. Each entry look like this:
>>                       terminated if it has full length)
>>   
>>   
>> +== Bitmaps extension ==
>> +
>> +Bitmaps extension is an optional header extension. It provides an ability to
>> +store virtual disk related bitmaps in a qcow2 image. For now there is only one
>> +type of such bitmaps: Dirty Tracking Bitmap, which just tracks virtual disk
>> +changes from some moment.
>> +
> I think "Bitmaps extension" is awkward without "The" leading it, so:
>
> "The Bitmaps extension is an optional header extension."
>
> And from Eric's suggestion:
>
> "It provides the ability to store bitmaps related to a virtual disk.
> For now, there is only one bitmap type: Dirty Tracking Bitmap, which
> tracks virtual disk changes from some moment."
>
>> +The data of the extension should be considered as consistent only if
>> +corresponding auto-clear feature bit is set (see autoclear_features above).
>> +
> I might remove the parenthetical.
>
> "The data in this extension should be considered consistent only if the
> corresponding auto-clear feature bit is set, see autoclear_features above."
>
>> +The fields of Bitmaps extension are:
>> +
> Adding 'the' again:
>
> "The fields of the Bitmaps extension are:"
>
>> +          0 -  3:  nb_bitmaps
>> +                   The number of bitmaps contained in the image. Must be
>> +                   greater or equal to 1.
>> +
> "Greater than or equal to" is a common phrasing -- adding the 'than'.
>
>> +                   Note: Qemu currently only supports up to 65535 bitmaps per
>> +                   image.
>> +
>> +          4 -  7:  bitmap_directory_size
>> +                   Size of the Bitmap Directory in bytes. It is a cumulative
>> +                   size of all (nb_bitmaps) bitmap headers.
>> +
>> +          8 - 15:  bitmap_directory_offset
>> +                   Offset into the image file at which the Bitmap Directory
>> +                   starts. Must be aligned to a cluster boundary.
>> +
>> +
>>   == Host cluster management ==
>>   
>>   qcow2 manages the allocation of host clusters by maintaining a reference count
>> @@ -360,3 +401,121 @@ Snapshot table entry:
>>   
>>           variable:   Padding to round up the snapshot table entry size to the
>>                       next multiple of 8.
>> +
>> +
>> +== Bitmaps ==
>> +
>> +The feature supports storing bitmaps in a qcow2 image. All bitmaps are related
>> +to the virtual disk, stored in this image.
>> +
> Maybe like Eric's earlier suggestion, we can avoid "in a qcow2 image"
> here. We can probably just repeat some of the earlier description here.
>
>> +=== Bitmap Directory ===
>> +
>> +Each bitmap saved in the image is described in a Bitmap Directory entry. Bitmap
> "The Bitmap Directory"
>
>> +Directory is a contiguous area in the image file, whose starting offset and
>> +length are given by the header extension fields bitmap_directory_offset and
>> +bitmap_directory_size. The entries of the bitmap directory have variable
>> +length, depending on the length of the bitmap name and extra data. These
>> +entries are also called bitmap headers.
>> +
>> +Bitmap Directory Entry:
>> +
>> +    Byte 0 -  7:    bitmap_table_offset
>> +                    Offset into the image file at which the Bitmap Table
>> +                    (described below) for the bitmap starts. Must be aligned to
>> +                    a cluster boundary.
>> +
>> +         8 - 11:    bitmap_table_size
>> +                    Number of entries in the Bitmap Table of the bitmap.
>> +
>> +        12 - 15:    flags
>> +                    Bit
>> +                      0: in_use
>> +                         The bitmap was not saved correctly and may be
>> +                         inconsistent.
>> +
>> +                      1: auto
>> +                         The bitmap must reflect all changes of the virtual
>> +                         disk by any application that would write to this qcow2
>> +                         file (including writes, snapshot switching, etc.). The
>> +                         type of this bitmap must be 'Dirty Tracking Bitmap'.
>> +
>> +                    Bits 2 - 31 are reserved and must be 0.
>> +
>> +             16:    type
>> +                    This field describes the sort of the bitmap.
>> +                    Values:
>> +                      1: Dirty Tracking Bitmap
>> +
>> +                    Values 0, 2 - 255 are reserved.
>> +
>> +             17:    granularity_bits
>> +                    Granularity bits. Valid values are: 0 - 63.
>> +
>> +                    Note: Qemu currently doesn't support granularity_bits
>> +                    greater than 31.
>> +
>> +                    Granularity is calculated as
>> +                        granularity = 1 << granularity_bits
>> +
>> +                    Granularity of the bitmap is how many bytes of the image
>> +                    accounts for one bit of the bitmap.
>> +
>> +        18 - 19:    name_size
>> +                    Size of the bitmap name. Valid values: 1 - 1023.
>> +
>> +        20 - 23:    extra_data_size
>> +                    Size of type-specific extra data.
>> +
>> +                    For now, as no extra data is defined, extra_data_size is
>> +                    reserved and must be zero.
>> +
>> +        variable:   Type-specific extra data for the bitmap.
>> +
> And I guess we don't have any definitions for the "Dirty Tracking" type,
> so this is zero.
>
> Should we add a table of the bitmap types and their corresponding
> definitions for the "Type-specific extra data"?
>
> e.g.
>
> === Bitmap Types ===
>
> 0: Reserved
>     extra_data_size: 0
>
>     This bitmap type is invalid and must not be used.
>
> 1: Dirty Tracking Bitmap
>     extra_data_size: 0

Are you sure? What about creation\last change dates, file links, user 
data, etc?
For now, formally, current "For now, as no extra data is defined, 
extra_data_size is reserved and must be zero." is equal to such table, 
but provides more flexibility for future..

>
>     This bitmap type represents data changed since a point in time
>     and has no extra data.
>
> 2: Lorem Ipsum Bitmap
>     extra_data_size: 4
>
>     Lorem ipsum, dolor sit amet...
>     This bitmap type has four byte of extra data:
>
>     0-1: Identifier
>     2-3: Padding
>
>> +        variable:   The name of the bitmap (not null terminated). Must be
>> +                    unique among all bitmap names within the Bitmaps extension.
>> +
>> +        variable:   Padding to round up the Bitmap Directory Entry size to the
>> +                    next multiple of 8.
>> +
>> +=== Bitmap Table ===
>> +
>> +Bitmaps are stored using a one-level (not two-level like refcounts and guest
>> +clusters mapping) structure for the mapping of bitmaps data to host clusters.
>> +It is called Bitmap Table.
>> +
> I might add "the" before Bitmap Table above.
>
>> +Each Bitmap Table has a variable size (stored in the Bitmap Directory Entry)
>> +and may use multiple clusters, however it must be contiguous in the image file.
>> +
>> +Bitmap Table entry:
>> +
>> +    Bit       0:    Reserved and must be zero if bits 9 - 55 are non-zero.
>> +                    If bits 9 - 55 are zero:
>> +                      0: Cluster should be read as all zeros.
>> +                      1: Cluster should be read as all ones.
>> +
>> +         1 -  8:    Reserved and must be zero.
>> +
>> +         9 - 55:    Bits 9 - 55 of host cluster offset. Must be aligned to a
>> +                    cluster boundary. If the offset is 0, the cluster is
>> +                    unallocated, see bit 0 description.
>> +
>> +        56 - 63:    Reserved and must be zero.
>> +
>> +=== Bitmap Data ===
>> +
>> +As noted above, bitmap data is stored in several (or may be one, exactly
>> +bitmap_table_size) separate clusters, described by Bitmap Table. Given an
>> +offset (in bytes) into the bitmap data, the offset into the image file can be
>> +obtained as follows:
>> +
>> +    image_offset =
>> +        bitmap_table[bitmap_data_offset / cluster_size] +
>> +            (bitmap_data_offset % cluster_size)
>> +
>> +Taking into account the granularity of the bitmap, an offset in bits into the
>> +image file, corresponding to byte number byte_nr of the virtual disk can be
>> +calculated like this:
>> +
>> +    bit_offset =
>> +        image_offset(byte_nr / granularity / 8) * 8 +
>> +            (byte_nr / granularity) % 8
>>
> That's all the language commentary I have on this. Hopefully not long
> before consensus.
>
> Thank you,
> --John
John Snow Jan. 11, 2016, 5:07 p.m. UTC | #9
On 01/11/2016 07:20 AM, Vladimir Sementsov-Ogievskiy wrote:
> 
> Are you sure? What about creation\last change dates, file links, user
> data, etc?
> For now, formally, current "For now, as no extra data is defined,
> extra_data_size is reserved and must be zero." is equal to such table,
> but provides more flexibility for future..

Oh, I see what you're trying to do.

In this case, perhaps we need a versioning system for the type-specific
data? We won't be able to just add data arbitrarily, we need to change
some field somewhere.

Maybe we can say something like...

"If extra_data_size is 0, there is no type-specific data and the version
of that data layout is 0. If extra_data_size is non-zero, the first byte
of the type-specific-data must be a version number greater than 0 that
indicates the layout of the data to follow.

For the Dirty Tracking bitmap type, only version 0 is currently valid."

This way it's explicit that data *could* show up for dirty tracking in
the future, but currently it does not.

--js
Vladimir Sementsov-Ogievskiy Jan. 12, 2016, 8:30 a.m. UTC | #10
On 11.01.2016 20:07, John Snow wrote:
>
> On 01/11/2016 07:20 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Are you sure? What about creation\last change dates, file links, user
>> data, etc?
>> For now, formally, current "For now, as no extra data is defined,
>> extra_data_size is reserved and must be zero." is equal to such table,
>> but provides more flexibility for future..
> Oh, I see what you're trying to do.
>
> In this case, perhaps we need a versioning system for the type-specific
> data? We won't be able to just add data arbitrarily, we need to change
> some field somewhere.
>
> Maybe we can say something like...
>
> "If extra_data_size is 0, there is no type-specific data and the version
> of that data layout is 0. If extra_data_size is non-zero, the first byte
> of the type-specific-data must be a version number greater than 0 that
> indicates the layout of the data to follow.
>
> For the Dirty Tracking bitmap type, only version 0 is currently valid."
>
> This way it's explicit that data *could* show up for dirty tracking in
> the future, but currently it does not.
>
> --js
It's ok for me.
diff mbox

Patch

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 121dfc8..b23966a 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -103,7 +103,19 @@  in the description of a field.
                     write to an image with unknown auto-clear features if it
                     clears the respective bits from this field first.
 
-                    Bits 0-63:  Reserved (set to 0)
+                    Bit 0:      Bitmaps extension bit.
+                                This bit is responsible for Bitmaps extension
+                                consistency.
+
+                                If it is set, but there is no Bitmaps
+                                extension, this should be considered as an
+                                error.
+
+                                If it is not set, but there is a Bitmaps
+                                extension, its data should be considered as
+                                inconsistent.
+
+                    Bits 1-63:  Reserved (set to 0)
 
          96 -  99:  refcount_order
                     Describes the width of a reference count block entry (width
@@ -123,6 +135,7 @@  be stored. Each extension has a structure like the following:
                         0x00000000 - End of the header extension area
                         0xE2792ACA - Backing file format name
                         0x6803f857 - Feature name table
+                        0x23852875 - Bitmaps extension
                         other      - Unknown header extension, can be safely
                                      ignored
 
@@ -166,6 +179,34 @@  the header extension data. Each entry look like this:
                     terminated if it has full length)
 
 
+== Bitmaps extension ==
+
+Bitmaps extension is an optional header extension. It provides an ability to
+store virtual disk related bitmaps in a qcow2 image. For now there is only one
+type of such bitmaps: Dirty Tracking Bitmap, which just tracks virtual disk
+changes from some moment.
+
+The data of the extension should be considered as consistent only if
+corresponding auto-clear feature bit is set (see autoclear_features above).
+
+The fields of Bitmaps extension are:
+
+          0 -  3:  nb_bitmaps
+                   The number of bitmaps contained in the image. Must be
+                   greater or equal to 1.
+
+                   Note: Qemu currently only supports up to 65535 bitmaps per
+                   image.
+
+          4 -  7:  bitmap_directory_size
+                   Size of the Bitmap Directory in bytes. It is a cumulative
+                   size of all (nb_bitmaps) bitmap headers.
+
+          8 - 15:  bitmap_directory_offset
+                   Offset into the image file at which the Bitmap Directory
+                   starts. Must be aligned to a cluster boundary.
+
+
 == Host cluster management ==
 
 qcow2 manages the allocation of host clusters by maintaining a reference count
@@ -360,3 +401,121 @@  Snapshot table entry:
 
         variable:   Padding to round up the snapshot table entry size to the
                     next multiple of 8.
+
+
+== Bitmaps ==
+
+The feature supports storing bitmaps in a qcow2 image. All bitmaps are related
+to the virtual disk, stored in this image.
+
+=== Bitmap Directory ===
+
+Each bitmap saved in the image is described in a Bitmap Directory entry. Bitmap
+Directory is a contiguous area in the image file, whose starting offset and
+length are given by the header extension fields bitmap_directory_offset and
+bitmap_directory_size. The entries of the bitmap directory have variable
+length, depending on the length of the bitmap name and extra data. These
+entries are also called bitmap headers.
+
+Bitmap Directory Entry:
+
+    Byte 0 -  7:    bitmap_table_offset
+                    Offset into the image file at which the Bitmap Table
+                    (described below) for the bitmap starts. Must be aligned to
+                    a cluster boundary.
+
+         8 - 11:    bitmap_table_size
+                    Number of entries in the Bitmap Table of the bitmap.
+
+        12 - 15:    flags
+                    Bit
+                      0: in_use
+                         The bitmap was not saved correctly and may be
+                         inconsistent.
+
+                      1: auto
+                         The bitmap must reflect all changes of the virtual
+                         disk by any application that would write to this qcow2
+                         file (including writes, snapshot switching, etc.). The
+                         type of this bitmap must be 'Dirty Tracking Bitmap'.
+
+                    Bits 2 - 31 are reserved and must be 0.
+
+             16:    type
+                    This field describes the sort of the bitmap.
+                    Values:
+                      1: Dirty Tracking Bitmap
+
+                    Values 0, 2 - 255 are reserved.
+
+             17:    granularity_bits
+                    Granularity bits. Valid values are: 0 - 63.
+
+                    Note: Qemu currently doesn't support granularity_bits
+                    greater than 31.
+
+                    Granularity is calculated as
+                        granularity = 1 << granularity_bits
+
+                    Granularity of the bitmap is how many bytes of the image
+                    accounts for one bit of the bitmap.
+
+        18 - 19:    name_size
+                    Size of the bitmap name. Valid values: 1 - 1023.
+
+        20 - 23:    extra_data_size
+                    Size of type-specific extra data.
+
+                    For now, as no extra data is defined, extra_data_size is
+                    reserved and must be zero.
+
+        variable:   Type-specific extra data for the bitmap.
+
+        variable:   The name of the bitmap (not null terminated). Must be
+                    unique among all bitmap names within the Bitmaps extension.
+
+        variable:   Padding to round up the Bitmap Directory Entry size to the
+                    next multiple of 8.
+
+=== Bitmap Table ===
+
+Bitmaps are stored using a one-level (not two-level like refcounts and guest
+clusters mapping) structure for the mapping of bitmaps data to host clusters.
+It is called Bitmap Table.
+
+Each Bitmap Table has a variable size (stored in the Bitmap Directory Entry)
+and may use multiple clusters, however it must be contiguous in the image file.
+
+Bitmap Table entry:
+
+    Bit       0:    Reserved and must be zero if bits 9 - 55 are non-zero.
+                    If bits 9 - 55 are zero:
+                      0: Cluster should be read as all zeros.
+                      1: Cluster should be read as all ones.
+
+         1 -  8:    Reserved and must be zero.
+
+         9 - 55:    Bits 9 - 55 of host cluster offset. Must be aligned to a
+                    cluster boundary. If the offset is 0, the cluster is
+                    unallocated, see bit 0 description.
+
+        56 - 63:    Reserved and must be zero.
+
+=== Bitmap Data ===
+
+As noted above, bitmap data is stored in several (or may be one, exactly
+bitmap_table_size) separate clusters, described by Bitmap Table. Given an
+offset (in bytes) into the bitmap data, the offset into the image file can be
+obtained as follows:
+
+    image_offset =
+        bitmap_table[bitmap_data_offset / cluster_size] +
+            (bitmap_data_offset % cluster_size)
+
+Taking into account the granularity of the bitmap, an offset in bits into the
+image file, corresponding to byte number byte_nr of the virtual disk can be
+calculated like this:
+
+    bit_offset =
+        image_offset(byte_nr / granularity / 8) * 8 +
+            (byte_nr / granularity) % 8