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 <vsement...@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? >> > >
