Re: [Qemu-devel] [PATCH 2/8] qcow2: add dirty-bitmaps feature

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

On 12.06.2015 02:04, John Snow wrote:
>
> 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'.
empty file or qcow2 files of size 0 (with header) ?
>
> 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.
>
>


-- 
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.