From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39017) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJrHs-00035E-2S for qemu-devel@nongnu.org; Thu, 14 Jan 2016 18:26:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aJrHo-0005xF-RI for qemu-devel@nongnu.org; Thu, 14 Jan 2016 18:26:40 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33148) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJrHo-0005x0-Gv for qemu-devel@nongnu.org; Thu, 14 Jan 2016 18:26:36 -0500 References: <1452517517-3953-1-git-send-email-vsementsov@virtuozzo.com> <56981C60.9020005@redhat.com> From: John Snow Message-ID: <56982EA9.6030602@redhat.com> Date: Thu, 14 Jan 2016 18:26:33 -0500 MIME-Version: 1.0 In-Reply-To: <56981C60.9020005@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v7] spec: add qcow2 bitmaps extension specification List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org Cc: kwolf@redhat.com, den@openvz.org, famz@redhat.com, stefanha@redhat.com, mreitz@redhat.com On 01/14/2016 05:08 PM, Eric Blake wrote: > On 01/11/2016 06:05 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 >> --- >> > >> @@ -166,6 +178,34 @@ the header extension data. Each entry look like this: >> terminated if it has full length) >> >> >> +== Bitmaps extension == > >> + 0 - 3: nb_bitmaps >> + The number of bitmaps contained in the image. Must be >> + greater than 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 the cumulative >> + size of all (nb_bitmaps) bitmap headers. > > Only 4 bytes - if we ever raise our 64k entry restriction (nb_bitmaps), > could we run into an image that has so many directory entries as to make > the directory itself spill past 4G? But I don't think it is likely, so > I can live with your choice. > "We'll never need this!" I hope someone in 2082 is reading this right now and is quite angry. (But really, I can't foresee needing this many per each drive -- and if we do, we have external storage mechanisms in development to handle such wild cases.) >> + >> +== Bitmaps == >> + >> +As mentioned above, the bitmaps extension provides the ability to store bitmaps >> +related a virtual disk. This section describes how these bitmaps are stored. >> + >> +Note: 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. The >> +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. >> + >> +Structure of a 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. > > Should this be the size in bytes, instead of the number of entries? But For what benefit? We can calculate either from the other, and this gives us a better resolution. > at least the entries are fixed width of 8 bytes each, so this lets you > get a bitmap table up to 32G bytes rather than just 4G in size. (Let's > see here - if we have 32G bytes in the bitmap table, that means 4G > clusters occupied by the bitmap itself; in the worst case of 512-byte > clusters and granularity 0, that is a maximum bitmap size of 2T > describing 16T of virtual guest image; with larger cluster size and/or > larger granularity, we cover a lot more virtual guest space with less > bitmap size; so I guess we aren't too worried about running out of space?). > Yes, worst case of g=0 and cluster size of 512 bytes, we can get 2T bitmaps describing 16T of virtual data. "default case" of 64K clusters and 64K granularity: 256TiB bitmaps describing ... let's see ... if my math is right, 128EiB? We're probably fine :) (Cue future space-person from 2159 wondering how I could have ever been so naive. Sorry, future space-person!) >> + 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. > > I'd write this as: > variable: extra_data > Type-specific extra data for the bitmap, > occupying extra_data_size bytes. > >> + >> + 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. > > Should we require the padding to be all NUL bytes? (We aren't > consistent on whether we require that for other locations of padding in > the spec, so that could be a followup patch). > >> + >> +=== Bitmap table === >> + >> +Bitmaps are stored using a one-level structure (as opposed to two-level >> +structure like for refcounts and guest clusters mapping) for the mapping of >> +bitmap data to host clusters. This structure is called the bitmap table. > > Possible wording tweak: > Bitmaps are stored using a one-level structure (as opposed to the > two-level structures for refcounts and guest cluster mapping), and are > used for the mapping of bitmap data to host clusters > >> + >> +Each bitmap table has a variable size (stored in the bitmap directory Entry) > > Does 'Entry' still need to be capitalized? > >> +and may use multiple clusters, however, it must be contiguous in the image >> +file. >> + >> +Structure of a 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 the host cluster offset. Must be aligned to >> + a cluster boundary. If the offset is 0, the cluster is >> + unallocated; in that case, bit 0 determines how this >> + cluster should be treated when read from. > > Possible wording tweak: > s/when read from/during reads/. > >> + >> + 56 - 63: Reserved and must be zero. >> + >> +=== Bitmap data === >> + >> +As noted above, bitmap data is stored in separate clusters, described by the >> +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) >> + >> +This offset is not defined if bits 9 - 55 of bitmap table entry are zero (see >> +above). >> + >> +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 >> + >> +If the size of the bitmap data is not a multiply of cluster size then the last > > s/multiply of cluster size/multiple of the cluster size,/ > >> +cluster of the bitmap data contains some unused tail bits. These bits must be >> +zero. >> > Thanks! --js