From: John Snow <jsnow@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
qemu-devel@nongnu.org
Cc: kwolf@redhat.com, pbonzini@redhat.com,
Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>,
stefanha@redhat.com, den@openvz.org
Subject: Re: [Qemu-devel] [PATCH 2/8] qcow2: add dirty-bitmaps feature
Date: Thu, 11 Jun 2015 19:04:23 -0400 [thread overview]
Message-ID: <557A13F7.6090101@redhat.com> (raw)
In-Reply-To: <1433776886-27239-3-git-send-email-vsementsov@virtuozzo.com>
On 06/08/2015 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
> From: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
>
> Adds dirty-bitmaps feature to qcow2 format as specified in
> docs/specs/qcow2.txt
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> 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
>
> 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 += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o
> -block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o
> +block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o qcow2-dirty-bitmap.o
> block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
> block-obj-y += qed-check.o
> block-obj-$(CONFIG_VHDX) += 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 obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 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 included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 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 = bs->opaque;
> + int i;
> +
> + for (i = 0; i < s->nb_dirty_bitmaps; i++) {
> + g_free(s->dirty_bitmaps[i].name);
> + }
> + g_free(s->dirty_bitmaps);
> + s->dirty_bitmaps = NULL;
> + s->nb_dirty_bitmaps = 0;
> +}
> +
> +int qcow2_read_dirty_bitmaps(BlockDriverState *bs)
> +{
> + BDRVQcowState *s = bs->opaque;
> + QCowDirtyBitmapHeader h;
> + QCowDirtyBitmap *bm;
> + int i, name_size;
> + int64_t offset;
> + int ret;
> +
> + if (!s->nb_dirty_bitmaps) {
> + s->dirty_bitmaps = NULL;
> + s->dirty_bitmaps_size = 0;
> + return 0;
> + }
> +
> + offset = s->dirty_bitmaps_offset;
> + s->dirty_bitmaps = g_new0(QCowDirtyBitmap, s->nb_dirty_bitmaps);
> +
> + for (i = 0; i < s->nb_dirty_bitmaps; i++) {
> + /* Read statically sized part of the dirty_bitmap header */
> + offset = align_offset(offset, 8);
> + ret = bdrv_pread(bs->file, offset, &h, sizeof(h));
> + if (ret < 0) {
> + goto fail;
> + }
> +
> + offset += sizeof(h);
> + bm = s->dirty_bitmaps + i;
> + bm->l1_table_offset = be64_to_cpu(h.l1_table_offset);
> + bm->l1_size = be32_to_cpu(h.l1_size);
> + bm->bitmap_granularity = be32_to_cpu(h.bitmap_granularity);
> + bm->bitmap_size = be64_to_cpu(h.bitmap_size);
> +
> + name_size = be16_to_cpu(h.name_size);
> +
> + /* Read dirty_bitmap name */
> + bm->name = g_malloc(name_size + 1);
> + ret = bdrv_pread(bs->file, offset, bm->name, name_size);
> + if (ret < 0) {
> + goto fail;
> + }
> + offset += name_size;
> + bm->name[name_size] = '\0';
> +
> + if (offset - s->dirty_bitmaps_offset > QCOW_MAX_DIRTY_BITMAPS_SIZE) {
> + ret = -EFBIG;
> + goto fail;
> + }
> + }
> +
> + assert(offset - s->dirty_bitmaps_offset <= INT_MAX);
> + s->dirty_bitmaps_size = 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 = bs->opaque;
> + QCowDirtyBitmap *bm;
> + QCowDirtyBitmapHeader h;
> + int i, name_size, dirty_bitmaps_size;
> + int64_t offset, dirty_bitmaps_offset = 0;
> + int ret;
> +
> + int old_dirty_bitmaps_size = s->dirty_bitmaps_size;
> + int64_t old_dirty_bitmaps_offset = s->dirty_bitmaps_offset;
> +
> + /* Compute the size of the dirty bitmaps table */
> + offset = 0;
> + for (i = 0; i < s->nb_dirty_bitmaps; i++) {
> + bm = s->dirty_bitmaps + i;
> + offset = align_offset(offset, 8);
> + offset += sizeof(h);
> + offset += strlen(bm->name);
> +
> + if (offset > QCOW_MAX_DIRTY_BITMAPS_SIZE) {
> + ret = -EFBIG;
> + goto fail;
> + }
> + }
> +
> + assert(offset <= INT_MAX);
> + dirty_bitmaps_size = offset;
> +
> + /* Allocate space for the new dirty bitmap table */
> + dirty_bitmaps_offset = qcow2_alloc_clusters(bs, dirty_bitmaps_size);
> + offset = dirty_bitmaps_offset;
> + if (offset < 0) {
> + ret = offset;
> + goto fail;
> + }
> + ret = bdrv_flush(bs);
> + if (ret < 0) {
> + goto fail;
> + }
> +
> + /* The dirty bitmap table position has not yet been updated, so these
> + * clusters must indeed be completely free */
> + ret = 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 = 0; i < s->nb_dirty_bitmaps; i++) {
> + bm = s->dirty_bitmaps + i;
> + memset(&h, 0, sizeof(h));
> + h.l1_table_offset = cpu_to_be64(bm->l1_table_offset);
> + h.l1_size = cpu_to_be32(bm->l1_size);
> + h.bitmap_granularity = cpu_to_be32(bm->bitmap_granularity);
> + h.bitmap_size = cpu_to_be64(bm->bitmap_size);
> +
> + name_size = strlen(bm->name);
> + assert(name_size <= UINT16_MAX);
> + h.name_size = cpu_to_be16(name_size);
> + offset = align_offset(offset, 8);
> +
> + ret = bdrv_pwrite(bs->file, offset, &h, sizeof(h));
> + if (ret < 0) {
> + goto fail;
> + }
> + offset += sizeof(h);
> +
> + ret = bdrv_pwrite(bs->file, offset, bm->name, name_size);
> + if (ret < 0) {
> + goto fail;
> + }
> + offset += name_size;
> + }
> +
> + /*
> + * Update the header extension to point to the new dirty bitmap table. This
> + * requires the new table and its refcounts to be stable on disk.
> + */
> + ret = bdrv_flush(bs);
> + if (ret < 0) {
> + goto fail;
> + }
> +
> + s->dirty_bitmaps_offset = dirty_bitmaps_offset;
> + s->dirty_bitmaps_size = dirty_bitmaps_size;
> + ret = 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_bitmaps_size,
> + QCOW2_DISCARD_ALWAYS);
> + return 0;
> +
> +fail:
> + if (dirty_bitmaps_offset > 0) {
> + qcow2_free_clusters(bs, dirty_bitmaps_offset, dirty_bitmaps_size,
> + QCOW2_DISCARD_ALWAYS);
> + }
> + return ret;
> +}
> +
> +static int find_dirty_bitmap_by_name(BlockDriverState *bs,
> + const char *name)
> +{
> + BDRVQcowState *s = bs->opaque;
> + int i;
> +
> + for (i = 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 = bs->opaque;
> + int i, dirty_bitmap_index, ret;
> + uint64_t offset;
> + QCowDirtyBitmap *bm;
> + uint64_t *l1_table;
> + uint8_t *buf;
> +
> + dirty_bitmap_index = find_dirty_bitmap_by_name(bs, name);
> + if (dirty_bitmap_index < 0) {
> + return NULL;
> + }
> + bm = &s->dirty_bitmaps[dirty_bitmap_index];
> +
> + if (size != bm->bitmap_size || granularity != bm->bitmap_granularity) {
> + return NULL;
> + }
> +
> + l1_table = g_malloc(bm->l1_size * sizeof(uint64_t));
> + ret = bdrv_pread(bs->file, bm->l1_table_offset, l1_table,
> + bm->l1_size * sizeof(uint64_t));
> + if (ret < 0) {
> + goto fail;
> + }
> +
> + buf = g_malloc0(bm->l1_size * s->cluster_size);
> + for (i = 0; i < bm->l1_size; ++i) {
> + offset = be64_to_cpu(l1_table[i]);
> + if (!(offset & 1)) {
> + ret = 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 = bs->opaque;
> + int cl_size = s->cluster_size;
> + int i, dirty_bitmap_index, ret = 0, n;
> + uint64_t *l1_table;
> + QCowDirtyBitmap *bm;
> + uint64_t buf_size;
> + uint8_t *p;
> + int sector_granularity = granularity >> BDRV_SECTOR_BITS;
> +
> + /* find/create dirty bitmap */
> + dirty_bitmap_index = find_dirty_bitmap_by_name(bs, name);
> + if (dirty_bitmap_index >= 0) {
> + bm = s->dirty_bitmaps + dirty_bitmap_index;
> +
> + if (size != bm->bitmap_size ||
> + granularity != bm->bitmap_granularity) {
> + qcow2_dirty_bitmap_delete(bs, name, NULL);
> + dirty_bitmap_index = -1;
> + }
> + }
> + if (dirty_bitmap_index < 0) {
> + qcow2_dirty_bitmap_create(bs, name, size, granularity);
> + dirty_bitmap_index = s->nb_dirty_bitmaps - 1;
> + }
> + bm = 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=bitmaps.qcow2,name=bitmap0,drive=drive0 -drive
if=none,file=hda.qcow2,id=drive0 -device ide-hd,drive=drive0
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) file.
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=, but
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 = g_malloc(bm->l1_size * sizeof(uint64_t));
> + ret = bdrv_pread(bs->file, bm->l1_table_offset, l1_table,
> + bm->l1_size * sizeof(uint64_t));
> + if (ret < 0) {
> + goto finish;
> + }
> +
> + buf_size = (((size - 1) / sector_granularity) >> 3) + 1;
> + buf_size = align_offset(buf_size, 4);
> + n = buf_size / cl_size;
> + p = buf;
> + for (i = 0; i < bm->l1_size; ++i) {
> + uint64_t addr = be64_to_cpu(l1_table[i]) & ~511;
> + int write_size = (i == n ? (buf_size % cl_size) : cl_size);
> +
> + if (buffer_is_zero(p, write_size)) {
> + if (addr) {
> + qcow2_free_clusters(bs, addr, cl_size,
> + QCOW2_DISCARD_ALWAYS);
> + }
> + l1_table[i] = cpu_to_be64(1);
> + } else {
> + if (!addr) {
> + addr = qcow2_alloc_clusters(bs, cl_size);
> + l1_table[i] = cpu_to_be64(addr);
> + }
> +
> + ret = bdrv_pwrite(bs->file, addr, p, write_size);
> + if (ret < 0) {
> + goto finish;
> + }
> + }
> +
> + p += cl_size;
> + }
> +
> + ret = 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 = bs->opaque;
> + QCowDirtyBitmap *new_dirty_bitmap_list = NULL;
> + QCowDirtyBitmap *old_dirty_bitmap_list = NULL;
> + QCowDirtyBitmap sn1, *bm = &sn1;
> + int i, ret;
> + uint64_t *l1_table = NULL;
> + int64_t l1_table_offset;
> + int sector_granularity = granularity >> BDRV_SECTOR_BITS;
> +
> + if (s->nb_dirty_bitmaps >= 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) >= 0) {
> + return -EEXIST;
> + }
> +
> + /* Populate bm with passed data */
> + bm->name = g_strdup(name);
> + bm->bitmap_granularity = granularity;
> + bm->bitmap_size = size;
> +
> + bm->l1_size =
> + size_to_clusters(s, (((size - 1) / sector_granularity) >> 3) + 1);
> + l1_table_offset =
> + qcow2_alloc_clusters(bs, s->l1_size * sizeof(uint64_t));
> + if (l1_table_offset < 0) {
> + ret = l1_table_offset;
> + goto fail;
> + }
> + bm->l1_table_offset = l1_table_offset;
> +
> + l1_table = g_try_new(uint64_t, bm->l1_size);
> + if (l1_table == NULL) {
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> + /* initialize with zero clusters */
> + for (i = 0; i < s->l1_size; i++) {
> + l1_table[i] = 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 = qcow2_pre_write_overlap_check(bs, 0, bm->l1_table_offset,
> + s->l1_size * sizeof(uint64_t));
> + if (ret < 0) {
> + goto fail;
> + }
> +
> + ret = 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=none,format=qcow2,cache=writethrough,file=hda.qcow2,id=drive0
-dirty-bitmap name=bitmap,drive=drive0
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:
==13284== Invalid write of size 8
==13284== at 0x53A15E: qcow2_dirty_bitmap_create
(qcow2-dirty-bitmap.c:406)
==13284== by 0x539D15: qcow2_dirty_bitmap_store
(qcow2-dirty-bitmap.c:307)
==13284== by 0x505F27: bdrv_store_dirty_bitmap (block.c:3176)
==13284== by 0x50306D: bdrv_close (block.c:1739)
==13284== by 0x5032FF: bdrv_close_all (block.c:1797)
==13284== by 0x3049DC: main (vl.c:4577)
==13284== Address 0x239b7978 is 0 bytes after a block of size 8 alloc'd
==13284== at 0x4A06BCF: malloc (vg_replace_malloc.c:296)
==13284== by 0x300111: malloc_and_trace (vl.c:2706)
==13284== by 0x62B954E: g_try_malloc (gmem.c:242)
==13284== by 0x53A11E: qcow2_dirty_bitmap_create
(qcow2-dirty-bitmap.c:398)
==13284== by 0x539D15: qcow2_dirty_bitmap_store
(qcow2-dirty-bitmap.c:307)
==13284== by 0x505F27: bdrv_store_dirty_bitmap (block.c:3176)
==13284== by 0x50306D: bdrv_close (block.c:1739)
==13284== by 0x5032FF: bdrv_close_all (block.c:1797)
==13284== by 0x3049DC: main (vl.c:4577)
==13284==
--13284-- VALGRIND INTERNAL ERROR: Valgrind received a signal 11
(SIGSEGV) - exiting
--13284-- si_code=80; Faulting address: 0x0; sp: 0x8090a1de0
valgrind: the 'impossible' happened:
Killed by fatal signal
> + l1_table = NULL;
> +
> + /* Append the new dirty bitmap to the dirty bitmap list */
> + new_dirty_bitmap_list = g_new(QCowDirtyBitmap, s->nb_dirty_bitmaps + 1);
> + if (s->dirty_bitmaps) {
> + memcpy(new_dirty_bitmap_list, s->dirty_bitmaps,
> + s->nb_dirty_bitmaps * sizeof(QCowDirtyBitmap));
> + old_dirty_bitmap_list = s->dirty_bitmaps;
> + }
> + s->dirty_bitmaps = new_dirty_bitmap_list;
> + s->dirty_bitmaps[s->nb_dirty_bitmaps++] = *bm;
> +
> + ret = qcow2_write_dirty_bitmaps(bs);
> + if (ret < 0) {
> + g_free(s->dirty_bitmaps);
> + s->dirty_bitmaps = 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 = bs->opaque;
> + int ret, i;
> + uint64_t *l1_table = g_new(uint64_t, bm->l1_size);
> +
> + ret = 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 = 0; i < bm->l1_size; ++i) {
> + uint64_t addr = be64_to_cpu(l1_table[i]);
> + qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_ALWAYS);
> + }
> +
> + 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 = bs->opaque;
> + QCowDirtyBitmap bm;
> + int dirty_bitmap_index, ret = 0;
> +
> + /* Search the dirty_bitmap */
> + dirty_bitmap_index = find_dirty_bitmap_by_name(bs, name);
> + if (dirty_bitmap_index < 0) {
> + error_setg(errp, "Can't find the dirty bitmap");
> + return -ENOENT;
> + }
> + bm = 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 = 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
>
> static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
> {
> @@ -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;
>
> #ifdef DEBUG_EXT
> printf("qcow2_read_extensions: start=%ld end=%ld\n", start_offset, end_offset);
> @@ -160,6 +162,33 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
> }
> break;
>
> + case QCOW2_EXT_MAGIC_DIRTY_BITMAPS:
> + ret = bdrv_pread(bs->file, offset, &dirty_bitmaps_ext, ext.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 = dirty_bitmaps_ext.dirty_bitmaps_offset;
> + s->nb_dirty_bitmaps = dirty_bitmaps_ext.nb_dirty_bitmaps;
> +
> + ret = qcow2_read_dirty_bitmaps(bs);
> + if (ret < 0) {
> + error_setg_errno(errp, -ret, "Could not read dirty bitmaps");
> + return ret;
> + }
> +
> +#ifdef DEBUG_EXT
> + printf("Qcow2: Got dirty bitmaps extension:"
> + " offset=%" PRIu64 " nb_bitmaps=%" 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);
> }
>
> static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
> @@ -1667,6 +1698,21 @@ int qcow2_update_header(BlockDriverState *bs)
> buf += ret;
> buflen -= ret;
>
> + if (s->nb_dirty_bitmaps > 0) {
> + Qcow2DirtyBitmapHeaderExt dirty_bitmaps_header = {
> + .nb_dirty_bitmaps = cpu_to_be32(s->nb_dirty_bitmaps),
> + .dirty_bitmaps_offset = cpu_to_be64(s->dirty_bitmaps_offset)
> + };
> + ret = header_ext_add(buf, QCOW2_EXT_MAGIC_DIRTY_BITMAPS,
> + &dirty_bitmaps_header, sizeof(dirty_bitmaps_header),
> + buflen);
> + if (ret < 0) {
> + goto fail;
> + }
> + buf += ret;
> + buflen -= ret;
> + }
> +
> /* Keep unknown header extensions */
> QLIST_FOREACH(uext, &s->unknown_header_ext, next) {
> ret = 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;
> }
>
> + /* 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 = {
> .bdrv_get_info = qcow2_get_info,
> .bdrv_get_specific_info = qcow2_get_specific_info,
>
> + .bdrv_dirty_bitmap_load = qcow2_dirty_bitmap_load,
> + .bdrv_dirty_bitmap_store = qcow2_dirty_bitmap_store,
> + .bdrv_dirty_bitmap_delete = qcow2_dirty_bitmap_delete,
> +
> .bdrv_save_vmstate = qcow2_save_vmstate,
> .bdrv_load_vmstate = qcow2_load_vmstate,
>
> 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 @@
>
> #define QCOW_MAX_CRYPT_CLUSTERS 32
> #define QCOW_MAX_SNAPSHOTS 65536
> +#define QCOW_MAX_DIRTY_BITMAPS 65536
>
> /* 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)
>
> +#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;
>
> +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;
>
> +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;
>
> @@ -218,6 +242,11 @@ typedef uint64_t Qcow2GetRefcountFunc(const void *refcount_array,
> typedef void Qcow2SetRefcountFunc(void *refcount_array,
> uint64_t index, uint64_t value);
>
> +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;
>
> + 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);
>
> +/* 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);
>
> + 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,
>
In light of this, some "sanity" tests that test cases like no writes,
empty bitmaps, empty files, etc I think will be appropriate.
next prev parent reply other threads:[~2015-06-11 23:04 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-08 15:21 [Qemu-devel] [PATCH v2 RFC 0/8] block: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
2015-06-08 15:21 ` [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification Vladimir Sementsov-Ogievskiy
2015-06-09 16:01 ` John Snow
2015-06-09 17:03 ` Stefan Hajnoczi
2015-06-10 8:19 ` Vladimir Sementsov-Ogievskiy
2015-06-10 8:49 ` Vladimir Sementsov-Ogievskiy
2015-06-10 13:00 ` Eric Blake
2015-06-11 10:16 ` Vladimir Sementsov-Ogievskiy
2015-06-10 13:24 ` Stefan Hajnoczi
2015-06-11 10:19 ` Vladimir Sementsov-Ogievskiy
2015-06-11 13:03 ` Stefan Hajnoczi
2015-06-11 16:21 ` John Snow
2015-06-12 10:28 ` Stefan Hajnoczi
2015-06-12 15:19 ` John Snow
2015-06-10 15:34 ` Kevin Wolf
2015-06-11 10:25 ` Vladimir Sementsov-Ogievskiy
2015-06-11 16:30 ` John Snow
2015-06-12 8:33 ` Kevin Wolf
2015-08-24 10:46 ` Vladimir Sementsov-Ogievskiy
2015-08-24 13:30 ` Vladimir Sementsov-Ogievskiy
2015-08-24 14:08 ` Vladimir Sementsov-Ogievskiy
2015-08-24 14:04 ` Vladimir Sementsov-Ogievskiy
2015-08-31 22:21 ` Eric Blake
2015-08-31 22:24 ` John Snow
2015-06-08 15:21 ` [Qemu-devel] [PATCH 2/8] qcow2: add dirty-bitmaps feature Vladimir Sementsov-Ogievskiy
2015-06-09 16:52 ` Stefan Hajnoczi
2015-06-10 14:30 ` Stefan Hajnoczi
2015-06-12 19:02 ` John Snow
2015-06-15 14:42 ` Stefan Hajnoczi
2015-06-23 17:57 ` John Snow
2015-06-24 9:39 ` Stefan Hajnoczi
2015-08-14 17:14 ` Vladimir Sementsov-Ogievskiy
2015-08-26 9:09 ` Stefan Hajnoczi
2015-06-11 23:04 ` John Snow [this message]
2015-06-15 14:05 ` Vladimir Sementsov-Ogievskiy
2015-06-15 16:53 ` John Snow
2015-06-12 21:55 ` John Snow
2015-08-26 13:15 ` Vladimir Sementsov-Ogievskiy
2015-08-26 14:14 ` Vladimir Sementsov-Ogievskiy
2015-08-27 12:43 ` Vladimir Sementsov-Ogievskiy
2015-06-08 15:21 ` [Qemu-devel] [PATCH 3/8] block: store persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
2015-06-08 15:21 ` [Qemu-devel] [PATCH 4/8] block: add bdrv_load_dirty_bitmap Vladimir Sementsov-Ogievskiy
2015-06-09 16:01 ` Stefan Hajnoczi
2015-06-10 22:33 ` John Snow
2015-06-11 10:41 ` Vladimir Sementsov-Ogievskiy
2015-06-08 15:21 ` [Qemu-devel] [PATCH 5/8] qcow2: add qcow2_dirty_bitmap_delete_all Vladimir Sementsov-Ogievskiy
2015-06-08 15:21 ` [Qemu-devel] [PATCH 6/8] qcow2: add autoclear bit for dirty bitmaps Vladimir Sementsov-Ogievskiy
2015-06-09 15:49 ` Stefan Hajnoczi
2015-06-09 15:50 ` Stefan Hajnoczi
2015-08-27 7:45 ` Vladimir Sementsov-Ogievskiy
2015-08-31 11:06 ` Vladimir Sementsov-Ogievskiy
2015-08-31 22:39 ` Eric Blake
2015-08-31 22:50 ` Eric Blake
2015-06-10 23:42 ` John Snow
2015-06-11 8:35 ` Kevin Wolf
2015-06-11 10:49 ` Vladimir Sementsov-Ogievskiy
2015-06-11 16:36 ` John Snow
2015-06-08 15:21 ` [Qemu-devel] [PATCH 7/8] qemu: command line option " Vladimir Sementsov-Ogievskiy
2015-06-11 20:57 ` John Snow
2015-06-12 21:49 ` John Snow
2015-06-08 15:21 ` [Qemu-devel] [PATCH 8/8] iotests: test internal persistent dirty bitmap Vladimir Sementsov-Ogievskiy
2015-06-09 16:17 ` Eric Blake
2015-06-10 15:27 ` [Qemu-devel] [PATCH v2 RFC 0/8] block: persistent dirty bitmaps Stefan Hajnoczi
2015-06-11 11:22 ` Vladimir Sementsov-Ogievskiy
2015-06-11 13:14 ` Stefan Hajnoczi
2015-06-11 20:06 ` Stefan Hajnoczi
2015-06-12 9:58 ` Denis V. Lunev
2015-06-12 10:36 ` Stefan Hajnoczi
2015-08-26 6:26 ` Vladimir Sementsov-Ogievskiy
2015-08-26 9:13 ` Stefan Hajnoczi
2015-06-12 19:34 ` John Snow
2015-06-17 14:29 ` Vladimir Sementsov-Ogievskiy
2015-06-24 0:21 ` John Snow
2015-07-08 12:24 ` Vladimir Sementsov-Ogievskiy
2015-07-08 15:21 ` John Snow
2015-08-27 10:08 ` Vladimir Sementsov-Ogievskiy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=557A13F7.6090101@redhat.com \
--to=jsnow@redhat.com \
--cc=den@openvz.org \
--cc=kwolf@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=vsementsov@parallels.com \
--cc=vsementsov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.