Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-08-31 Thread Eric Blake 
On 06/08/2015 09:21 AM, Vladimir Sementsov-Ogievskiy wrote:
> From: Vladimir Sementsov-Ogievskiy 
> 
> Persistent dirty bitmaps will be saved into qcow2 files. It may be used
> as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
> other drives (there may be qcow2 file with zero disk size but with
> several dirty bitmaps for other drives).
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  docs/specs/qcow2.txt | 66 
> 
>  1 file changed, 66 insertions(+)
> 

> +== Dirty bitmaps ==
> +
> +Dirty bitmaps is an optional header extension. It provides a possibility of
> +storing dirty bitmaps in qcow2 image. The fields are:
> +
> +  0 -  3:  nb_dirty_bitmaps
> +   Number of dirty bitmaps contained in the image
> +
> +  4 - 11:  dirty_bitmaps_offset
> +   Offset into the image file at which the dirty bitmaps 
> table
> +   starts. Must be aligned to a cluster boundary.

To date, all 8-byte fields in qcow2 have been 8-byte aligned; this would
break that nice feature.  You could keep that nice property by swapping
the order of these two fields.

[Note that the spec on header extensions already requires clients to
recognize that a header extension of 12 bytes implicitly pads out an
additional 4 bytes, so that the next header extension type field once
again lands on 8-byte alignment]

> +== Dirty bitmaps ==
> +
> +The feature supports storing several dirty bitmaps in the qcow2 file.

Is it possible to have a qcow2 file that stores JUST dirty bitmap(s) and
no guest data (that is, no L1 table, no backing file)?  It might make
sense, if we intend to allow persistent bitmap files that can be
associated with a raw disk.  But right now, the spec seems to require
that l1_table_offset must be non-zero.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-08-31 Thread John Snow 


On 08/31/2015 06:21 PM, Eric Blake wrote:
> On 06/08/2015 09:21 AM, Vladimir Sementsov-Ogievskiy wrote:
>> From: Vladimir Sementsov-Ogievskiy 
>>
>> Persistent dirty bitmaps will be saved into qcow2 files. It may be used
>> as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
>> other drives (there may be qcow2 file with zero disk size but with
>> several dirty bitmaps for other drives).
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>> ---
>>  docs/specs/qcow2.txt | 66 
>> 
>>  1 file changed, 66 insertions(+)
>>
> 
>> +== Dirty bitmaps ==
>> +
>> +Dirty bitmaps is an optional header extension. It provides a possibility of
>> +storing dirty bitmaps in qcow2 image. The fields are:
>> +
>> +  0 -  3:  nb_dirty_bitmaps
>> +   Number of dirty bitmaps contained in the image
>> +
>> +  4 - 11:  dirty_bitmaps_offset
>> +   Offset into the image file at which the dirty bitmaps 
>> table
>> +   starts. Must be aligned to a cluster boundary.
> 
> To date, all 8-byte fields in qcow2 have been 8-byte aligned; this would
> break that nice feature.  You could keep that nice property by swapping
> the order of these two fields.
> 
> [Note that the spec on header extensions already requires clients to
> recognize that a header extension of 12 bytes implicitly pads out an
> additional 4 bytes, so that the next header extension type field once
> again lands on 8-byte alignment]
> 
>> +== Dirty bitmaps ==
>> +
>> +The feature supports storing several dirty bitmaps in the qcow2 file.
> 
> Is it possible to have a qcow2 file that stores JUST dirty bitmap(s) and
> no guest data (that is, no L1 table, no backing file)?  It might make
> sense, if we intend to allow persistent bitmap files that can be
> associated with a raw disk.  But right now, the spec seems to require
> that l1_table_offset must be non-zero.
> 

To my knowledge, this is a consequence of the decision that bitmaps do
not have to be related to the data contained within the .qcow2.

Ultimately, it's fine (for our purposes) there's no data in the qcow2. I
have been testing with a 0-length qcow2 to test this patchset.

--js

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-08-24 Thread Vladimir Sementsov-Ogievskiy 

About structs and constraints:

== Optional Header ==

64bit nb_dirty_bitmaps
valid: 1 - 65536. I think here should not be 0, in this case 
dirty-bitmap-optional-header should not exist at all. Should it instead 
be 0 - 65536


64bit dirty_bitmaps_offset
valid: any, but dirty_bitmaps_offset % cluster_size = 0

== Dirty BItmap Directory Enrty ( = bitmap header) ==

64bit dirty_bitmap_table_offset
valid: any, but dirty_bitmap_table_offset % cluster_size = 0

64bit nb_virtual_bits (before it was called bitmap_size)
valid: no direct constraints (as for disk size), but it should be 
= dirty_bitmap_table_size * cluster_size * 8 * bitmap_granularity


32bit dirty_bitmap_table_size
? The bitmap will take ~ dirty_bitmap_table_size * cluster_size 
bytes in RAM. What the limit should be for it?

for bdrv dirty bitmaps
  max disk size = QCOW_MAX_L1_SIZE / 8 * (cluster_size / 8) * 
cluster_size = 524288 * cluster_size ^ 2,
  dirty bitmap covers the following size: 
dirty_bitmap_table_size * cluster_size * 8 * byte_granularity

  therefore, to cover the whole disk,
  dirty_bitmap_table_size * cluster_size * 8 * byte_granularity 
= 524288 * cluster_size ^ 2
  i.e. dirty_bitmap_table_size = 65536 * cluster_size / 
byte_granularity
  max cluster size is 0x20 ( = 1  21 ), min granularity = 
sector_size = 512, so
  dirty_bitmap_table_size = 0x1000, and this limit should 
be enough for any current disk configuration, but, with cluster of size 
1  21
  and such dirty_bitmap_table_size we will have tooo huge 
bitmap in RAM and it is unreal..


32bit bitmap_granularity_bits ( before it was bitmap_granularity)
valid; 0 - 63 (as for HBitmap)
(1  bitmap_granularity_bits) is number of virtual bits in one 
physical bit. Not related to sectors/bytes, etc. Let this format be 
closer to HBitmap than to BdrvDirtyBitmap


16bit name_size
valid: 1 - 1023. // should it be 0 - 1023 ?

/* name follows */
/* offset to 8 bytes boundary follows */

==About granularity==
To make things more general, granularity is not related to 
disk/bytes/sectors. Granularity is just number of virtual bits in one 
physical bit.


Then, for bdrv dirty bitmaps (let's assume, that it may be not only one 
type of store dirty bitmaps), let establish, that one virtual bit is 
corresponding to one sector of the disk, i.e., byte granularity would be:

byte_granularity = (1  bitmap_granularity_bits) * 512
for ex:
65536 = (1  7) * 512

On 08.06.2015 18:21, Vladimir Sementsov-Ogievskiy wrote:

From: Vladimir Sementsov-Ogievskiy vsement...@parallels.com

Persistent dirty bitmaps will be saved into qcow2 files. It may be used
as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
other drives (there may be qcow2 file with zero disk size but with
several dirty bitmaps for other drives).

Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@virtuozzo.com
---
  docs/specs/qcow2.txt | 66 
  1 file changed, 66 insertions(+)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 121dfc8..0fffba2 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -123,6 +123,7 @@ be stored. Each extension has a structure like the 
following:
  0x - End of the header extension area
  0xE2792ACA - Backing file format name
  0x6803f857 - Feature name table
+0x23852875 - Dirty bitmaps
  other  - Unknown header extension, can be safely
   ignored
  
@@ -166,6 +167,19 @@ the header extension data. Each entry look like this:

  terminated if it has full length)
  
  
+== Dirty bitmaps ==

+
+Dirty bitmaps is an optional header extension. It provides a possibility of
+storing dirty bitmaps in qcow2 image. The fields are:
+
+  0 -  3:  nb_dirty_bitmaps
+   Number of dirty bitmaps contained in the image
+
+  4 - 11:  dirty_bitmaps_offset
+   Offset into the image file at which the dirty bitmaps table
+   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 +374,55 @@ Snapshot table entry:
  
  variable:   Padding to round up the snapshot table entry size to the

  next multiple of 8.
+
+
+== Dirty bitmaps ==
+
+The feature supports storing several dirty bitmaps in the qcow2 file.
+
+=== Cluster mapping ===
+
+Dirty bitmaps are stored using a ONE-level structure for the mapping of
+bitmaps to host clusters. There is only an L1 table.
+
+The L1 table has a variable size (stored in the Bitmap table entry) and may
+use multiple clusters, however it must be contiguous in the image file.
+
+Given an offset into the

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-08-24 Thread Vladimir Sementsov-Ogievskiy 

About structs and constraints:

Optional Header:

64bit nb_dirty_bitmaps
valid: 1 - 65536. I think here should not be 0, in this case 
dirty-bitmap-optional-header should not exist at all. Should it instead 
be 0 - 65536

64bit dirty_bitmaps_offset
valid: any, but dirty_bitmaps_offset % cluster_size = 0

Dirty BItmap Directory Enrty ( = bitmap header):

64bit dirty_bitmap_table_offset
valid: any, but dirty_bitmaps_offset % cluster_size = 0
64bit nb_virtual_bits (before it was called bitmap_size)
valid: no direct constraints (as for disk size), but it should be 
less then dirty_bitmap_table_size * cluster_size * 8 * bitmap_granularity

32bit dirty_bitmap_table_size
? The bitmap will take ~ dirty_bitmap_table_size * cluster_size 
bytes in RAM. What the limit should be for it?

32bit bitmap_granularity_bits ( before it was bitmap_granularity)
valid; 0 - 63 (as for HBitmap)
(1  bitmap_granularity_bits) is number of virtual bits in one 
physical bit. Not related to sectors/bytes, etc. Let this format be 
closer to HBitmap than to BdrvDirtyBitmap

16bit name_size
valid: 1 - 1023. // should it be 0 - 1023 ?
/* name follows */
/* offset to 8 bytes boundary follows */

On 08.06.2015 18:21, Vladimir Sementsov-Ogievskiy wrote:

From: Vladimir Sementsov-Ogievskiy vsement...@parallels.com

Persistent dirty bitmaps will be saved into qcow2 files. It may be used
as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
other drives (there may be qcow2 file with zero disk size but with
several dirty bitmaps for other drives).

Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@virtuozzo.com
---
  docs/specs/qcow2.txt | 66 
  1 file changed, 66 insertions(+)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 121dfc8..0fffba2 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -123,6 +123,7 @@ be stored. Each extension has a structure like the 
following:
  0x - End of the header extension area
  0xE2792ACA - Backing file format name
  0x6803f857 - Feature name table
+0x23852875 - Dirty bitmaps
  other  - Unknown header extension, can be safely
   ignored
  
@@ -166,6 +167,19 @@ the header extension data. Each entry look like this:

  terminated if it has full length)
  
  
+== Dirty bitmaps ==

+
+Dirty bitmaps is an optional header extension. It provides a possibility of
+storing dirty bitmaps in qcow2 image. The fields are:
+
+  0 -  3:  nb_dirty_bitmaps
+   Number of dirty bitmaps contained in the image
+
+  4 - 11:  dirty_bitmaps_offset
+   Offset into the image file at which the dirty bitmaps table
+   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 +374,55 @@ Snapshot table entry:
  
  variable:   Padding to round up the snapshot table entry size to the

  next multiple of 8.
+
+
+== Dirty bitmaps ==
+
+The feature supports storing several dirty bitmaps in the qcow2 file.
+
+=== Cluster mapping ===
+
+Dirty bitmaps are stored using a ONE-level structure for the mapping of
+bitmaps to host clusters. There is only an L1 table.
+
+The L1 table has a variable size (stored in the Bitmap table entry) and may
+use multiple clusters, however it must be contiguous in the image file.
+
+Given an offset into the bitmap, the offset into the image file can be
+obtained as follows:
+
+offset = l1_table[offset / cluster_size] + (offset % cluster_size)
+
+L1 table entry:
+
+Bit  0 -  61:   Standard cluster descriptor
+
+62 -  63:   Reserved
+
+=== Bitmap table ===
+
+A directory of all bitmaps is stored in the bitmap table, a contiguous area in
+the image file, whose starting offset and length are given by the header fields
+dirty_bitmaps_offset and nb_dirty_bitmaps. The entries of the bitmap table have
+variable length, depending on the length of name and extra data.
+
+Bitmap table entry:
+
+Byte 0 -  7:Offset into the image file at which the L1 table for the
+bitmap starts. Must be aligned to a cluster boundary.
+
+ 8 - 11:Number of entries in the L1 table of the bitmap
+
+12 - 15:Bitmap granularity in bytes
+
+16 - 23:Bitmap size in sectors
+
+24 - 25:Size of the bitmap name
+
+variable:   The name of the bitmap (not null terminated)
+
+variable:   Padding to round up the bitmap table entry size to the
+next multiple of 8.
+
+The fields size, granularity and name are corresponding with the fields
+in struct BdrvDirtyBitmap.



--
Best regards,
Vladimir
* now, @virtuozzo.com

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-08-24 Thread Vladimir Sementsov-Ogievskiy 

Sorry, drop this if you, look at the new version of this litter
On 24.08.2015 16:30, Vladimir Sementsov-Ogievskiy wrote:

About structs and constraints:

Optional Header:

64bit nb_dirty_bitmaps
valid: 1 - 65536. I think here should not be 0, in this case 
dirty-bitmap-optional-header should not exist at all. Should it 
instead be 0 - 65536

64bit dirty_bitmaps_offset
valid: any, but dirty_bitmaps_offset % cluster_size = 0

Dirty BItmap Directory Enrty ( = bitmap header):

64bit dirty_bitmap_table_offset
valid: any, but dirty_bitmaps_offset % cluster_size = 0
64bit nb_virtual_bits (before it was called bitmap_size)
valid: no direct constraints (as for disk size), but it should be 
less then dirty_bitmap_table_size * cluster_size * 8 * bitmap_granularity

32bit dirty_bitmap_table_size
? The bitmap will take ~ dirty_bitmap_table_size * cluster_size 
bytes in RAM. What the limit should be for it?

32bit bitmap_granularity_bits ( before it was bitmap_granularity)
valid; 0 - 63 (as for HBitmap)
(1  bitmap_granularity_bits) is number of virtual bits in one 
physical bit. Not related to sectors/bytes, etc. Let this format be 
closer to HBitmap than to BdrvDirtyBitmap

16bit name_size
valid: 1 - 1023. // should it be 0 - 1023 ?
/* name follows */
/* offset to 8 bytes boundary follows */

On 08.06.2015 18:21, Vladimir Sementsov-Ogievskiy wrote:

From: Vladimir Sementsov-Ogievskiy vsement...@parallels.com

Persistent dirty bitmaps will be saved into qcow2 files. It may be used
as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
other drives (there may be qcow2 file with zero disk size but with
several dirty bitmaps for other drives).

Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@virtuozzo.com
---
  docs/specs/qcow2.txt | 66 


  1 file changed, 66 insertions(+)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 121dfc8..0fffba2 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -123,6 +123,7 @@ be stored. Each extension has a structure like 
the following:

  0x - End of the header extension area
  0xE2792ACA - Backing file format name
  0x6803f857 - Feature name table
+0x23852875 - Dirty bitmaps
  other  - Unknown header extension, can 
be safely

   ignored
  @@ -166,6 +167,19 @@ the header extension data. Each entry look 
like this:

  terminated if it has full length)
+== Dirty bitmaps ==
+
+Dirty bitmaps is an optional header extension. It provides a 
possibility of

+storing dirty bitmaps in qcow2 image. The fields are:
+
+  0 -  3:  nb_dirty_bitmaps
+   Number of dirty bitmaps contained in the image
+
+  4 - 11:  dirty_bitmaps_offset
+   Offset into the image file at which the dirty 
bitmaps table

+   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 +374,55 @@ Snapshot table entry:
variable:   Padding to round up the snapshot table entry 
size to the

  next multiple of 8.
+
+
+== Dirty bitmaps ==
+
+The feature supports storing several dirty bitmaps in the qcow2 file.
+
+=== Cluster mapping ===
+
+Dirty bitmaps are stored using a ONE-level structure for the mapping of
+bitmaps to host clusters. There is only an L1 table.
+
+The L1 table has a variable size (stored in the Bitmap table entry) 
and may

+use multiple clusters, however it must be contiguous in the image file.
+
+Given an offset into the bitmap, the offset into the image file can be
+obtained as follows:
+
+offset = l1_table[offset / cluster_size] + (offset % cluster_size)
+
+L1 table entry:
+
+Bit  0 -  61:   Standard cluster descriptor
+
+62 -  63:   Reserved
+
+=== Bitmap table ===
+
+A directory of all bitmaps is stored in the bitmap table, a 
contiguous area in
+the image file, whose starting offset and length are given by the 
header fields
+dirty_bitmaps_offset and nb_dirty_bitmaps. The entries of the bitmap 
table have

+variable length, depending on the length of name and extra data.
+
+Bitmap table entry:
+
+Byte 0 -  7:Offset into the image file at which the L1 table 
for the
+bitmap starts. Must be aligned to a cluster 
boundary.

+
+ 8 - 11:Number of entries in the L1 table of the bitmap
+
+12 - 15:Bitmap granularity in bytes
+
+16 - 23:Bitmap size in sectors
+
+24 - 25:Size of the bitmap name
+
+variable:   The name of the bitmap (not null terminated)
+
+variable:   Padding to round up the bitmap table entry size 
to the

+next multiple of 8.
+
+The fields

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-08-24 Thread Vladimir Sementsov-Ogievskiy 

On 10.06.2015 18:34, Kevin Wolf wrote:

Am 08.06.2015 um 17:21 hat Vladimir Sementsov-Ogievskiy geschrieben:

From: Vladimir Sementsov-Ogievskiy vsement...@parallels.com

Persistent dirty bitmaps will be saved into qcow2 files. It may be used
as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
other drives (there may be qcow2 file with zero disk size but with
several dirty bitmaps for other drives).

Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@virtuozzo.com
---
  docs/specs/qcow2.txt | 66 
  1 file changed, 66 insertions(+)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 121dfc8..0fffba2 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -123,6 +123,7 @@ be stored. Each extension has a structure like the 
following:
  0x - End of the header extension area
  0xE2792ACA - Backing file format name
  0x6803f857 - Feature name table
+0x23852875 - Dirty bitmaps
  other  - Unknown header extension, can be safely
   ignored
  
@@ -166,6 +167,19 @@ the header extension data. Each entry look like this:

  terminated if it has full length)
  
  
+== Dirty bitmaps ==

+
+Dirty bitmaps is an optional header extension. It provides a possibility of
+storing dirty bitmaps in qcow2 image. The fields are:
+
+  0 -  3:  nb_dirty_bitmaps
+   Number of dirty bitmaps contained in the image
+
+  4 - 11:  dirty_bitmaps_offset
+   Offset into the image file at which the dirty bitmaps table
+   starts. Must be aligned to a cluster boundary.
+
+
  == Host cluster management ==

You need to use a compatibility flag because for old qemu versions, the
dirty bitmaps (and associated metadata) are leaked clusters and qemu-img
check would repair them by resetting the refcount to 0.

At second sight, I see that your patches add an autoclear flag.
Presumably the contents of the dirty bitmaps is outdated when you
accessed the image with an older version, so this seems right. We just
need to document it.


  qcow2 manages the allocation of host clusters by maintaining a reference count
@@ -360,3 +374,55 @@ Snapshot table entry:
  
  variable:   Padding to round up the snapshot table entry size to the

  next multiple of 8.
+
+
+== Dirty bitmaps ==
+
+The feature supports storing several dirty bitmaps in the qcow2 file.
+
+=== Cluster mapping ===
+
+Dirty bitmaps are stored using a ONE-level structure for the mapping of
+bitmaps to host clusters. There is only an L1 table.
+
+The L1 table has a variable size (stored in the Bitmap table entry) and may
+use multiple clusters, however it must be contiguous in the image file.
+
+Given an offset into the bitmap, the offset into the image file can be
+obtained as follows:
+
+offset = l1_table[offset / cluster_size] + (offset % cluster_size)
+
+L1 table entry:
+
+Bit  0 -  61:   Standard cluster descriptor
+
+62 -  63:   Reserved

Stefan already mentioned that we don't have a L1 when there is only
one level, and that you shouldn't reuse the cluster descriptors from L2
tables.


+=== Bitmap table ===
+
+A directory of all bitmaps is stored in the bitmap table, a contiguous area in
+the image file, whose starting offset and length are given by the header fields
+dirty_bitmaps_offset and nb_dirty_bitmaps. The entries of the bitmap table have
+variable length, depending on the length of name and extra data.
+
+Bitmap table entry:
+
+Byte 0 -  7:Offset into the image file at which the L1 table for the
+bitmap starts. Must be aligned to a cluster boundary.
+
+ 8 - 11:Number of entries in the L1 table of the bitmap

Worth using 64 bits here? This can only cover 4 * 512 GB = 2 TB for the
smallest possible cluster size. Though it's 65536 * 512 = 32 PB for the
default, which might be enough for a while.


We store the bitmap in RAM.. I think 2 TB bitmap should not appear, 
larger granularity should be used for big disks.





+12 - 15:Bitmap granularity in bytes
+
+16 - 23:Bitmap size in sectors

Please don't use sectors, that's a meaningless unit. Bytes is better.


+24 - 25:Size of the bitmap name

We should use a smaller limit than the possible 64k to avoid too large
memory allocations. Nobody needs really long bitmap names.


+
+variable:   The name of the bitmap (not null terminated)
+
+variable:   Padding to round up the bitmap table entry size to the
+next multiple of 8.

Kevin



--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-12 Thread Kevin Wolf 
Am 11.06.2015 um 18:30 hat John Snow geschrieben:
 On 06/11/2015 06:25 AM, Vladimir Sementsov-Ogievskiy wrote:
  On 10.06.2015 18:34, Kevin Wolf wrote:
  Am 08.06.2015 um 17:21 hat Vladimir Sementsov-Ogievskiy geschrieben:
  +=== Bitmap table ===
  +
  +A directory of all bitmaps is stored in the bitmap table, a
  contiguous area in
  +the image file, whose starting offset and length are given by the
  header fields
  +dirty_bitmaps_offset and nb_dirty_bitmaps. The entries of the bitmap
  table have
  +variable length, depending on the length of name and extra data.
  +
  +Bitmap table entry:
  +
  +Byte 0 -  7:Offset into the image file at which the L1 table
  for the
  +bitmap starts. Must be aligned to a cluster
  boundary.
  +
  + 8 - 11:Number of entries in the L1 table of the bitmap
  Worth using 64 bits here? This can only cover 4 * 512 GB = 2 TB for the
  smallest possible cluster size. Though it's 65536 * 512 = 32 PB for the
  default, which might be enough for a while.
 
  +12 - 15:Bitmap granularity in bytes
  +
  +16 - 23:Bitmap size in sectors
  Please don't use sectors, that's a meaningless unit. Bytes is better.
  Just bad description. Actually it is ~ (number of bits in bitmap *
  granularity), and it is corresponding to number of sectors in the image.
 
 In defense of this, it does happen to be sectors, but what it /really/
 represents is the virtual addressable range of the bitmap (its 'size'),
 which just-so-happens to be a sector bitmap.

So not the size of the bitmap, but the size of (range in) the image that
is covered by the bitmap?

 We could just remove the word sectors entirely, and just flatly call
 it the bitmap size -- but this does reveal the internal nature of the
 block layer, which uses sector bitmaps.
 
 If you wish, we can rework this field to use bytes and just convert on
 every load/store into the format that we actually require. I suppose
 it'd match the QMP interface in that way.

Internally we can do whatever we want, but what is stored in the image
format can't be changed later on, so it should be kept as generic as
possible.

How about number of bits in the bitmap as the unit for the size? And
possibly require that it's a multiple of 8.

Kevin

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-12 Thread Stefan Hajnoczi 
On Thu, Jun 11, 2015 at 12:21:31PM -0400, John Snow wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA256
 
 
 
 On 06/11/2015 09:03 AM, Stefan Hajnoczi wrote:
  On Thu, Jun 11, 2015 at 01:19:24PM +0300, Vladimir
  Sementsov-Ogievskiy wrote:
  On 10.06.2015 16:24, Stefan Hajnoczi wrote:
  On Wed, Jun 10, 2015 at 11:19:30AM +0300, Vladimir
  Sementsov-Ogievskiy wrote:
  On 09.06.2015 20:03, Stefan Hajnoczi wrote:
  On Mon, Jun 08, 2015 at 06:21:19PM +0300, Vladimir
  Sementsov-Ogievskiy wrote:
  @@ -166,6 +167,19 @@ the header extension data. Each
  entry look like this: terminated if it has full length) 
  +== Dirty bitmaps == + +Dirty bitmaps is an optional
  header extension. It provides a possibility of +storing
  dirty bitmaps in qcow2 image. The fields are: + +
  0 -  3:  nb_dirty_bitmaps +   Number of
  dirty bitmaps contained in the image
  Is there a maximum?
  hmm. any proposals for this?
  65535 seems practical.
  
  So, you suggest to reduce this field width to 2b? And additional
  2 bytes reserved field, to achieve 8b-alignment?
  
  No, I would leave it 32-bit but impose a little (which can be

s/little/limit/

  increased later if necessary).  That's how nb_snapshots works too.
  
 
 Doesn't the code already limit the number of bitmaps via +#define
 QCOW_MAX_DIRTY_BITMAPS 65536, from patch 2?

It needs to be in the specification.


pgpybVyikc6GU.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-12 Thread John Snow 


On 06/12/2015 06:28 AM, Stefan Hajnoczi wrote:
 On Thu, Jun 11, 2015 at 12:21:31PM -0400, John Snow wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA256



 On 06/11/2015 09:03 AM, Stefan Hajnoczi wrote:
 On Thu, Jun 11, 2015 at 01:19:24PM +0300, Vladimir
 Sementsov-Ogievskiy wrote:
 On 10.06.2015 16:24, Stefan Hajnoczi wrote:
 On Wed, Jun 10, 2015 at 11:19:30AM +0300, Vladimir
 Sementsov-Ogievskiy wrote:
 On 09.06.2015 20:03, Stefan Hajnoczi wrote:
 On Mon, Jun 08, 2015 at 06:21:19PM +0300, Vladimir
 Sementsov-Ogievskiy wrote:
 @@ -166,6 +167,19 @@ the header extension data. Each
 entry look like this: terminated if it has full length) 
 +== Dirty bitmaps == + +Dirty bitmaps is an optional
 header extension. It provides a possibility of +storing
 dirty bitmaps in qcow2 image. The fields are: + +
 0 -  3:  nb_dirty_bitmaps +   Number of
 dirty bitmaps contained in the image
 Is there a maximum?
 hmm. any proposals for this?
 65535 seems practical.

 So, you suggest to reduce this field width to 2b? And additional
 2 bytes reserved field, to achieve 8b-alignment?

 No, I would leave it 32-bit but impose a little (which can be
 
 s/little/limit/
 
 increased later if necessary).  That's how nb_snapshots works too.


 Doesn't the code already limit the number of bitmaps via +#define
 QCOW_MAX_DIRTY_BITMAPS 65536, from patch 2?
 
 It needs to be in the specification.
 

Yes, but the way the replies read made it sound like we hadn't decided
on what the limit *was*, so I was just trying to clarify for myself, here.

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-11 Thread Vladimir Sementsov-Ogievskiy 

On 10.06.2015 16:00, Eric Blake wrote:

On 06/10/2015 02:19 AM, Vladimir Sementsov-Ogievskiy wrote:


+Dirty bitmaps is an optional header extension. It provides a
possibility of
+storing dirty bitmaps in qcow2 image. The fields are:
+
+  0 -  3:  nb_dirty_bitmaps
+   Number of dirty bitmaps contained in the image

Is there a maximum?

hmm. any proposals for this?

+
+  4 - 11:  dirty_bitmaps_offset

I'm not sure if there is a reasonable cap on the number of dirty
bitmaps; I doubt that anyone will actually supply all 4G possible images
allowed by the four-byte field, but don't have a suggestion on a smaller
limit that doesn't feel arbitrary.

[meta-comment] It's very hard to pick out the new content in your reply
if you do not separate your new text with a newline both before and
after (as I'm doing here).



+Dirty bitmaps are stored using a ONE-level structure for the mapping of
+bitmaps to host clusters. There is only an L1 table.
+
+The L1 table has a variable size (stored in the Bitmap table entry)
and may
+use multiple clusters, however it must be contiguous in the image file.

The use of L1 table could be confusing.  The refcount metadata uses
refcount table and refcount block to describe a one-level table.

I agree. Hmm.. dirty bitmaps table? ok?

dirty bitmaps table works for me, as a name for the one-level table.



for now, dirty bitmaps table is the table with bitmap descriptors, and 
each bitmap descriptor contains its own l1 table..
What about dirty bitmap directory for descriptors and dirty bitmap table 
for l1? like pde pte)


--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-11 Thread Vladimir Sementsov-Ogievskiy 

On 10.06.2015 16:24, Stefan Hajnoczi wrote:

On Wed, Jun 10, 2015 at 11:19:30AM +0300, Vladimir Sementsov-Ogievskiy wrote:

On 09.06.2015 20:03, Stefan Hajnoczi wrote:

On Mon, Jun 08, 2015 at 06:21:19PM +0300, Vladimir Sementsov-Ogievskiy wrote:

@@ -166,6 +167,19 @@ the header extension data. Each entry look like this:
  terminated if it has full length)
+== Dirty bitmaps ==
+
+Dirty bitmaps is an optional header extension. It provides a possibility of
+storing dirty bitmaps in qcow2 image. The fields are:
+
+  0 -  3:  nb_dirty_bitmaps
+   Number of dirty bitmaps contained in the image

Is there a maximum?

hmm. any proposals for this?

65535 seems practical.


So, you suggest to reduce this field width to 2b? And additional 2 bytes 
reserved field, to achieve 8b-alignment?





+=== Cluster mapping ===
+
+Dirty bitmaps are stored using a ONE-level structure for the mapping of
+bitmaps to host clusters. There is only an L1 table.
+
+The L1 table has a variable size (stored in the Bitmap table entry) and may
+use multiple clusters, however it must be contiguous in the image file.

The use of L1 table could be confusing.  The refcount metadata uses
refcount table and refcount block to describe a one-level table.

I agree. Hmm.. dirty bitmaps table? ok?

Yes, that is good.


+
+Given an offset into the bitmap, the offset into the image file can be
+obtained as follows:
+
+offset = l1_table[offset / cluster_size] + (offset % cluster_size)

It might help to add granularity to this formula.

Instead of offset, bit_number or bitnr might be clearer since
offset means something different in other parts of the document.

Hmm. In my opinion, the bitmap here is stored as raw data. And granularity
is an additional parameter (for deserializing this data). So, it is an
offset in bytes for this data. The format is not for accessing bitmap bits,
it's only for loading the whole bitmap one time.

You are right, it wasn't clear when I read this the first time.  My
problem was the offset into the bitmap doesn't have any units.  So
let's make this more explicit.  Can you document how to go from a bit
number down to the offset?



--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-11 Thread Vladimir Sementsov-Ogievskiy 

On 10.06.2015 18:34, Kevin Wolf wrote:

Am 08.06.2015 um 17:21 hat Vladimir Sementsov-Ogievskiy geschrieben:

From: Vladimir Sementsov-Ogievskiy vsement...@parallels.com

Persistent dirty bitmaps will be saved into qcow2 files. It may be used
as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
other drives (there may be qcow2 file with zero disk size but with
several dirty bitmaps for other drives).

Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@virtuozzo.com
---
  docs/specs/qcow2.txt | 66 
  1 file changed, 66 insertions(+)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 121dfc8..0fffba2 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -123,6 +123,7 @@ be stored. Each extension has a structure like the 
following:
  0x - End of the header extension area
  0xE2792ACA - Backing file format name
  0x6803f857 - Feature name table
+0x23852875 - Dirty bitmaps
  other  - Unknown header extension, can be safely
   ignored
  
@@ -166,6 +167,19 @@ the header extension data. Each entry look like this:

  terminated if it has full length)
  
  
+== Dirty bitmaps ==

+
+Dirty bitmaps is an optional header extension. It provides a possibility of
+storing dirty bitmaps in qcow2 image. The fields are:
+
+  0 -  3:  nb_dirty_bitmaps
+   Number of dirty bitmaps contained in the image
+
+  4 - 11:  dirty_bitmaps_offset
+   Offset into the image file at which the dirty bitmaps table
+   starts. Must be aligned to a cluster boundary.
+
+
  == Host cluster management ==

You need to use a compatibility flag because for old qemu versions, the
dirty bitmaps (and associated metadata) are leaked clusters and qemu-img
check would repair them by resetting the refcount to 0.

At second sight, I see that your patches add an autoclear flag.
Presumably the contents of the dirty bitmaps is outdated when you
accessed the image with an older version, so this seems right. We just
need to document it.


  qcow2 manages the allocation of host clusters by maintaining a reference count
@@ -360,3 +374,55 @@ Snapshot table entry:
  
  variable:   Padding to round up the snapshot table entry size to the

  next multiple of 8.
+
+
+== Dirty bitmaps ==
+
+The feature supports storing several dirty bitmaps in the qcow2 file.
+
+=== Cluster mapping ===
+
+Dirty bitmaps are stored using a ONE-level structure for the mapping of
+bitmaps to host clusters. There is only an L1 table.
+
+The L1 table has a variable size (stored in the Bitmap table entry) and may
+use multiple clusters, however it must be contiguous in the image file.
+
+Given an offset into the bitmap, the offset into the image file can be
+obtained as follows:
+
+offset = l1_table[offset / cluster_size] + (offset % cluster_size)
+
+L1 table entry:
+
+Bit  0 -  61:   Standard cluster descriptor
+
+62 -  63:   Reserved

Stefan already mentioned that we don't have a L1 when there is only
one level, and that you shouldn't reuse the cluster descriptors from L2
tables.


+=== Bitmap table ===
+
+A directory of all bitmaps is stored in the bitmap table, a contiguous area in
+the image file, whose starting offset and length are given by the header fields
+dirty_bitmaps_offset and nb_dirty_bitmaps. The entries of the bitmap table have
+variable length, depending on the length of name and extra data.
+
+Bitmap table entry:
+
+Byte 0 -  7:Offset into the image file at which the L1 table for the
+bitmap starts. Must be aligned to a cluster boundary.
+
+ 8 - 11:Number of entries in the L1 table of the bitmap

Worth using 64 bits here? This can only cover 4 * 512 GB = 2 TB for the
smallest possible cluster size. Though it's 65536 * 512 = 32 PB for the
default, which might be enough for a while.


+12 - 15:Bitmap granularity in bytes
+
+16 - 23:Bitmap size in sectors

Please don't use sectors, that's a meaningless unit. Bytes is better.
Just bad description. Actually it is ~ (number of bits in bitmap * 
granularity), and it is corresponding to number of sectors in the image.



+24 - 25:Size of the bitmap name

We should use a smaller limit than the possible 64k to avoid too large
memory allocations. Nobody needs really long bitmap names.


+
+variable:   The name of the bitmap (not null terminated)
+
+variable:   Padding to round up the bitmap table entry size to the
+next multiple of 8.

Kevin



--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-11 Thread Stefan Hajnoczi 
On Thu, Jun 11, 2015 at 01:19:24PM +0300, Vladimir Sementsov-Ogievskiy wrote:
 On 10.06.2015 16:24, Stefan Hajnoczi wrote:
 On Wed, Jun 10, 2015 at 11:19:30AM +0300, Vladimir Sementsov-Ogievskiy wrote:
 On 09.06.2015 20:03, Stefan Hajnoczi wrote:
 On Mon, Jun 08, 2015 at 06:21:19PM +0300, Vladimir Sementsov-Ogievskiy 
 wrote:
 @@ -166,6 +167,19 @@ the header extension data. Each entry look like this:
   terminated if it has full length)
 +== Dirty bitmaps ==
 +
 +Dirty bitmaps is an optional header extension. It provides a possibility 
 of
 +storing dirty bitmaps in qcow2 image. The fields are:
 +
 +  0 -  3:  nb_dirty_bitmaps
 +   Number of dirty bitmaps contained in the image
 Is there a maximum?
 hmm. any proposals for this?
 65535 seems practical.
 
 So, you suggest to reduce this field width to 2b? And additional 2 bytes
 reserved field, to achieve 8b-alignment?

No, I would leave it 32-bit but impose a little (which can be increased
later if necessary).  That's how nb_snapshots works too.


pgpUiTMGn7_FP.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-11 Thread John Snow 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256



On 06/11/2015 09:03 AM, Stefan Hajnoczi wrote:
 On Thu, Jun 11, 2015 at 01:19:24PM +0300, Vladimir
 Sementsov-Ogievskiy wrote:
 On 10.06.2015 16:24, Stefan Hajnoczi wrote:
 On Wed, Jun 10, 2015 at 11:19:30AM +0300, Vladimir
 Sementsov-Ogievskiy wrote:
 On 09.06.2015 20:03, Stefan Hajnoczi wrote:
 On Mon, Jun 08, 2015 at 06:21:19PM +0300, Vladimir
 Sementsov-Ogievskiy wrote:
 @@ -166,6 +167,19 @@ the header extension data. Each
 entry look like this: terminated if it has full length) 
 +== Dirty bitmaps == + +Dirty bitmaps is an optional
 header extension. It provides a possibility of +storing
 dirty bitmaps in qcow2 image. The fields are: + +
 0 -  3:  nb_dirty_bitmaps +   Number of
 dirty bitmaps contained in the image
 Is there a maximum?
 hmm. any proposals for this?
 65535 seems practical.
 
 So, you suggest to reduce this field width to 2b? And additional
 2 bytes reserved field, to achieve 8b-alignment?
 
 No, I would leave it 32-bit but impose a little (which can be
 increased later if necessary).  That's how nb_snapshots works too.
 

Doesn't the code already limit the number of bitmaps via +#define
QCOW_MAX_DIRTY_BITMAPS 65536, from patch 2?
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQIcBAEBCAAGBQJVebWLAAoJEH3vgQaq/DkOJCYP/jUSJT+jhb3+GvAtddCssyYR
u1BHZacXyTsDTwX4WDZQ5eGEZJZeSwu7++w5N+m+62yDxervarfEE0G/nuGRSNWx
0zYF0RrlYZFdDqed18rgXJJjCtNo1jp67ojk+xpEBUMx9cgFa6s+BTkrY0h+4hiO
V3mvU0H1+8by1Ss5lvziKCHdrksGyBIS4gw+WZNshdOc46/nBZfSlh6CWmtOO/5S
XZwXLKE7QMJMzigdcLJBOlymRwnF094Myklf8fZQILgbdoHoKhEEj9gVWkSpoNk9
FkMDDS1qN5vtYy5Ehzwy9QpbsN5ZEhuHoj5N8k0vDfFHgB9KKvOChvxf2lVhgbz7
fvGpqUb4eEdTvRno9V+8KoEcs99JXLvhed8LrfcZzq05WKbLeAdXYj18QrDw8pdY
Fl4kV5Ca4dpvDAcNZDlCKERv+STLh56hYXEYtjzNEXL+ryQwUyHetY/M6Qodq0j2
FtJq21aj68vEOovQQcX2QxqRxkPzDEvNPbM+phBOh2FjQkbvB6I5bs/ueloyi2q9
UtXWhR6ImUgA6LN25OIc6GS9xYJsFiQlLh1uI/bJoDEpQvVnMojAXE7SohyTya89
2+HIGJsdkbBZsc4SN1INqcsRCeN1at8KiwdIbAijrciF9WIsv0kUEvCvmA93UVYp
s2Os9g5QgMXrK1icCK5J
=CIuZ
-END PGP SIGNATURE-

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-11 Thread John Snow 


On 06/11/2015 06:25 AM, Vladimir Sementsov-Ogievskiy wrote:
 On 10.06.2015 18:34, Kevin Wolf wrote:
 Am 08.06.2015 um 17:21 hat Vladimir Sementsov-Ogievskiy geschrieben:
 From: Vladimir Sementsov-Ogievskiy vsement...@parallels.com

 Persistent dirty bitmaps will be saved into qcow2 files. It may be used
 as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
 other drives (there may be qcow2 file with zero disk size but with
 several dirty bitmaps for other drives).

 Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@virtuozzo.com
 ---
   docs/specs/qcow2.txt | 66
 
   1 file changed, 66 insertions(+)

 diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
 index 121dfc8..0fffba2 100644
 --- a/docs/specs/qcow2.txt
 +++ b/docs/specs/qcow2.txt
 @@ -123,6 +123,7 @@ be stored. Each extension has a structure like
 the following:
   0x - End of the header extension area
   0xE2792ACA - Backing file format name
   0x6803f857 - Feature name table
 +0x23852875 - Dirty bitmaps
   other  - Unknown header extension, can
 be safely
ignored
   @@ -166,6 +167,19 @@ the header extension data. Each entry look
 like this:
   terminated if it has full length)
 +== Dirty bitmaps ==
 +
 +Dirty bitmaps is an optional header extension. It provides a
 possibility of
 +storing dirty bitmaps in qcow2 image. The fields are:
 +
 +  0 -  3:  nb_dirty_bitmaps
 +   Number of dirty bitmaps contained in the image
 +
 +  4 - 11:  dirty_bitmaps_offset
 +   Offset into the image file at which the dirty
 bitmaps table
 +   starts. Must be aligned to a cluster boundary.
 +
 +
   == Host cluster management ==
 You need to use a compatibility flag because for old qemu versions, the
 dirty bitmaps (and associated metadata) are leaked clusters and qemu-img
 check would repair them by resetting the refcount to 0.

 At second sight, I see that your patches add an autoclear flag.
 Presumably the contents of the dirty bitmaps is outdated when you
 accessed the image with an older version, so this seems right. We just
 need to document it.

   qcow2 manages the allocation of host clusters by maintaining a
 reference count
 @@ -360,3 +374,55 @@ Snapshot table entry:
 variable:   Padding to round up the snapshot table entry
 size to the
   next multiple of 8.
 +
 +
 +== Dirty bitmaps ==
 +
 +The feature supports storing several dirty bitmaps in the qcow2 file.
 +
 +=== Cluster mapping ===
 +
 +Dirty bitmaps are stored using a ONE-level structure for the mapping of
 +bitmaps to host clusters. There is only an L1 table.
 +
 +The L1 table has a variable size (stored in the Bitmap table entry)
 and may
 +use multiple clusters, however it must be contiguous in the image file.
 +
 +Given an offset into the bitmap, the offset into the image file can be
 +obtained as follows:
 +
 +offset = l1_table[offset / cluster_size] + (offset % cluster_size)
 +
 +L1 table entry:
 +
 +Bit  0 -  61:   Standard cluster descriptor
 +
 +62 -  63:   Reserved
 Stefan already mentioned that we don't have a L1 when there is only
 one level, and that you shouldn't reuse the cluster descriptors from L2
 tables.

 +=== Bitmap table ===
 +
 +A directory of all bitmaps is stored in the bitmap table, a
 contiguous area in
 +the image file, whose starting offset and length are given by the
 header fields
 +dirty_bitmaps_offset and nb_dirty_bitmaps. The entries of the bitmap
 table have
 +variable length, depending on the length of name and extra data.
 +
 +Bitmap table entry:
 +
 +Byte 0 -  7:Offset into the image file at which the L1 table
 for the
 +bitmap starts. Must be aligned to a cluster
 boundary.
 +
 + 8 - 11:Number of entries in the L1 table of the bitmap
 Worth using 64 bits here? This can only cover 4 * 512 GB = 2 TB for the
 smallest possible cluster size. Though it's 65536 * 512 = 32 PB for the
 default, which might be enough for a while.

 +12 - 15:Bitmap granularity in bytes
 +
 +16 - 23:Bitmap size in sectors
 Please don't use sectors, that's a meaningless unit. Bytes is better.
 Just bad description. Actually it is ~ (number of bits in bitmap *
 granularity), and it is corresponding to number of sectors in the image.

In defense of this, it does happen to be sectors, but what it /really/
represents is the virtual addressable range of the bitmap (its 'size'),
which just-so-happens to be a sector bitmap.

We could just remove the word sectors entirely, and just flatly call
it the bitmap size -- but this does reveal the internal nature of the
block layer, which uses sector bitmaps.

If you wish, we can rework this field to use bytes

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-10 Thread Vladimir Sementsov-Ogievskiy 

On 09.06.2015 20:03, Stefan Hajnoczi wrote:

On Mon, Jun 08, 2015 at 06:21:19PM +0300, Vladimir Sementsov-Ogievskiy wrote:

From: Vladimir Sementsov-Ogievskiy vsement...@parallels.com

Persistent dirty bitmaps will be saved into qcow2 files. It may be used
as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
other drives (there may be qcow2 file with zero disk size but with
several dirty bitmaps for other drives).

Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@virtuozzo.com
---
  docs/specs/qcow2.txt | 66 
  1 file changed, 66 insertions(+)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 121dfc8..0fffba2 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -123,6 +123,7 @@ be stored. Each extension has a structure like the 
following:
  0x - End of the header extension area
  0xE2792ACA - Backing file format name
  0x6803f857 - Feature name table
+0x23852875 - Dirty bitmaps
  other  - Unknown header extension, can be safely
   ignored
  
@@ -166,6 +167,19 @@ the header extension data. Each entry look like this:

  terminated if it has full length)
  
  
+== Dirty bitmaps ==

+
+Dirty bitmaps is an optional header extension. It provides a possibility of
+storing dirty bitmaps in qcow2 image. The fields are:
+
+  0 -  3:  nb_dirty_bitmaps
+   Number of dirty bitmaps contained in the image

Is there a maximum?

hmm. any proposals for this?



+
+  4 - 11:  dirty_bitmaps_offset
+   Offset into the image file at which the dirty bitmaps table
+   starts. Must be aligned to a cluster boundary.

The autoclear feature bit is undocumented.


  == Host cluster management ==
  
  qcow2 manages the allocation of host clusters by maintaining a reference count

@@ -360,3 +374,55 @@ Snapshot table entry:
  
  variable:   Padding to round up the snapshot table entry size to the

  next multiple of 8.
+
+
+== Dirty bitmaps ==
+
+The feature supports storing several dirty bitmaps in the qcow2 file.
+
+=== Cluster mapping ===
+
+Dirty bitmaps are stored using a ONE-level structure for the mapping of
+bitmaps to host clusters. There is only an L1 table.
+
+The L1 table has a variable size (stored in the Bitmap table entry) and may
+use multiple clusters, however it must be contiguous in the image file.

The use of L1 table could be confusing.  The refcount metadata uses
refcount table and refcount block to describe a one-level table.

I agree. Hmm.. dirty bitmaps table? ok?



+
+Given an offset into the bitmap, the offset into the image file can be
+obtained as follows:
+
+offset = l1_table[offset / cluster_size] + (offset % cluster_size)

It might help to add granularity to this formula.

Instead of offset, bit_number or bitnr might be clearer since
offset means something different in other parts of the document.
Hmm. In my opinion, the bitmap here is stored as raw data. And 
granularity is an additional parameter (for deserializing this data). 
So, it is an offset in bytes for this data. The format is not for 
accessing bitmap bits, it's only for loading the whole bitmap one time.



+
+L1 table entry:
+
+Bit  0 -  61:   Standard cluster descriptor
+
+62 -  63:   Reserved

Do you really want to use the standard cluster descriptor with it's zero
bit?

Since bitmaps don't honor backing files there doesn't seem much point in
using the zero bit, things are simpler if just bits 9-55 are contain the
host cluster offset and 0 means the cluster is unallocated.

By honoring the zero bit there are three states:
1. Zero bit set, read zeroes
2. Zero bit not set, host cluster offset != 0, bits valid
3. Zero bit not set, host cluster offset == 0, unallocated

State 1 is not useful.


+=== Bitmap table ===
+
+A directory of all bitmaps is stored in the bitmap table, a contiguous area in
+the image file, whose starting offset and length are given by the header fields
+dirty_bitmaps_offset and nb_dirty_bitmaps. The entries of the bitmap table have
+variable length, depending on the length of name and extra data.
+
+Bitmap table entry:
+
+Byte 0 -  7:Offset into the image file at which the L1 table for the
+bitmap starts. Must be aligned to a cluster boundary.
+
+ 8 - 11:Number of entries in the L1 table of the bitmap
+
+12 - 15:Bitmap granularity in bytes
+
+16 - 23:Bitmap size in sectors
+
+24 - 25:Size of the bitmap name
+
+variable:   The name of the bitmap (not null terminated)
+
+variable:   Padding to round up the bitmap table entry size to the
+next multiple of 8.
+
+The fields size, granularity and name are corresponding with

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-10 Thread Eric Blake 
On 06/10/2015 02:19 AM, Vladimir Sementsov-Ogievskiy wrote:

 +Dirty bitmaps is an optional header extension. It provides a
 possibility of
 +storing dirty bitmaps in qcow2 image. The fields are:
 +
 +  0 -  3:  nb_dirty_bitmaps
 +   Number of dirty bitmaps contained in the image
 Is there a maximum?
 hmm. any proposals for this?

 +
 +  4 - 11:  dirty_bitmaps_offset

I'm not sure if there is a reasonable cap on the number of dirty
bitmaps; I doubt that anyone will actually supply all 4G possible images
allowed by the four-byte field, but don't have a suggestion on a smaller
limit that doesn't feel arbitrary.

[meta-comment] It's very hard to pick out the new content in your reply
if you do not separate your new text with a newline both before and
after (as I'm doing here).


 +Dirty bitmaps are stored using a ONE-level structure for the mapping of
 +bitmaps to host clusters. There is only an L1 table.
 +
 +The L1 table has a variable size (stored in the Bitmap table entry)
 and may
 +use multiple clusters, however it must be contiguous in the image file.
 The use of L1 table could be confusing.  The refcount metadata uses
 refcount table and refcount block to describe a one-level table.
 I agree. Hmm.. dirty bitmaps table? ok?

dirty bitmaps table works for me, as a name for the one-level table.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-10 Thread Vladimir Sementsov-Ogievskiy 

On 10.06.2015 11:19, Vladimir Sementsov-Ogievskiy wrote:

On 09.06.2015 20:03, Stefan Hajnoczi wrote:
On Mon, Jun 08, 2015 at 06:21:19PM +0300, Vladimir 
Sementsov-Ogievskiy wrote:

From: Vladimir Sementsov-Ogievskiy vsement...@parallels.com

Persistent dirty bitmaps will be saved into qcow2 files. It may be used
as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
other drives (there may be qcow2 file with zero disk size but with
several dirty bitmaps for other drives).

Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@virtuozzo.com
---
  docs/specs/qcow2.txt | 66 


  1 file changed, 66 insertions(+)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 121dfc8..0fffba2 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -123,6 +123,7 @@ be stored. Each extension has a structure like 
the following:

  0x - End of the header extension area
  0xE2792ACA - Backing file format name
  0x6803f857 - Feature name table
+0x23852875 - Dirty bitmaps
  other  - Unknown header extension, can 
be safely

   ignored
  @@ -166,6 +167,19 @@ the header extension data. Each entry look 
like this:

  terminated if it has full length)
+== Dirty bitmaps ==
+
+Dirty bitmaps is an optional header extension. It provides a 
possibility of

+storing dirty bitmaps in qcow2 image. The fields are:
+
+  0 -  3:  nb_dirty_bitmaps
+   Number of dirty bitmaps contained in the image

Is there a maximum?

hmm. any proposals for this?



+
+  4 - 11:  dirty_bitmaps_offset
+   Offset into the image file at which the dirty 
bitmaps table

+   starts. Must be aligned to a cluster boundary.

The autoclear feature bit is undocumented.


  == Host cluster management ==
qcow2 manages the allocation of host clusters by maintaining a 
reference count

@@ -360,3 +374,55 @@ Snapshot table entry:
variable:   Padding to round up the snapshot table entry 
size to the

  next multiple of 8.
+
+
+== Dirty bitmaps ==
+
+The feature supports storing several dirty bitmaps in the qcow2 file.
+
+=== Cluster mapping ===
+
+Dirty bitmaps are stored using a ONE-level structure for the 
mapping of

+bitmaps to host clusters. There is only an L1 table.
+
+The L1 table has a variable size (stored in the Bitmap table entry) 
and may
+use multiple clusters, however it must be contiguous in the image 
file.

The use of L1 table could be confusing.  The refcount metadata uses
refcount table and refcount block to describe a one-level table.

I agree. Hmm.. dirty bitmaps table? ok?

oh, no, bad idea. dirty bitmaps table is other thing))



+
+Given an offset into the bitmap, the offset into the image file can be
+obtained as follows:
+
+offset = l1_table[offset / cluster_size] + (offset % cluster_size)

It might help to add granularity to this formula.

Instead of offset, bit_number or bitnr might be clearer since
offset means something different in other parts of the document.
Hmm. In my opinion, the bitmap here is stored as raw data. And 
granularity is an additional parameter (for deserializing this data). 
So, it is an offset in bytes for this data. The format is not for 
accessing bitmap bits, it's only for loading the whole bitmap one time.



+
+L1 table entry:
+
+Bit  0 -  61:   Standard cluster descriptor
+
+62 -  63:   Reserved

Do you really want to use the standard cluster descriptor with it's zero
bit?

Since bitmaps don't honor backing files there doesn't seem much point in
using the zero bit, things are simpler if just bits 9-55 are contain the
host cluster offset and 0 means the cluster is unallocated.

By honoring the zero bit there are three states:
1. Zero bit set, read zeroes
2. Zero bit not set, host cluster offset != 0, bits valid
3. Zero bit not set, host cluster offset == 0, unallocated

State 1 is not useful.


+=== Bitmap table ===
+
+A directory of all bitmaps is stored in the bitmap table, a 
contiguous area in
+the image file, whose starting offset and length are given by the 
header fields
+dirty_bitmaps_offset and nb_dirty_bitmaps. The entries of the 
bitmap table have

+variable length, depending on the length of name and extra data.
+
+Bitmap table entry:
+
+Byte 0 -  7:Offset into the image file at which the L1 
table for the
+bitmap starts. Must be aligned to a cluster 
boundary.

+
+ 8 - 11:Number of entries in the L1 table of the bitmap
+
+12 - 15:Bitmap granularity in bytes
+
+16 - 23:Bitmap size in sectors
+
+24 - 25:Size of the bitmap name
+
+variable:   The name of the bitmap (not null terminated)
+
+variable:   Padding to round up the bitmap

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-10 Thread Stefan Hajnoczi 
On Wed, Jun 10, 2015 at 11:19:30AM +0300, Vladimir Sementsov-Ogievskiy wrote:
 On 09.06.2015 20:03, Stefan Hajnoczi wrote:
 On Mon, Jun 08, 2015 at 06:21:19PM +0300, Vladimir Sementsov-Ogievskiy wrote:
 @@ -166,6 +167,19 @@ the header extension data. Each entry look like this:
   terminated if it has full length)
 +== Dirty bitmaps ==
 +
 +Dirty bitmaps is an optional header extension. It provides a possibility of
 +storing dirty bitmaps in qcow2 image. The fields are:
 +
 +  0 -  3:  nb_dirty_bitmaps
 +   Number of dirty bitmaps contained in the image
 Is there a maximum?
 hmm. any proposals for this?

65535 seems practical.

 +=== Cluster mapping ===
 +
 +Dirty bitmaps are stored using a ONE-level structure for the mapping of
 +bitmaps to host clusters. There is only an L1 table.
 +
 +The L1 table has a variable size (stored in the Bitmap table entry) and may
 +use multiple clusters, however it must be contiguous in the image file.
 The use of L1 table could be confusing.  The refcount metadata uses
 refcount table and refcount block to describe a one-level table.
 I agree. Hmm.. dirty bitmaps table? ok?

Yes, that is good.

 +
 +Given an offset into the bitmap, the offset into the image file can be
 +obtained as follows:
 +
 +offset = l1_table[offset / cluster_size] + (offset % cluster_size)
 It might help to add granularity to this formula.
 
 Instead of offset, bit_number or bitnr might be clearer since
 offset means something different in other parts of the document.
 Hmm. In my opinion, the bitmap here is stored as raw data. And granularity
 is an additional parameter (for deserializing this data). So, it is an
 offset in bytes for this data. The format is not for accessing bitmap bits,
 it's only for loading the whole bitmap one time.

You are right, it wasn't clear when I read this the first time.  My
problem was the offset into the bitmap doesn't have any units.  So
let's make this more explicit.  Can you document how to go from a bit
number down to the offset?


pgpRn6fpG8jF7.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-10 Thread Kevin Wolf 
Am 08.06.2015 um 17:21 hat Vladimir Sementsov-Ogievskiy geschrieben:
 From: Vladimir Sementsov-Ogievskiy vsement...@parallels.com
 
 Persistent dirty bitmaps will be saved into qcow2 files. It may be used
 as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
 other drives (there may be qcow2 file with zero disk size but with
 several dirty bitmaps for other drives).
 
 Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@virtuozzo.com
 ---
  docs/specs/qcow2.txt | 66 
 
  1 file changed, 66 insertions(+)
 
 diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
 index 121dfc8..0fffba2 100644
 --- a/docs/specs/qcow2.txt
 +++ b/docs/specs/qcow2.txt
 @@ -123,6 +123,7 @@ be stored. Each extension has a structure like the 
 following:
  0x - End of the header extension area
  0xE2792ACA - Backing file format name
  0x6803f857 - Feature name table
 +0x23852875 - Dirty bitmaps
  other  - Unknown header extension, can be safely
   ignored
  
 @@ -166,6 +167,19 @@ the header extension data. Each entry look like this:
  terminated if it has full length)
  
  
 +== Dirty bitmaps ==
 +
 +Dirty bitmaps is an optional header extension. It provides a possibility of
 +storing dirty bitmaps in qcow2 image. The fields are:
 +
 +  0 -  3:  nb_dirty_bitmaps
 +   Number of dirty bitmaps contained in the image
 +
 +  4 - 11:  dirty_bitmaps_offset
 +   Offset into the image file at which the dirty bitmaps 
 table
 +   starts. Must be aligned to a cluster boundary.
 +
 +
  == Host cluster management ==

You need to use a compatibility flag because for old qemu versions, the
dirty bitmaps (and associated metadata) are leaked clusters and qemu-img
check would repair them by resetting the refcount to 0.

At second sight, I see that your patches add an autoclear flag.
Presumably the contents of the dirty bitmaps is outdated when you
accessed the image with an older version, so this seems right. We just
need to document it.

  qcow2 manages the allocation of host clusters by maintaining a reference 
 count
 @@ -360,3 +374,55 @@ Snapshot table entry:
  
  variable:   Padding to round up the snapshot table entry size to the
  next multiple of 8.
 +
 +
 +== Dirty bitmaps ==
 +
 +The feature supports storing several dirty bitmaps in the qcow2 file.
 +
 +=== Cluster mapping ===
 +
 +Dirty bitmaps are stored using a ONE-level structure for the mapping of
 +bitmaps to host clusters. There is only an L1 table.
 +
 +The L1 table has a variable size (stored in the Bitmap table entry) and may
 +use multiple clusters, however it must be contiguous in the image file.
 +
 +Given an offset into the bitmap, the offset into the image file can be
 +obtained as follows:
 +
 +offset = l1_table[offset / cluster_size] + (offset % cluster_size)
 +
 +L1 table entry:
 +
 +Bit  0 -  61:   Standard cluster descriptor
 +
 +62 -  63:   Reserved

Stefan already mentioned that we don't have a L1 when there is only
one level, and that you shouldn't reuse the cluster descriptors from L2
tables.

 +=== Bitmap table ===
 +
 +A directory of all bitmaps is stored in the bitmap table, a contiguous area 
 in
 +the image file, whose starting offset and length are given by the header 
 fields
 +dirty_bitmaps_offset and nb_dirty_bitmaps. The entries of the bitmap table 
 have
 +variable length, depending on the length of name and extra data.
 +
 +Bitmap table entry:
 +
 +Byte 0 -  7:Offset into the image file at which the L1 table for the
 +bitmap starts. Must be aligned to a cluster boundary.
 +
 + 8 - 11:Number of entries in the L1 table of the bitmap

Worth using 64 bits here? This can only cover 4 * 512 GB = 2 TB for the
smallest possible cluster size. Though it's 65536 * 512 = 32 PB for the
default, which might be enough for a while.

 +12 - 15:Bitmap granularity in bytes
 +
 +16 - 23:Bitmap size in sectors

Please don't use sectors, that's a meaningless unit. Bytes is better.

 +24 - 25:Size of the bitmap name

We should use a smaller limit than the possible 64k to avoid too large
memory allocations. Nobody needs really long bitmap names.

 +
 +variable:   The name of the bitmap (not null terminated)
 +
 +variable:   Padding to round up the bitmap table entry size to the
 +next multiple of 8.

Kevin

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-09 Thread John Snow 


On 06/08/2015 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
 From: Vladimir Sementsov-Ogievskiy vsement...@parallels.com
 
 Persistent dirty bitmaps will be saved into qcow2 files. It may be used
 as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
 other drives (there may be qcow2 file with zero disk size but with
 several dirty bitmaps for other drives).
 
 Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@virtuozzo.com
 ---
  docs/specs/qcow2.txt | 66 
 
  1 file changed, 66 insertions(+)
 
 diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
 index 121dfc8..0fffba2 100644
 --- a/docs/specs/qcow2.txt
 +++ b/docs/specs/qcow2.txt
 @@ -123,6 +123,7 @@ be stored. Each extension has a structure like the 
 following:
  0x - End of the header extension area
  0xE2792ACA - Backing file format name
  0x6803f857 - Feature name table
 +0x23852875 - Dirty bitmaps
  other  - Unknown header extension, can be safely
   ignored
  
 @@ -166,6 +167,19 @@ the header extension data. Each entry look like this:
  terminated if it has full length)
  
  
 +== Dirty bitmaps ==
 +
 +Dirty bitmaps is an optional header extension. It provides a possibility of
 +storing dirty bitmaps in qcow2 image. The fields are:
 +

I would say It provides the ability to store dirty bitmaps in a qcow2
image. We have more than the possibility now :)

 +  0 -  3:  nb_dirty_bitmaps
 +   Number of dirty bitmaps contained in the image
 +

Most fields seem to be documented as complete sentences, so maybe The
number of dirty bitmaps contained in the image. and add the period.

 +  4 - 11:  dirty_bitmaps_offset
 +   Offset into the image file at which the dirty bitmaps 
 table
 +   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 +374,55 @@ Snapshot table entry:
  
  variable:   Padding to round up the snapshot table entry size to the
  next multiple of 8.
 +
 +
 +== Dirty bitmaps ==
 +
 +The feature supports storing several dirty bitmaps in the qcow2 file.

You could drop several from this sentence, we support an arbitrary number.

 +
 +=== Cluster mapping ===
 +
 +Dirty bitmaps are stored using a ONE-level structure for the mapping of
 +bitmaps to host clusters. There is only an L1 table.
 +
 +The L1 table has a variable size (stored in the Bitmap table entry) and may
 +use multiple clusters, however it must be contiguous in the image file.
 +
 +Given an offset into the bitmap, the offset into the image file can be
 +obtained as follows:
 +
 +offset = l1_table[offset / cluster_size] + (offset % cluster_size)
 +
 +L1 table entry:
 +
 +Bit  0 -  61:   Standard cluster descriptor
 +
 +62 -  63:   Reserved
 +
 +=== Bitmap table ===
 +
 +A directory of all bitmaps is stored in the bitmap table, a contiguous area 
 in
 +the image file, whose starting offset and length are given by the header 
 fields
 +dirty_bitmaps_offset and nb_dirty_bitmaps. The entries of the bitmap table 
 have
 +variable length, depending on the length of name and extra data.
 +
 +Bitmap table entry:
 +
 +Byte 0 -  7:Offset into the image file at which the L1 table for the
 +bitmap starts. Must be aligned to a cluster boundary.
 +
 + 8 - 11:Number of entries in the L1 table of the bitmap
 +
 +12 - 15:Bitmap granularity in bytes
 +
 +16 - 23:Bitmap size in sectors
 +
 +24 - 25:Size of the bitmap name
 +

This is, believe it or not, the first place in code that I am aware of
that actually places a limit on how big a bitmap name can be (64K!) --
we should probably clamp this value to something even lower (1024 is
probably graciously sufficient) and enforce that in the various bitmap
add/create routines.

 +variable:   The name of the bitmap (not null terminated)
 +
 +variable:   Padding to round up the bitmap table entry size to the
 +next multiple of 8.
 +
 +The fields size, granularity and name are corresponding with the fields
 +in struct BdrvDirtyBitmap.
 

Not yet being intricately familiar with qcow2, this looks good to me.

--js

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-06-09 Thread Stefan Hajnoczi 
On Mon, Jun 08, 2015 at 06:21:19PM +0300, Vladimir Sementsov-Ogievskiy wrote:
 From: Vladimir Sementsov-Ogievskiy vsement...@parallels.com
 
 Persistent dirty bitmaps will be saved into qcow2 files. It may be used
 as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
 other drives (there may be qcow2 file with zero disk size but with
 several dirty bitmaps for other drives).
 
 Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@virtuozzo.com
 ---
  docs/specs/qcow2.txt | 66 
 
  1 file changed, 66 insertions(+)
 
 diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
 index 121dfc8..0fffba2 100644
 --- a/docs/specs/qcow2.txt
 +++ b/docs/specs/qcow2.txt
 @@ -123,6 +123,7 @@ be stored. Each extension has a structure like the 
 following:
  0x - End of the header extension area
  0xE2792ACA - Backing file format name
  0x6803f857 - Feature name table
 +0x23852875 - Dirty bitmaps
  other  - Unknown header extension, can be safely
   ignored
  
 @@ -166,6 +167,19 @@ the header extension data. Each entry look like this:
  terminated if it has full length)
  
  
 +== Dirty bitmaps ==
 +
 +Dirty bitmaps is an optional header extension. It provides a possibility of
 +storing dirty bitmaps in qcow2 image. The fields are:
 +
 +  0 -  3:  nb_dirty_bitmaps
 +   Number of dirty bitmaps contained in the image

Is there a maximum?

 +
 +  4 - 11:  dirty_bitmaps_offset
 +   Offset into the image file at which the dirty bitmaps 
 table
 +   starts. Must be aligned to a cluster boundary.

The autoclear feature bit is undocumented.

  == Host cluster management ==
  
  qcow2 manages the allocation of host clusters by maintaining a reference 
 count
 @@ -360,3 +374,55 @@ Snapshot table entry:
  
  variable:   Padding to round up the snapshot table entry size to the
  next multiple of 8.
 +
 +
 +== Dirty bitmaps ==
 +
 +The feature supports storing several dirty bitmaps in the qcow2 file.
 +
 +=== Cluster mapping ===
 +
 +Dirty bitmaps are stored using a ONE-level structure for the mapping of
 +bitmaps to host clusters. There is only an L1 table.
 +
 +The L1 table has a variable size (stored in the Bitmap table entry) and may
 +use multiple clusters, however it must be contiguous in the image file.

The use of L1 table could be confusing.  The refcount metadata uses
refcount table and refcount block to describe a one-level table.

 +
 +Given an offset into the bitmap, the offset into the image file can be
 +obtained as follows:
 +
 +offset = l1_table[offset / cluster_size] + (offset % cluster_size)

It might help to add granularity to this formula.

Instead of offset, bit_number or bitnr might be clearer since
offset means something different in other parts of the document.

 +
 +L1 table entry:
 +
 +Bit  0 -  61:   Standard cluster descriptor
 +
 +62 -  63:   Reserved

Do you really want to use the standard cluster descriptor with it's zero
bit?

Since bitmaps don't honor backing files there doesn't seem much point in
using the zero bit, things are simpler if just bits 9-55 are contain the
host cluster offset and 0 means the cluster is unallocated.

By honoring the zero bit there are three states:
1. Zero bit set, read zeroes
2. Zero bit not set, host cluster offset != 0, bits valid
3. Zero bit not set, host cluster offset == 0, unallocated

State 1 is not useful.

 +=== Bitmap table ===
 +
 +A directory of all bitmaps is stored in the bitmap table, a contiguous area 
 in
 +the image file, whose starting offset and length are given by the header 
 fields
 +dirty_bitmaps_offset and nb_dirty_bitmaps. The entries of the bitmap table 
 have
 +variable length, depending on the length of name and extra data.
 +
 +Bitmap table entry:
 +
 +Byte 0 -  7:Offset into the image file at which the L1 table for the
 +bitmap starts. Must be aligned to a cluster boundary.
 +
 + 8 - 11:Number of entries in the L1 table of the bitmap
 +
 +12 - 15:Bitmap granularity in bytes
 +
 +16 - 23:Bitmap size in sectors
 +
 +24 - 25:Size of the bitmap name
 +
 +variable:   The name of the bitmap (not null terminated)
 +
 +variable:   Padding to round up the bitmap table entry size to the
 +next multiple of 8.
 +
 +The fields size, granularity and name are corresponding with the fields
 +in struct BdrvDirtyBitmap.

Referring to the internals of a C struct in QEMU is not appropriate for
a file format specification.  Please document the fields fully including
their constraints, minimums, maximums, etc.


pgpLOgB4r5VWF.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification

2015-01-27 Thread Eric Blake 
On 01/13/2015 10:02 AM, Vladimir Sementsov-Ogievskiy wrote:
 Persistent dirty bitmaps will be saved into qcow2 files. It may be used
 as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
 other drives (there may be qcow2 file with zero disk size but with
 several dirty bitmaps for other drives).
 
 Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@parallels.com
 ---
  docs/specs/qcow2.txt | 59 
 
  1 file changed, 59 insertions(+)
 
 diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
 index 121dfc8..b29de40 100644
 --- a/docs/specs/qcow2.txt
 +++ b/docs/specs/qcow2.txt
 @@ -116,6 +116,13 @@ in the description of a field.
  Length of the header structure in bytes. For version 2
  images, the length is always assumed to be 72 bytes.
  
 +104 - 107:  nb_dirty_bitmaps
 +Number of dirty bitmaps contained in the image
 +
 +108 - 115:  dirty_bitmaps_offset
 +Offset into the image file at which the dirty bitmaps 
 table
 +starts. Must be aligned to a cluster boundary.

NACK.  You cannot add new fields directly into the header without
bumping the version number; but that is overkill.  Instead, stick the
dirty bitmaps into a new header extension.  The existing fields of the
header are sufficient to record that a new extension is present.  Also,
you need to decide whether dirty bitmaps are an ignorable feature or a
mandatory feature, and set the appropriate feature bit in the header
flag fields (thus when an older qemu that doesn't know about dirty
bitmaps opens the file, it will either reject the open due to not
knowing how to keep dirty bitmaps sane, or will accept the open but
clear the ignorable bit so that the next time a new qemu opens the file
it will know that the dirty bitmap is no longer reliable).

 +=== Cluster mapping ===
 +
 +Dirty bitmaps are stored using a ONE-level structure for the mapping of
 +bitmaps to host clusters. There only an L1 table.

s/There/There is/

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature