From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33725) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z3BWP-0005Yu-P7 for qemu-devel@nongnu.org; Thu, 11 Jun 2015 19:04:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z3BWM-00051z-GY for qemu-devel@nongnu.org; Thu, 11 Jun 2015 19:04:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45764) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z3BWM-00051m-3A for qemu-devel@nongnu.org; Thu, 11 Jun 2015 19:04:26 -0400 Message-ID: <557A13F7.6090101@redhat.com> Date: Thu, 11 Jun 2015 19:04:23 -0400 From: John Snow MIME-Version: 1.0 References: <1433776886-27239-1-git-send-email-vsementsov@virtuozzo.com> <1433776886-27239-3-git-send-email-vsementsov@virtuozzo.com> In-Reply-To: <1433776886-27239-3-git-send-email-vsementsov@virtuozzo.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/8] qcow2: add dirty-bitmaps feature List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org Cc: kwolf@redhat.com, pbonzini@redhat.com, Vladimir Sementsov-Ogievskiy , stefanha@redhat.com, den@openvz.org On 06/08/2015 11:21 AM, Vladimir Sementsov-Ogievskiy wrote: > From: Vladimir Sementsov-Ogievskiy >=20 > Adds dirty-bitmaps feature to qcow2 format as specified in > docs/specs/qcow2.txt >=20 > Signed-off-by: Vladimir Sementsov-Ogievskiy > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > block/Makefile.objs | 2 +- > block/qcow2-dirty-bitmap.c | 503 +++++++++++++++++++++++++++++++++++++= ++++++++ > block/qcow2.c | 56 +++++ > block/qcow2.h | 50 +++++ > include/block/block_int.h | 10 + > 5 files changed, 620 insertions(+), 1 deletion(-) > create mode 100644 block/qcow2-dirty-bitmap.c >=20 > diff --git a/block/Makefile.objs b/block/Makefile.objs > index 0d8c2a4..bff12b4 100644 > --- a/block/Makefile.objs > +++ b/block/Makefile.objs > @@ -1,5 +1,5 @@ > block-obj-y +=3D raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o v= vfat.o > -block-obj-y +=3D qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapsh= ot.o qcow2-cache.o > +block-obj-y +=3D qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapsh= ot.o qcow2-cache.o qcow2-dirty-bitmap.o > block-obj-y +=3D qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-clus= ter.o > block-obj-y +=3D qed-check.o > block-obj-$(CONFIG_VHDX) +=3D vhdx.o vhdx-endian.o vhdx-log.o > diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c > new file mode 100644 > index 0000000..bc0167c > --- /dev/null > +++ b/block/qcow2-dirty-bitmap.c > @@ -0,0 +1,503 @@ > +/* > + * Dirty bitmpas for the QCOW version 2 format > + * > + * Copyright (c) 2014-2015 Vladimir Sementsov-Ogievskiy > + * > + * This file is derived from qcow2-snapshot.c, original copyright: > + * Copyright (c) 2004-2006 Fabrice Bellard > + * > + * Permission is hereby granted, free of charge, to any person obtaini= ng a copy > + * of this software and associated documentation files (the "Software"= ), to deal > + * in the Software without restriction, including without limitation t= he rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/o= r sell > + * copies of the Software, and to permit persons to whom the Software = is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be incl= uded in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXP= RESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABI= LITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT S= HALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES O= R OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARI= SING FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALI= NGS IN > + * THE SOFTWARE. > + */ > + > +#include "qemu-common.h" > +#include "block/block_int.h" > +#include "block/qcow2.h" > + > +void qcow2_free_dirty_bitmaps(BlockDriverState *bs) > +{ > + BDRVQcowState *s =3D bs->opaque; > + int i; > + > + for (i =3D 0; i < s->nb_dirty_bitmaps; i++) { > + g_free(s->dirty_bitmaps[i].name); > + } > + g_free(s->dirty_bitmaps); > + s->dirty_bitmaps =3D NULL; > + s->nb_dirty_bitmaps =3D 0; > +} > + > +int qcow2_read_dirty_bitmaps(BlockDriverState *bs) > +{ > + BDRVQcowState *s =3D bs->opaque; > + QCowDirtyBitmapHeader h; > + QCowDirtyBitmap *bm; > + int i, name_size; > + int64_t offset; > + int ret; > + > + if (!s->nb_dirty_bitmaps) { > + s->dirty_bitmaps =3D NULL; > + s->dirty_bitmaps_size =3D 0; > + return 0; > + } > + > + offset =3D s->dirty_bitmaps_offset; > + s->dirty_bitmaps =3D g_new0(QCowDirtyBitmap, s->nb_dirty_bitmaps); > + > + for (i =3D 0; i < s->nb_dirty_bitmaps; i++) { > + /* Read statically sized part of the dirty_bitmap header */ > + offset =3D align_offset(offset, 8); > + ret =3D bdrv_pread(bs->file, offset, &h, sizeof(h)); > + if (ret < 0) { > + goto fail; > + } > + > + offset +=3D sizeof(h); > + bm =3D s->dirty_bitmaps + i; > + bm->l1_table_offset =3D be64_to_cpu(h.l1_table_offset); > + bm->l1_size =3D be32_to_cpu(h.l1_size); > + bm->bitmap_granularity =3D be32_to_cpu(h.bitmap_granularity); > + bm->bitmap_size =3D be64_to_cpu(h.bitmap_size); > + > + name_size =3D be16_to_cpu(h.name_size); > + > + /* Read dirty_bitmap name */ > + bm->name =3D g_malloc(name_size + 1); > + ret =3D bdrv_pread(bs->file, offset, bm->name, name_size); > + if (ret < 0) { > + goto fail; > + } > + offset +=3D name_size; > + bm->name[name_size] =3D '\0'; > + > + if (offset - s->dirty_bitmaps_offset > QCOW_MAX_DIRTY_BITMAPS_= SIZE) { > + ret =3D -EFBIG; > + goto fail; > + } > + } > + > + assert(offset - s->dirty_bitmaps_offset <=3D INT_MAX); > + s->dirty_bitmaps_size =3D offset - s->dirty_bitmaps_offset; > + return 0; > + > +fail: > + qcow2_free_dirty_bitmaps(bs); > + return ret; > +} > + > +/* Add at the end of the file a new table of dirty bitmaps */ > +static int qcow2_write_dirty_bitmaps(BlockDriverState *bs) > +{ > + BDRVQcowState *s =3D bs->opaque; > + QCowDirtyBitmap *bm; > + QCowDirtyBitmapHeader h; > + int i, name_size, dirty_bitmaps_size; > + int64_t offset, dirty_bitmaps_offset =3D 0; > + int ret; > + > + int old_dirty_bitmaps_size =3D s->dirty_bitmaps_size; > + int64_t old_dirty_bitmaps_offset =3D s->dirty_bitmaps_offset; > + > + /* Compute the size of the dirty bitmaps table */ > + offset =3D 0; > + for (i =3D 0; i < s->nb_dirty_bitmaps; i++) { > + bm =3D s->dirty_bitmaps + i; > + offset =3D align_offset(offset, 8); > + offset +=3D sizeof(h); > + offset +=3D strlen(bm->name); > + > + if (offset > QCOW_MAX_DIRTY_BITMAPS_SIZE) { > + ret =3D -EFBIG; > + goto fail; > + } > + } > + > + assert(offset <=3D INT_MAX); > + dirty_bitmaps_size =3D offset; > + > + /* Allocate space for the new dirty bitmap table */ > + dirty_bitmaps_offset =3D qcow2_alloc_clusters(bs, dirty_bitmaps_si= ze); > + offset =3D dirty_bitmaps_offset; > + if (offset < 0) { > + ret =3D offset; > + goto fail; > + } > + ret =3D bdrv_flush(bs); > + if (ret < 0) { > + goto fail; > + } > + > + /* The dirty bitmap table position has not yet been updated, so th= ese > + * clusters must indeed be completely free */ > + ret =3D qcow2_pre_write_overlap_check(bs, 0, offset, dirty_bitmaps= _size); > + if (ret < 0) { > + goto fail; > + } > + > + /* Write all dirty bitmaps to the new table */ > + for (i =3D 0; i < s->nb_dirty_bitmaps; i++) { > + bm =3D s->dirty_bitmaps + i; > + memset(&h, 0, sizeof(h)); > + h.l1_table_offset =3D cpu_to_be64(bm->l1_table_offset); > + h.l1_size =3D cpu_to_be32(bm->l1_size); > + h.bitmap_granularity =3D cpu_to_be32(bm->bitmap_granularity); > + h.bitmap_size =3D cpu_to_be64(bm->bitmap_size); > + > + name_size =3D strlen(bm->name); > + assert(name_size <=3D UINT16_MAX); > + h.name_size =3D cpu_to_be16(name_size); > + offset =3D align_offset(offset, 8); > + > + ret =3D bdrv_pwrite(bs->file, offset, &h, sizeof(h)); > + if (ret < 0) { > + goto fail; > + } > + offset +=3D sizeof(h); > + > + ret =3D bdrv_pwrite(bs->file, offset, bm->name, name_size); > + if (ret < 0) { > + goto fail; > + } > + offset +=3D name_size; > + } > + > + /* > + * Update the header extension to point to the new dirty bitmap ta= ble. This > + * requires the new table and its refcounts to be stable on disk. > + */ > + ret =3D bdrv_flush(bs); > + if (ret < 0) { > + goto fail; > + } > + > + s->dirty_bitmaps_offset =3D dirty_bitmaps_offset; > + s->dirty_bitmaps_size =3D dirty_bitmaps_size; > + ret =3D qcow2_update_header(bs); > + if (ret < 0) { > + fprintf(stderr, "Could not update qcow2 header\n"); > + goto fail; > + } > + > + /* Free old dirty bitmap table */ > + qcow2_free_clusters(bs, old_dirty_bitmaps_offset, old_dirty_bitmap= s_size, > + QCOW2_DISCARD_ALWAYS); > + return 0; > + > +fail: > + if (dirty_bitmaps_offset > 0) { > + qcow2_free_clusters(bs, dirty_bitmaps_offset, dirty_bitmaps_si= ze, > + QCOW2_DISCARD_ALWAYS); > + } > + return ret; > +} > + > +static int find_dirty_bitmap_by_name(BlockDriverState *bs, > + const char *name) > +{ > + BDRVQcowState *s =3D bs->opaque; > + int i; > + > + for (i =3D 0; i < s->nb_dirty_bitmaps; i++) { > + if (!strcmp(s->dirty_bitmaps[i].name, name)) { > + return i; > + } > + } > + > + return -1; > +} > + > +uint8_t *qcow2_dirty_bitmap_load(BlockDriverState *bs, > + const char *name, uint64_t size, > + int granularity) > +{ > + BDRVQcowState *s =3D bs->opaque; > + int i, dirty_bitmap_index, ret; > + uint64_t offset; > + QCowDirtyBitmap *bm; > + uint64_t *l1_table; > + uint8_t *buf; > + > + dirty_bitmap_index =3D find_dirty_bitmap_by_name(bs, name); > + if (dirty_bitmap_index < 0) { > + return NULL; > + } > + bm =3D &s->dirty_bitmaps[dirty_bitmap_index]; > + > + if (size !=3D bm->bitmap_size || granularity !=3D bm->bitmap_granu= larity) { > + return NULL; > + } > + > + l1_table =3D g_malloc(bm->l1_size * sizeof(uint64_t)); > + ret =3D bdrv_pread(bs->file, bm->l1_table_offset, l1_table, > + bm->l1_size * sizeof(uint64_t)); > + if (ret < 0) { > + goto fail; > + } > + > + buf =3D g_malloc0(bm->l1_size * s->cluster_size); > + for (i =3D 0; i < bm->l1_size; ++i) { > + offset =3D be64_to_cpu(l1_table[i]); > + if (!(offset & 1)) { > + ret =3D bdrv_pread(bs->file, offset, buf + i * s->cluster_= size, > + s->cluster_size); > + if (ret < 0) { > + goto fail; > + } > + } > + } > + > + g_free(l1_table); > + return buf; > + > +fail: > + g_free(l1_table); > + return NULL; > +} > + > +int qcow2_dirty_bitmap_store(BlockDriverState *bs, uint8_t *buf, > + const char *name, uint64_t size, > + int granularity) > +{ > + BDRVQcowState *s =3D bs->opaque; > + int cl_size =3D s->cluster_size; > + int i, dirty_bitmap_index, ret =3D 0, n; > + uint64_t *l1_table; > + QCowDirtyBitmap *bm; > + uint64_t buf_size; > + uint8_t *p; > + int sector_granularity =3D granularity >> BDRV_SECTOR_BITS; > + > + /* find/create dirty bitmap */ > + dirty_bitmap_index =3D find_dirty_bitmap_by_name(bs, name); > + if (dirty_bitmap_index >=3D 0) { > + bm =3D s->dirty_bitmaps + dirty_bitmap_index; > + > + if (size !=3D bm->bitmap_size || > + granularity !=3D bm->bitmap_granularity) { > + qcow2_dirty_bitmap_delete(bs, name, NULL); > + dirty_bitmap_index =3D -1; > + } > + } > + if (dirty_bitmap_index < 0) { > + qcow2_dirty_bitmap_create(bs, name, size, granularity); > + dirty_bitmap_index =3D s->nb_dirty_bitmaps - 1; > + } > + bm =3D s->dirty_bitmaps + dirty_bitmap_index; I catch a segfault right around here if I do the following: ./x86_64-softmmu/qemu-system-x86_64 --dirty-bitmap file=3Dbitmaps.qcow2,name=3Dbitmap0,drive=3Ddrive0 -drive if=3Dnone,file=3Dhda.qcow2,id=3Ddrive0 -device ide-hd,drive=3Ddrive0 hda.qcow2 and bitmaps.qcow2 are both empty files, but bitmaps.qcow2 has a size of '0'. Then when I click close in the QEMU GTK frontend, we hit a segfault when trying to close because s->dirty_bitmaps is NULL, because it appears as if we've never actually tried to add the (empty) bitmap to the (empty) fi= le. Your iotest works, but I am not actually sure why, because I don't actually know how to *create* a persistent bitmap. I thought that the -dirty-bitmap CLI would create one in the file specified with file=3D, bu= t it apparently only creates an in-memory bitmap and sets the file pointer, but never initializes any of these structures. Then, when we go to close, it gets confused and everything breaks a bit. > + > + /* read l1 table */ > + l1_table =3D g_malloc(bm->l1_size * sizeof(uint64_t)); > + ret =3D bdrv_pread(bs->file, bm->l1_table_offset, l1_table, > + bm->l1_size * sizeof(uint64_t)); > + if (ret < 0) { > + goto finish; > + } > + > + buf_size =3D (((size - 1) / sector_granularity) >> 3) + 1; > + buf_size =3D align_offset(buf_size, 4); > + n =3D buf_size / cl_size; > + p =3D buf; > + for (i =3D 0; i < bm->l1_size; ++i) { > + uint64_t addr =3D be64_to_cpu(l1_table[i]) & ~511; > + int write_size =3D (i =3D=3D n ? (buf_size % cl_size) : cl_siz= e); > + > + if (buffer_is_zero(p, write_size)) { > + if (addr) { > + qcow2_free_clusters(bs, addr, cl_size, > + QCOW2_DISCARD_ALWAYS); > + } > + l1_table[i] =3D cpu_to_be64(1); > + } else { > + if (!addr) { > + addr =3D qcow2_alloc_clusters(bs, cl_size); > + l1_table[i] =3D cpu_to_be64(addr); > + } > + > + ret =3D bdrv_pwrite(bs->file, addr, p, write_size); > + if (ret < 0) { > + goto finish; > + } > + } > + > + p +=3D cl_size; > + } > + > + ret =3D bdrv_pwrite(bs->file, bm->l1_table_offset, l1_table, > + bm->l1_size * sizeof(uint64_t)); > + if (ret < 0) { > + goto finish; > + } > + > +finish: > + g_free(l1_table); > + return ret; > +} > +/* if no id is provided, a new one is constructed */ > +int qcow2_dirty_bitmap_create(BlockDriverState *bs, const char *name, > + uint64_t size, int granularity) > +{ > + BDRVQcowState *s =3D bs->opaque; > + QCowDirtyBitmap *new_dirty_bitmap_list =3D NULL; > + QCowDirtyBitmap *old_dirty_bitmap_list =3D NULL; > + QCowDirtyBitmap sn1, *bm =3D &sn1; > + int i, ret; > + uint64_t *l1_table =3D NULL; > + int64_t l1_table_offset; > + int sector_granularity =3D granularity >> BDRV_SECTOR_BITS; > + > + if (s->nb_dirty_bitmaps >=3D QCOW_MAX_DIRTY_BITMAPS) { > + return -EFBIG; > + } > + > + memset(bm, 0, sizeof(*bm)); > + > + /* Check that the ID is unique */ > + if (find_dirty_bitmap_by_name(bs, name) >=3D 0) { > + return -EEXIST; > + } > + > + /* Populate bm with passed data */ > + bm->name =3D g_strdup(name); > + bm->bitmap_granularity =3D granularity; > + bm->bitmap_size =3D size; > + > + bm->l1_size =3D > + size_to_clusters(s, (((size - 1) / sector_granularity) >> 3) += 1); > + l1_table_offset =3D > + qcow2_alloc_clusters(bs, s->l1_size * sizeof(uint64_t)); > + if (l1_table_offset < 0) { > + ret =3D l1_table_offset; > + goto fail; > + } > + bm->l1_table_offset =3D l1_table_offset; > + > + l1_table =3D g_try_new(uint64_t, bm->l1_size); > + if (l1_table =3D=3D NULL) { > + ret =3D -ENOMEM; > + goto fail; > + } > + > + /* initialize with zero clusters */ > + for (i =3D 0; i < s->l1_size; i++) { > + l1_table[i] =3D cpu_to_be64(1); bm->l1_size here in my crash output is just "1", but s->l1_size is 16, so we crash all over this array. I assume you meant bm->l1_size here. This is a good case to make against calling everything "L1." > + } > + > + ret =3D qcow2_pre_write_overlap_check(bs, 0, bm->l1_table_offset, > + s->l1_size * sizeof(uint64_t))= ; > + if (ret < 0) { > + goto fail; > + } > + > + ret =3D bdrv_pwrite(bs->file, bm->l1_table_offset, l1_table, > + s->l1_size * sizeof(uint64_t)); > + if (ret < 0) { > + goto fail; > + } > + > + g_free(l1_table); I can also catch a segfault here by doing something like this: ./x86_64-softmmu/qemu-system-x86_64 -drive if=3Dnone,format=3Dqcow2,cache=3Dwritethrough,file=3Dhda.qcow2,id=3Ddrive= 0 -dirty-bitmap name=3Dbitmap,drive=3Ddrive0 Trying to mimick your iotest which does not use an external bitmap file -- it uses the implicit self-storage. In this case, hda.qcow2 is still empty (but sized as 8GB) and I try to quit before any writes occur. freeing l1_table here causes memory corruption and even valgrind goes down in flames: =3D=3D13284=3D=3D Invalid write of size 8 =3D=3D13284=3D=3D at 0x53A15E: qcow2_dirty_bitmap_create (qcow2-dirty-bitmap.c:406) =3D=3D13284=3D=3D by 0x539D15: qcow2_dirty_bitmap_store (qcow2-dirty-bitmap.c:307) =3D=3D13284=3D=3D by 0x505F27: bdrv_store_dirty_bitmap (block.c:3176) =3D=3D13284=3D=3D by 0x50306D: bdrv_close (block.c:1739) =3D=3D13284=3D=3D by 0x5032FF: bdrv_close_all (block.c:1797) =3D=3D13284=3D=3D by 0x3049DC: main (vl.c:4577) =3D=3D13284=3D=3D Address 0x239b7978 is 0 bytes after a block of size 8 = alloc'd =3D=3D13284=3D=3D at 0x4A06BCF: malloc (vg_replace_malloc.c:296) =3D=3D13284=3D=3D by 0x300111: malloc_and_trace (vl.c:2706) =3D=3D13284=3D=3D by 0x62B954E: g_try_malloc (gmem.c:242) =3D=3D13284=3D=3D by 0x53A11E: qcow2_dirty_bitmap_create (qcow2-dirty-bitmap.c:398) =3D=3D13284=3D=3D by 0x539D15: qcow2_dirty_bitmap_store (qcow2-dirty-bitmap.c:307) =3D=3D13284=3D=3D by 0x505F27: bdrv_store_dirty_bitmap (block.c:3176) =3D=3D13284=3D=3D by 0x50306D: bdrv_close (block.c:1739) =3D=3D13284=3D=3D by 0x5032FF: bdrv_close_all (block.c:1797) =3D=3D13284=3D=3D by 0x3049DC: main (vl.c:4577) =3D=3D13284=3D=3D --13284-- VALGRIND INTERNAL ERROR: Valgrind received a signal 11 (SIGSEGV) - exiting --13284-- si_code=3D80; Faulting address: 0x0; sp: 0x8090a1de0 valgrind: the 'impossible' happened: Killed by fatal signal > + l1_table =3D NULL; > + > + /* Append the new dirty bitmap to the dirty bitmap list */ > + new_dirty_bitmap_list =3D g_new(QCowDirtyBitmap, s->nb_dirty_bitma= ps + 1); > + if (s->dirty_bitmaps) { > + memcpy(new_dirty_bitmap_list, s->dirty_bitmaps, > + s->nb_dirty_bitmaps * sizeof(QCowDirtyBitmap)); > + old_dirty_bitmap_list =3D s->dirty_bitmaps; > + } > + s->dirty_bitmaps =3D new_dirty_bitmap_list; > + s->dirty_bitmaps[s->nb_dirty_bitmaps++] =3D *bm; > + > + ret =3D qcow2_write_dirty_bitmaps(bs); > + if (ret < 0) { > + g_free(s->dirty_bitmaps); > + s->dirty_bitmaps =3D old_dirty_bitmap_list; > + s->nb_dirty_bitmaps--; > + goto fail; > + } > + > + g_free(old_dirty_bitmap_list); > + > + return 0; > + Disk is 8GiB, 16,777,216 sectors, and bm->bitmap_size matches that. > +fail: > + g_free(bm->name); > + g_free(l1_table); > + > + return ret; > +} > + > +static int qcow2_dirty_bitmap_free_clusters(BlockDriverState *bs, > + QCowDirtyBitmap *bm) > +{ > + BDRVQcowState *s =3D bs->opaque; > + int ret, i; > + uint64_t *l1_table =3D g_new(uint64_t, bm->l1_size); > + > + ret =3D bdrv_pread(bs->file, bm->l1_table_offset, l1_table, > + bm->l1_size * sizeof(uint64_t)); > + if (ret < 0) { > + g_free(l1_table); > + return ret; > + } > + > + for (i =3D 0; i < bm->l1_size; ++i) { > + uint64_t addr =3D be64_to_cpu(l1_table[i]); > + qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_A= LWAYS); > + } > + > + qcow2_free_clusters(bs, bm->l1_table_offset, bm->l1_size * sizeof(= uint64_t), > + QCOW2_DISCARD_ALWAYS); > + > + g_free(l1_table); > + return 0; > +} > + > +int qcow2_dirty_bitmap_delete(BlockDriverState *bs, > + const char *name, > + Error **errp) > +{ > + BDRVQcowState *s =3D bs->opaque; > + QCowDirtyBitmap bm; > + int dirty_bitmap_index, ret =3D 0; > + > + /* Search the dirty_bitmap */ > + dirty_bitmap_index =3D find_dirty_bitmap_by_name(bs, name); > + if (dirty_bitmap_index < 0) { > + error_setg(errp, "Can't find the dirty bitmap"); > + return -ENOENT; > + } > + bm =3D s->dirty_bitmaps[dirty_bitmap_index]; > + > + /* Remove it from the dirty_bitmap list */ > + memmove(s->dirty_bitmaps + dirty_bitmap_index, > + s->dirty_bitmaps + dirty_bitmap_index + 1, > + (s->nb_dirty_bitmaps - dirty_bitmap_index - 1) * sizeof(bm= )); > + s->nb_dirty_bitmaps--; > + ret =3D qcow2_write_dirty_bitmaps(bs); > + if (ret < 0) { > + error_setg_errno(errp, -ret, > + "Failed to remove dirty bitmap" > + " from dirty bitmap list"); > + return ret; > + } > + > + qcow2_dirty_bitmap_free_clusters(bs, &bm); > + g_free(bm.name); > + > + return ret; > +} > diff --git a/block/qcow2.c b/block/qcow2.c > index b9a72e3..406e55d 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -61,6 +61,7 @@ typedef struct { > #define QCOW2_EXT_MAGIC_END 0 > #define QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA > #define QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857 > +#define QCOW2_EXT_MAGIC_DIRTY_BITMAPS 0x23852875 > =20 > static int qcow2_probe(const uint8_t *buf, int buf_size, const char *f= ilename) > { > @@ -90,6 +91,7 @@ static int qcow2_read_extensions(BlockDriverState *bs= , uint64_t start_offset, > QCowExtension ext; > uint64_t offset; > int ret; > + Qcow2DirtyBitmapHeaderExt dirty_bitmaps_ext; > =20 > #ifdef DEBUG_EXT > printf("qcow2_read_extensions: start=3D%ld end=3D%ld\n", start_off= set, end_offset); > @@ -160,6 +162,33 @@ static int qcow2_read_extensions(BlockDriverState = *bs, uint64_t start_offset, > } > break; > =20 > + case QCOW2_EXT_MAGIC_DIRTY_BITMAPS: > + ret =3D bdrv_pread(bs->file, offset, &dirty_bitmaps_ext, e= xt.len); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "ERROR: dirty_bitmaps_ext= : " > + "Could not read ext header"); > + return ret; > + } > + > + be64_to_cpus(&dirty_bitmaps_ext.dirty_bitmaps_offset); > + be32_to_cpus(&dirty_bitmaps_ext.nb_dirty_bitmaps); > + > + s->dirty_bitmaps_offset =3D dirty_bitmaps_ext.dirty_bitmap= s_offset; > + s->nb_dirty_bitmaps =3D dirty_bitmaps_ext.nb_dirty_bitmaps= ; > + > + ret =3D qcow2_read_dirty_bitmaps(bs); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Could not read dirty bit= maps"); > + return ret; > + } > + > +#ifdef DEBUG_EXT > + printf("Qcow2: Got dirty bitmaps extension:" > + " offset=3D%" PRIu64 " nb_bitmaps=3D%" PRIu32 "\n", > + s->dirty_bitmaps_offset, s->nb_dirty_bitmaps); > +#endif > + break; > + > default: > /* unknown magic - save it in case we need to rewrite the = header */ > { > @@ -1000,6 +1029,7 @@ static int qcow2_open(BlockDriverState *bs, QDict= *options, int flags, > g_free(s->unknown_header_fields); > cleanup_unknown_header_ext(bs); > qcow2_free_snapshots(bs); > + qcow2_free_dirty_bitmaps(bs); > qcow2_refcount_close(bs); > qemu_vfree(s->l1_table); > /* else pre-write overlap checks in cache_destroy may crash */ > @@ -1466,6 +1496,7 @@ static void qcow2_close(BlockDriverState *bs) > qemu_vfree(s->cluster_data); > qcow2_refcount_close(bs); > qcow2_free_snapshots(bs); > + qcow2_free_dirty_bitmaps(bs); > } > =20 > static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) > @@ -1667,6 +1698,21 @@ int qcow2_update_header(BlockDriverState *bs) > buf +=3D ret; > buflen -=3D ret; > =20 > + if (s->nb_dirty_bitmaps > 0) { > + Qcow2DirtyBitmapHeaderExt dirty_bitmaps_header =3D { > + .nb_dirty_bitmaps =3D cpu_to_be32(s->nb_dirty_bitmaps), > + .dirty_bitmaps_offset =3D cpu_to_be64(s->dirty_bitmaps_off= set) > + }; > + ret =3D header_ext_add(buf, QCOW2_EXT_MAGIC_DIRTY_BITMAPS, > + &dirty_bitmaps_header, sizeof(dirty_bitma= ps_header), > + buflen); > + if (ret < 0) { > + goto fail; > + } > + buf +=3D ret; > + buflen -=3D ret; > + } > + > /* Keep unknown header extensions */ > QLIST_FOREACH(uext, &s->unknown_header_ext, next) { > ret =3D header_ext_add(buf, uext->magic, uext->data, uext->len= , buflen); > @@ -2176,6 +2222,12 @@ static int qcow2_truncate(BlockDriverState *bs, = int64_t offset) > return -ENOTSUP; > } > =20 > + /* cannot proceed if image has dirty_bitmaps */ > + if (s->nb_dirty_bitmaps) { > + error_report("Can't resize an image which has dirty bitmaps"); > + return -ENOTSUP; > + } > + > /* shrinking is currently not supported */ > if (offset < bs->total_sectors * 512) { > error_report("qcow2 doesn't support shrinking images yet"); > @@ -2952,6 +3004,10 @@ BlockDriver bdrv_qcow2 =3D { > .bdrv_get_info =3D qcow2_get_info, > .bdrv_get_specific_info =3D qcow2_get_specific_info, > =20 > + .bdrv_dirty_bitmap_load =3D qcow2_dirty_bitmap_load, > + .bdrv_dirty_bitmap_store =3D qcow2_dirty_bitmap_store, > + .bdrv_dirty_bitmap_delete =3D qcow2_dirty_bitmap_delete, > + > .bdrv_save_vmstate =3D qcow2_save_vmstate, > .bdrv_load_vmstate =3D qcow2_load_vmstate, > =20 > diff --git a/block/qcow2.h b/block/qcow2.h > index 422b825..24beee0 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -39,6 +39,7 @@ > =20 > #define QCOW_MAX_CRYPT_CLUSTERS 32 > #define QCOW_MAX_SNAPSHOTS 65536 > +#define QCOW_MAX_DIRTY_BITMAPS 65536 > =20 > /* 8 MB refcount table is enough for 2 PB images at 64k cluster size > * (128 GB for 512 byte clusters, 2 EB for 2 MB clusters) */ > @@ -52,6 +53,8 @@ > * space for snapshot names and IDs */ > #define QCOW_MAX_SNAPSHOTS_SIZE (1024 * QCOW_MAX_SNAPSHOTS) > =20 > +#define QCOW_MAX_DIRTY_BITMAPS_SIZE (1024 * QCOW_MAX_DIRTY_BITMAPS) > + > /* indicate that the refcount of the referenced cluster is exactly one= . */ > #define QCOW_OFLAG_COPIED (1ULL << 63) > /* indicate that the cluster is compressed (they never have the copied= flag) */ > @@ -138,6 +141,19 @@ typedef struct QEMU_PACKED QCowSnapshotHeader { > /* name follows */ > } QCowSnapshotHeader; > =20 > +typedef struct QEMU_PACKED QCowDirtyBitmapHeader { > + /* header is 8 byte aligned */ > + uint64_t l1_table_offset; > + > + uint32_t l1_size; > + uint32_t bitmap_granularity; > + > + uint64_t bitmap_size; > + uint16_t name_size; > + > + /* name follows */ > +} QCowDirtyBitmapHeader; > + > typedef struct QEMU_PACKED QCowSnapshotExtraData { > uint64_t vm_state_size_large; > uint64_t disk_size; > @@ -156,6 +172,14 @@ typedef struct QCowSnapshot { > uint64_t vm_clock_nsec; > } QCowSnapshot; > =20 > +typedef struct QCowDirtyBitmap { > + uint64_t l1_table_offset; > + uint32_t l1_size; > + char *name; > + int bitmap_granularity; > + uint64_t bitmap_size; > +} QCowDirtyBitmap; > + > struct Qcow2Cache; > typedef struct Qcow2Cache Qcow2Cache; > =20 > @@ -218,6 +242,11 @@ typedef uint64_t Qcow2GetRefcountFunc(const void *= refcount_array, > typedef void Qcow2SetRefcountFunc(void *refcount_array, > uint64_t index, uint64_t value); > =20 > +typedef struct Qcow2DirtyBitmapHeaderExt { > + uint32_t nb_dirty_bitmaps; > + uint64_t dirty_bitmaps_offset; > +} QEMU_PACKED Qcow2DirtyBitmapHeaderExt; > + > typedef struct BDRVQcowState { > int cluster_bits; > int cluster_size; > @@ -259,6 +288,11 @@ typedef struct BDRVQcowState { > unsigned int nb_snapshots; > QCowSnapshot *snapshots; > =20 > + uint64_t dirty_bitmaps_offset; > + int dirty_bitmaps_size; > + unsigned int nb_dirty_bitmaps; > + QCowDirtyBitmap *dirty_bitmaps; > + > int flags; > int qcow_version; > bool use_lazy_refcounts; > @@ -570,6 +604,22 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, > void qcow2_free_snapshots(BlockDriverState *bs); > int qcow2_read_snapshots(BlockDriverState *bs); > =20 > +/* qcow2-dirty-bitmap.c functions */ > +int qcow2_dirty_bitmap_store(BlockDriverState *bs, uint8_t *buf, > + const char *name, uint64_t size, > + int granularity); > +uint8_t *qcow2_dirty_bitmap_load(BlockDriverState *bs, > + const char *name, uint64_t size, > + int granularity); > +int qcow2_dirty_bitmap_create(BlockDriverState *bs, const char *name, > + uint64_t size, int granularity); > +int qcow2_dirty_bitmap_delete(BlockDriverState *bs, > + const char *name, > + Error **errp); > + > +void qcow2_free_dirty_bitmaps(BlockDriverState *bs); > +int qcow2_read_dirty_bitmaps(BlockDriverState *bs); > + > /* qcow2-cache.c functions */ > Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables); > int qcow2_cache_destroy(BlockDriverState* bs, Qcow2Cache *c); > diff --git a/include/block/block_int.h b/include/block/block_int.h > index db29b74..88855b4 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -206,6 +206,16 @@ struct BlockDriver { > int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi); > ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs)= ; > =20 > + int (*bdrv_dirty_bitmap_store)(BlockDriverState *bs, uint8_t *buf, > + const char *name, uint64_t size, > + int granularity); > + uint8_t *(*bdrv_dirty_bitmap_load)(BlockDriverState *bs, > + const char *name, uint64_t size= , > + int granularity); > + int (*bdrv_dirty_bitmap_delete)(BlockDriverState *bs, > + const char *name, > + Error **errp); > + > int (*bdrv_save_vmstate)(BlockDriverState *bs, QEMUIOVector *qiov, > int64_t pos); > int (*bdrv_load_vmstate)(BlockDriverState *bs, uint8_t *buf, >=20 In light of this, some "sanity" tests that test cases like no writes, empty bitmaps, empty files, etc I think will be appropriate.