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

[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