All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC)
@ 2015-01-13 17:02 Vladimir Sementsov-Ogievskiy
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification Vladimir Sementsov-Ogievskiy
                   ` (9 more replies)
  0 siblings, 10 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-01-13 17:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, vsementsov, stefanha, pbonzini, den, jsnow

The bitmaps are saved into qcow2 file format. It provides both
'internal' and 'external' dirty bitmaps feature:
 - for qcow2 drives we can store bitmaps in the same file
 - for other formats we can store bitmaps in the separate qcow2 file

QCow2 header is extended by fields 'nb_dirty_bitmaps' and
'dirty_bitmaps_offset' like with snapshots.

Proposed command line syntax is the following:

-dirty-bitmap [option1=val1][,option2=val2]...
    Available options are:
    name         The name for the bitmap (necessary).

    file         The file to load the bitmap from.

    file_id      When specified with 'file' option, then this file will
                 be available through this id for other -dirty-bitmap
                 options when specified without 'file' option, then it
                 is a reference to 'file', specified with another
                 -dirty-bitmap option, and it will be used to load the
                 bitmap from.

    drive        The drive to bind the bitmap to. It should be specified
                 as 'id' suboption of one of -drive options. If nor
                 'file' neither 'file_id' are specified, then the bitmap
                 will be loaded from that drive (internal dirty bitmap).

    granularity  The granularity for the bitmap. Not necessary, the
                 default value may be used.

    enabled      on|off. Default is 'on'. Disabled bitmaps are not
                 changing regardless of writes to corresponding drive.

Examples:

qemu -drive file=a.qcow2,id=disk -dirty-bitmap name=b,drive=disk
qemu -drive file=a.raw,id=disk \
     -dirty-bitmap name=b,drive=disk,file=b.qcow2,enabled=off

Vladimir Sementsov-Ogievskiy (8):
  spec: add qcow2-dirty-bitmaps specification
  hbitmap: store / restore
  qcow2: add dirty-bitmaps feature
  block: store persistent dirty bitmaps
  block: add bdrv_load_dirty_bitmap
  qemu: command line option for dirty bitmaps
  qmp: print dirty bitmap
  iotests: test internal persistent dirty bitmap

 block.c                    | 113 ++++++++++
 block/Makefile.objs        |   2 +-
 block/qcow2-dirty-bitmap.c | 514 +++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c              |  26 +++
 block/qcow2.h              |  48 +++++
 blockdev.c                 |  51 +++++
 docs/specs/qcow2.txt       |  59 ++++++
 hmp-commands.hx            |  15 ++
 hmp.c                      |   8 +
 hmp.h                      |   1 +
 include/block/block.h      |   9 +
 include/block/block_int.h  |  10 +
 include/qemu/hbitmap.h     |  49 +++++
 include/sysemu/blockdev.h  |   1 +
 include/sysemu/sysemu.h    |   1 +
 qapi-schema.json           |   3 +-
 qapi/block-core.json       |   3 +
 qemu-options.hx            |  37 ++++
 qmp-commands.hx            |   5 +
 tests/qemu-iotests/115     |  96 +++++++++
 tests/qemu-iotests/115.out |  64 ++++++
 tests/qemu-iotests/group   |   1 +
 util/hbitmap.c             |  87 ++++++++
 vl.c                       | 100 +++++++++
 24 files changed, 1301 insertions(+), 2 deletions(-)
 create mode 100644 block/qcow2-dirty-bitmap.c
 create mode 100755 tests/qemu-iotests/115
 create mode 100644 tests/qemu-iotests/115.out

-- 
1.9.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification
  2015-01-13 17:02 [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC) Vladimir Sementsov-Ogievskiy
@ 2015-01-13 17:02 ` Vladimir Sementsov-Ogievskiy
  2015-01-27 15:39   ` Eric Blake
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 2/8] hbitmap: store / restore Vladimir Sementsov-Ogievskiy
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-01-13 17:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, vsementsov, stefanha, pbonzini, den, jsnow

Persistent dirty bitmaps will be saved into qcow2 files. It may be used
as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
other drives (there may be qcow2 file with zero disk size but with
several dirty bitmaps for other drives).

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
---
 docs/specs/qcow2.txt | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 121dfc8..b29de40 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -116,6 +116,13 @@ in the description of a field.
                     Length of the header structure in bytes. For version 2
                     images, the length is always assumed to be 72 bytes.
 
+        104 - 107:  nb_dirty_bitmaps
+                    Number of dirty bitmaps contained in the image
+
+        108 - 115:  dirty_bitmaps_offset
+                    Offset into the image file at which the dirty bitmaps table
+                    starts. Must be aligned to a cluster boundary.
+
 Directly after the image header, optional sections called header extensions can
 be stored. Each extension has a structure like the following:
 
@@ -360,3 +367,55 @@ Snapshot table entry:
 
         variable:   Padding to round up the snapshot table entry size to the
                     next multiple of 8.
+
+
+== Dirty bitmaps ==
+
+The feature supports storing several dirty bitmaps in the qcow2 file.
+
+=== Cluster mapping ===
+
+Dirty bitmaps are stored using a ONE-level structure for the mapping of
+bitmaps to host clusters. There only an L1 table.
+
+The L1 table has a variable size (stored in the Bitmap table entry) and may
+use multiple clusters, however it must be contiguous in the image file.
+
+Given an offset into the bitmap, the offset into the image file can be
+obtained as follows:
+
+    offset = l1_table[offset / cluster_size] + (offset % cluster_size)
+
+L1 table entry:
+
+    Bit  0 -  61:   Standard cluster descriptor
+
+        62 -  63:   Reserved
+
+=== Bitmap table ===
+
+A directory of all bitmaps is stored in the bitmap table, a contiguous area in
+the image file, whose starting offset and length are given by the header fields
+dirty_bitmaps_offset and nb_dirty_bitmaps. The entries of the bitmap table have
+variable length, depending on the length of name and extra data.
+
+Bitmap table entry:
+
+    Byte 0 -  7:    Offset into the image file at which the L1 table for the
+                    bitmap starts. Must be aligned to a cluster boundary.
+
+         8 - 11:    Number of entries in the L1 table of the bitmap
+
+        12 - 15:    Bitmap granularity in bytes
+
+        16 - 23:    Bitmap size in sectors
+
+        24 - 25:    Size of the bitmap name
+
+        variable:   The name of the bitmap (not null terminated)
+
+        variable:   Padding to round up the bitmap table entry size to the
+                    next multiple of 8.
+
+The fields "size", "granularity" and "name" are corresponding with the fields
+in struct BdrvDirtyBitmap.
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 2/8] hbitmap: store / restore
  2015-01-13 17:02 [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC) Vladimir Sementsov-Ogievskiy
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification Vladimir Sementsov-Ogievskiy
@ 2015-01-13 17:02 ` Vladimir Sementsov-Ogievskiy
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 3/8] qcow2: add dirty-bitmaps feature Vladimir Sementsov-Ogievskiy
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-01-13 17:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, vsementsov, stefanha, pbonzini, den, jsnow

Functions to store / restore HBitmap. HBitmap should be saved to linear
bitmap format independently of endianess.

These functions are appropriate for dirty bitmap migration, retoring the
bitmap in several steps is available.  To save performance, every step
writes only the last level of the bitmap. All other levels are restored
by hbitmap_restore_finish as a last step of restoring. So, HBitmap is
inconsistent while restoring.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
---
 include/qemu/hbitmap.h | 49 ++++++++++++++++++++++++++++
 util/hbitmap.c         | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+)

diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index c48c50a..f432f7f 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -137,6 +137,55 @@ void hbitmap_reset(HBitmap *hb, uint64_t start, uint64_t count);
 bool hbitmap_get(const HBitmap *hb, uint64_t item);
 
 /**
+ * hbitmap_data_size:
+ * @hb: HBitmap to operate on.
+ * @count: Number of bits
+ *
+ * Return amount of bytes hbitmap_store_data needs
+ */
+uint64_t hbitmap_data_size(const HBitmap *hb, uint64_t count);
+
+/**
+ * hbitmap_store_data
+ * @hb: HBitmap to oprate on.
+ * @buf: Buffer to store bitmap data.
+ * @start: First bit to store.
+ * @count: Number of bits to store.
+ *
+ * Stores HBitmap data corresponding to given region. The format of saved data
+ * is linear sequence of bits, so it can be used by hbitmap_restore_data
+ * independently of endianess
+ */
+void hbitmap_store_data(const HBitmap *hb, uint8_t *buf,
+                        uint64_t start, uint64_t count);
+
+/**
+ * hbitmap_restore_data
+ * @hb: HBitmap to oprate on.
+ * @buf: Buffer to restore bitmap data from.
+ * @start: First bit to restore.
+ * @count: Number of bits to restore.
+ *
+ * Retores HBitmap data corresponding to given region. The format is the same
+ * as for hbitmap_store_data.
+ *
+ * ! The bitmap becomes inconsistent after this operation.
+ * hbitmap_restore_finish should be called before using the bitmap after
+ * data restoring.
+ */
+void hbitmap_restore_data(HBitmap *hb, uint8_t *buf,
+                          uint64_t start, uint64_t count);
+
+/**
+ * hbitmap_restore_finish
+ * @hb: HBitmap to operate on.
+ *
+ * Repair HBitmap after calling hbitmap_restore_data. Actuall all HBitmap
+ * layers are restore here.
+ */
+void hbitmap_restore_finish(HBitmap *hb);
+
+/**
  * hbitmap_free:
  * @hb: HBitmap to operate on.
  *
diff --git a/util/hbitmap.c b/util/hbitmap.c
index f400dcb..5d1a776 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -366,6 +366,93 @@ bool hbitmap_get(const HBitmap *hb, uint64_t item)
     return (hb->levels[HBITMAP_LEVELS - 1][pos >> BITS_PER_LEVEL] & bit) != 0;
 }
 
+uint64_t hbitmap_data_size(const HBitmap *hb, uint64_t count)
+{
+    uint64_t size;
+
+    if (count == 0) {
+        return 0;
+    }
+
+    size = (((count - 1) >> hb->granularity) >> BITS_PER_LEVEL) + 1;
+
+    return size * sizeof(unsigned long);
+}
+
+void hbitmap_store_data(const HBitmap *hb, uint8_t *buf,
+                        uint64_t start, uint64_t count)
+{
+    uint64_t last = start + count - 1;
+    unsigned long *out = (unsigned long *)buf;
+
+    if (count == 0) {
+        return;
+    }
+
+    start = (start >> hb->granularity) >> BITS_PER_LEVEL;
+    last = (last >> hb->granularity) >> BITS_PER_LEVEL;
+    count = last - start + 1;
+
+#ifdef __BIG_ENDIAN_BITFIELD
+    for (i = start; i <= last; ++i) {
+        unsigned long el = hb->levels[HBITMAP_LEVELS - 1][i];
+        out[i] = (BITS_PER_LONG == 32 ? cpu_to_le32(el) : cpu_to_le64(el));
+    }
+#else
+    memcpy(out, &hb->levels[HBITMAP_LEVELS - 1][start],
+           count * sizeof(unsigned long));
+#endif
+}
+
+void hbitmap_restore_data(HBitmap *hb, uint8_t *buf,
+                          uint64_t start, uint64_t count)
+{
+    uint64_t last = start + count - 1;
+    unsigned long *in = (unsigned long *)buf;
+
+    if (count == 0) {
+        return;
+    }
+
+    start = (start >> hb->granularity) >> BITS_PER_LEVEL;
+    last = (last >> hb->granularity) >> BITS_PER_LEVEL;
+    count = last - start + 1;
+
+#ifdef __BIG_ENDIAN_BITFIELD
+    for (i = start; i <= last; ++i) {
+        hb->levels[HBITMAP_LEVELS - 1][i] =
+            (BITS_PER_LONG == 32 ? be32_to_cpu(in[i]) : be64_to_cpu(in[i]));
+    }
+#else
+    memcpy(&hb->levels[HBITMAP_LEVELS - 1][start], in,
+           count * sizeof(unsigned long));
+#endif
+}
+
+void hbitmap_restore_finish(HBitmap *bitmap)
+{
+    int64_t i, size, prev_size;
+    int lev;
+
+    /* restore levels starting from penultimate to zero level, assuming
+     * that the last level is ok */
+    size = MAX((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+    for (lev = HBITMAP_LEVELS - 1; lev-- > 0; ) {
+        prev_size = size;
+        size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+        memset(bitmap->levels[lev], 0, size * sizeof(unsigned long));
+
+        for (i = 0; i < prev_size; ++i) {
+            if (bitmap->levels[lev + 1][i]) {
+                bitmap->levels[lev][i >> BITS_PER_LEVEL] |=
+                    1 << (i & (BITS_PER_LONG - 1));
+            }
+        }
+    }
+
+    bitmap->levels[0][0] |= 1UL << (BITS_PER_LONG - 1);
+}
+
 void hbitmap_free(HBitmap *hb)
 {
     unsigned i;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 3/8] qcow2: add dirty-bitmaps feature
  2015-01-13 17:02 [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC) Vladimir Sementsov-Ogievskiy
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification Vladimir Sementsov-Ogievskiy
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 2/8] hbitmap: store / restore Vladimir Sementsov-Ogievskiy
@ 2015-01-13 17:02 ` Vladimir Sementsov-Ogievskiy
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 4/8] block: store persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-01-13 17:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, vsementsov, stefanha, pbonzini, den, jsnow

Adds dirty-bitmaps feature to qcow2 format as specified in
docs/specs/qcow2.txt

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
---
 block/Makefile.objs        |   2 +-
 block/qcow2-dirty-bitmap.c | 514 +++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c              |  26 +++
 block/qcow2.h              |  48 +++++
 include/block/block_int.h  |  10 +
 5 files changed, 599 insertions(+), 1 deletion(-)
 create mode 100644 block/qcow2-dirty-bitmap.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 04b0e43..eebd1c9 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 dmg.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..b3d114f
--- /dev/null
+++ b/block/qcow2-dirty-bitmap.c
@@ -0,0 +1,514 @@
+/*
+ * 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 list 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;
+    struct {
+        uint32_t nb_dirty_bitmaps;
+        uint64_t dirty_bitmaps_offset;
+    } QEMU_PACKED header_data;
+    int64_t offset, dirty_bitmaps_offset = 0;
+    int ret;
+
+    /* compute the size of the dirty bitmaps */
+    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 list */
+    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 list 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 list */
+    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 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;
+    }
+
+    QEMU_BUILD_BUG_ON(offsetof(QCowHeader, dirty_bitmaps_offset) !=
+                      offsetof(QCowHeader, nb_dirty_bitmaps) +
+                      sizeof(header_data.nb_dirty_bitmaps));
+
+    header_data.nb_dirty_bitmaps        = cpu_to_be32(s->nb_dirty_bitmaps);
+    header_data.dirty_bitmaps_offset    = cpu_to_be64(dirty_bitmaps_offset);
+
+    ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, nb_dirty_bitmaps),
+                           &header_data, sizeof(header_data));
+    if (ret < 0) {
+        goto fail;
+    }
+
+    /* free the old dirty bitmap table */
+    qcow2_free_clusters(bs, s->dirty_bitmaps_offset, s->dirty_bitmaps_size,
+                        QCOW2_DISCARD_ALWAYS);
+    s->dirty_bitmaps_offset = dirty_bitmaps_offset;
+    s->dirty_bitmaps_size = dirty_bitmaps_size;
+    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;
+
+    /* 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);
+    }
+
+    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);
+    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;
+
+fail:
+    g_free(bm->name);
+    g_free(l1_table);
+
+    return ret;
+}
+
+int qcow2_dirty_bitmap_delete(BlockDriverState *bs,
+                              const char *name,
+                              Error **errp)
+{
+    BDRVQcowState *s = bs->opaque;
+    QCowDirtyBitmap bm;
+    int dirty_bitmap_index, ret = 0, i;
+    uint64_t *l1_table;
+
+    /* 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;
+    }
+
+    /*
+     * The dirty_bitmap is now unused, clean up. If we fail after this point, we
+     * won't recover but just leak clusters.
+     */
+    g_free(bm.name);
+
+    /*
+     * Now decrease the refcounts of clusters referenced by the dirty_bitmap and
+     * free the L1 table.
+     */
+    l1_table = g_try_new(uint64_t, bm.l1_size);
+    if (l1_table == NULL) {
+        ret = -ENOMEM;
+        goto finish;
+    }
+    ret = bdrv_pread(bs->file, bm.l1_table_offset, l1_table,
+                     s->l1_size * sizeof(uint64_t));
+    if (ret < 0) {
+        goto finish;
+    }
+
+    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);
+
+finish:
+    g_free(l1_table);
+    return ret;
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index e4e690a..6512788 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -570,6 +570,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     be32_to_cpus(&header.refcount_table_clusters);
     be64_to_cpus(&header.snapshots_offset);
     be32_to_cpus(&header.nb_snapshots);
+    be64_to_cpus(&header.dirty_bitmaps_offset);
+    be32_to_cpus(&header.nb_dirty_bitmaps);
 
     if (header.magic != QCOW_MAGIC) {
         error_setg(errp, "Image is not in qcow2 format");
@@ -892,6 +894,16 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
+    /* Internal bitmaps */
+    s->dirty_bitmaps_offset = header.dirty_bitmaps_offset;
+    s->nb_dirty_bitmaps = header.nb_dirty_bitmaps;
+
+    ret = qcow2_read_dirty_bitmaps(bs);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not read dirty bitmaps");
+        goto fail;
+    }
+
     /* Clear unknown autoclear feature bits */
     if (!bs->read_only && !(flags & BDRV_O_INCOMING) && s->autoclear_features) {
         s->autoclear_features = 0;
@@ -994,6 +1006,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 */
@@ -1457,6 +1470,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)
@@ -1579,6 +1593,8 @@ int qcow2_update_header(BlockDriverState *bs)
         .refcount_table_clusters = cpu_to_be32(refcount_table_clusters),
         .nb_snapshots           = cpu_to_be32(s->nb_snapshots),
         .snapshots_offset       = cpu_to_be64(s->snapshots_offset),
+        .nb_dirty_bitmaps           = cpu_to_be32(s->nb_dirty_bitmaps),
+        .dirty_bitmaps_offset       = cpu_to_be64(s->dirty_bitmaps_offset),
 
         /* Version 3 fields */
         .incompatible_features  = cpu_to_be64(s->incompatible_features),
@@ -2123,6 +2139,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");
@@ -2888,6 +2910,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 6e39a1b..45a166d 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) */
@@ -116,6 +119,9 @@ typedef struct QCowHeader {
 
     uint32_t refcount_order;
     uint32_t header_length;
+
+    uint32_t nb_dirty_bitmaps;
+    uint64_t dirty_bitmaps_offset;
 } QEMU_PACKED QCowHeader;
 
 typedef struct QEMU_PACKED QCowSnapshotHeader {
@@ -138,6 +144,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 +175,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;
 
@@ -254,6 +281,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;
@@ -558,6 +590,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 cb1e4a1..0e3b5b3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -204,6 +204,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,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 4/8] block: store persistent dirty bitmaps
  2015-01-13 17:02 [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC) Vladimir Sementsov-Ogievskiy
                   ` (2 preceding siblings ...)
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 3/8] qcow2: add dirty-bitmaps feature Vladimir Sementsov-Ogievskiy
@ 2015-01-13 17:02 ` Vladimir Sementsov-Ogievskiy
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 5/8] block: add bdrv_load_dirty_bitmap Vladimir Sementsov-Ogievskiy
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-01-13 17:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, vsementsov, stefanha, pbonzini, den, jsnow

Persistent dirty bitmaps are the bitmaps, for which the new field
BdrvDirtyBitmap.file is not NULL. We save all persistent dirty bitmaps
owned by BlockDriverState in corresponding bdrv_close().
BdrvDirtyBitmap.file is a BlockDriverState, where we want to save the
bitmap. It may be set in bdrv_dirty_bitmap_set_file() only once.
bdrv_ref/bdrv_unref are used for BdrvDirtyBitmap.file to be sure that
files will be closed and resources will be freed.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
---
 block.c               | 43 +++++++++++++++++++++++++++++++++++++++++++
 include/block/block.h |  3 +++
 2 files changed, 46 insertions(+)

diff --git a/block.c b/block.c
index 2466ba8..7237b95 100644
--- a/block.c
+++ b/block.c
@@ -54,6 +54,7 @@
 struct BdrvDirtyBitmap {
     HBitmap *bitmap;
     BdrvDirtyBitmap *originator;
+    BlockDriverState *file;
     int64_t size;
     int64_t granularity;
     char *name;
@@ -1840,6 +1841,7 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
 void bdrv_close(BlockDriverState *bs)
 {
     BdrvAioNotifier *ban, *ban_next;
+    BdrvDirtyBitmap *bm, *bm_next;
 
     if (bs->job) {
         block_job_cancel_sync(bs->job);
@@ -1849,6 +1851,15 @@ void bdrv_close(BlockDriverState *bs)
     bdrv_drain_all(); /* in case flush left pending I/O */
     notifier_list_notify(&bs->close_notifiers, bs);
 
+    /* save and release persistent dirty bitmaps */
+    QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, bm_next) {
+        if (bm->file) {
+            bdrv_store_dirty_bitmap(bm);
+            bdrv_unref(bm->file);
+            bdrv_release_dirty_bitmap(bs, bm);
+        }
+    }
+
     if (bs->drv) {
         if (bs->backing_hd) {
             BlockDriverState *backing_hd = bs->backing_hd;
@@ -5373,6 +5384,29 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
     return originator;
 }
 
+int bdrv_store_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+    BlockDriverState *bs = bitmap->file;
+    uint8_t *buf;
+    uint64_t size;
+    assert(bs);
+    assert(bs->drv);
+    assert(bs->drv->bdrv_dirty_bitmap_store);
+
+    size = hbitmap_data_size(bitmap->bitmap, bitmap->size);
+    size = (size + 3) & ~3;
+    buf = g_malloc(size);
+
+    hbitmap_store_data(bitmap->bitmap, buf, 0, bitmap->size);
+
+    int res = bs->drv->bdrv_dirty_bitmap_store(bs, buf,
+                                               bitmap->name,
+                                               bitmap->size,
+                                               bitmap->granularity);
+
+    g_free(buf);
+    return res;
+}
 
 BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
                                           int granularity,
@@ -5421,6 +5455,15 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
     }
 }
 
+void bdrv_dirty_bitmap_set_file(BdrvDirtyBitmap *bitmap, BlockDriverState *file)
+{
+    assert(bitmap->file == NULL);
+    bitmap->file = file;
+    if (file != NULL) {
+        bdrv_ref(file);
+    }
+}
+
 void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
 {
     bitmap->enabled = false;
diff --git a/include/block/block.h b/include/block/block.h
index cb1f28d..0dfefe3 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -442,6 +442,8 @@ BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
 BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
                                            BdrvDirtyBitmap *failed);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+void bdrv_dirty_bitmap_set_file(BdrvDirtyBitmap *bitmap,
+                                BlockDriverState *file);
 void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
@@ -458,6 +460,7 @@ void bdrv_dirty_iter_init(BlockDriverState *bs,
                           BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
 void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
 int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+int bdrv_store_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 
 void bdrv_enable_copy_on_read(BlockDriverState *bs);
 void bdrv_disable_copy_on_read(BlockDriverState *bs);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 5/8] block: add bdrv_load_dirty_bitmap
  2015-01-13 17:02 [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC) Vladimir Sementsov-Ogievskiy
                   ` (3 preceding siblings ...)
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 4/8] block: store persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
@ 2015-01-13 17:02 ` Vladimir Sementsov-Ogievskiy
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 6/8] qemu: command line option for dirty bitmaps Vladimir Sementsov-Ogievskiy
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-01-13 17:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, vsementsov, stefanha, pbonzini, den, jsnow

The funcion loads dirty bitmap from file, using underlying driver
function.

Note: the function doesn't change BdrvDirtyBitmap.file field. This field
is only used by bdrv_store_dirty_bitmap() function and is ONLY written
by bdrv_dirty_bitmap_set_file() function.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
---
 block.c               | 37 +++++++++++++++++++++++++++++++++++++
 include/block/block.h |  5 +++++
 2 files changed, 42 insertions(+)

diff --git a/block.c b/block.c
index 7237b95..77419e9 100644
--- a/block.c
+++ b/block.c
@@ -5384,6 +5384,43 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
     return originator;
 }
 
+BdrvDirtyBitmap *bdrv_load_dirty_bitmap(BlockDriverState *bs,
+                                        BlockDriverState *file,
+                                        int granularity,
+                                        const char *name,
+                                        Error **errp)
+{
+    BlockDriver *drv = file->drv;
+    if (!drv) {
+        return NULL;
+    }
+    if (drv->bdrv_dirty_bitmap_load) {
+        BdrvDirtyBitmap *bitmap;
+        uint64_t bitmap_size = bdrv_nb_sectors(bs);
+        uint8_t *buf = drv->bdrv_dirty_bitmap_load(file, name, bitmap_size,
+                                                   granularity);
+        if (buf == NULL) {
+            return NULL;
+        }
+
+        bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
+        if (bitmap == NULL) {
+            g_free(buf);
+            return NULL;
+        }
+
+        hbitmap_restore_data(bitmap->bitmap, buf, 0, bitmap_size);
+        hbitmap_restore_finish(bitmap->bitmap);
+
+        return bitmap;
+    }
+    if (file->file)  {
+        return bdrv_load_dirty_bitmap(bs, file->file, granularity, name,
+                                      errp);
+    }
+    return NULL;
+}
+
 int bdrv_store_dirty_bitmap(BdrvDirtyBitmap *bitmap)
 {
     BlockDriverState *bs = bitmap->file;
diff --git a/include/block/block.h b/include/block/block.h
index 0dfefe3..f36557f 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -460,6 +460,11 @@ void bdrv_dirty_iter_init(BlockDriverState *bs,
                           BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
 void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
 int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+BdrvDirtyBitmap *bdrv_load_dirty_bitmap(BlockDriverState *bs,
+                                        BlockDriverState *file,
+                                        int granularity,
+                                        const char *name,
+                                        Error **errp);
 int bdrv_store_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 
 void bdrv_enable_copy_on_read(BlockDriverState *bs);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 6/8] qemu: command line option for dirty bitmaps
  2015-01-13 17:02 [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC) Vladimir Sementsov-Ogievskiy
                   ` (4 preceding siblings ...)
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 5/8] block: add bdrv_load_dirty_bitmap Vladimir Sementsov-Ogievskiy
@ 2015-01-13 17:02 ` Vladimir Sementsov-Ogievskiy
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 7/8] qmp: print dirty bitmap Vladimir Sementsov-Ogievskiy
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-01-13 17:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, vsementsov, stefanha, pbonzini, den, jsnow

The patch adds the following command line option:

-dirty-bitmap [option1=val1][,option2=val2]...
    Available options are:
    name         The name for the bitmap (necessary).

    file         The file to load the bitmap from.

    file_id      When specified with 'file' option, then this file will
                 be available through this id for other -dirty-bitmap
                 options when specified without 'file' option, then it
                 is a reference to 'file', specified with another
                 -dirty-bitmap option, and it will be used to load the
                 bitmap from.

    drive        The drive to bind the bitmap to. It should be specified
                 as 'id' suboption of one of -drive options. If nor
                 'file' neither 'file_id' are specified, then the bitmap
                 will be loaded from that drive (internal dirty bitmap).

    granularity  The granularity for the bitmap. Not necessary, the
                 default value may be used.

    enabled      on|off. Default is 'on'. Disabled bitmaps are not
                 changing regardless of writes to corresponding drive.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
---
 blockdev.c                |  38 ++++++++++++++++++
 include/sysemu/blockdev.h |   1 +
 include/sysemu/sysemu.h   |   1 +
 qemu-options.hx           |  37 +++++++++++++++++
 vl.c                      | 100 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 177 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 209fedd..8a9be08 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -176,6 +176,11 @@ QemuOpts *drive_def(const char *optstr)
     return qemu_opts_parse(qemu_find_opts("drive"), optstr, 0);
 }
 
+QemuOpts *dirty_bitmap_def(const char *optstr)
+{
+    return qemu_opts_parse(qemu_find_opts("dirty-bitmap"), optstr, 0);
+}
+
 QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
                     const char *optstr)
 {
@@ -3044,6 +3049,39 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
     return head;
 }
 
+QemuOptsList qemu_dirty_bitmap_opts = {
+    .name = "dirty-bitmap",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_dirty_bitmap_opts.head),
+    .desc = {
+        {
+            .name = "name",
+            .type = QEMU_OPT_STRING,
+            .help = "Name of the dirty bitmap",
+        },{
+            .name = "file",
+            .type = QEMU_OPT_STRING,
+            .help = "file name to load the bitmap from",
+        },{
+            .name = "file_id",
+            .type = QEMU_OPT_STRING,
+            .help = "node name to load the bitmap from (or to set id for"
+                    " for file, opened by previous option)",
+        },{
+            .name = "drive",
+            .type = QEMU_OPT_STRING,
+            .help = "drive id to bind the bitmap to",
+        },{
+            .name = "granularity",
+            .type = QEMU_OPT_NUMBER,
+            .help = "granularity",
+        },{
+            .name = "enabled",
+            .type = QEMU_OPT_BOOL,
+            .help = "enabled flag (default is 'on')",
+        }
+    }
+};
+
 QemuOptsList qemu_common_drive_opts = {
     .name = "drive",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_common_drive_opts.head),
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 09d1e30..48cce5c 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -57,6 +57,7 @@ int drive_get_max_devs(BlockInterfaceType type);
 DriveInfo *drive_get_next(BlockInterfaceType type);
 
 QemuOpts *drive_def(const char *optstr);
+QemuOpts *dirty_bitmap_def(const char *optstr);
 QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
                     const char *optstr);
 DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type);
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 503e5a4..c984a52 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -231,6 +231,7 @@ bool usb_enabled(bool default_usb);
 
 extern QemuOptsList qemu_legacy_drive_opts;
 extern QemuOptsList qemu_common_drive_opts;
+extern QemuOptsList qemu_dirty_bitmap_opts;
 extern QemuOptsList qemu_drive_opts;
 extern QemuOptsList qemu_chardev_opts;
 extern QemuOptsList qemu_device_opts;
diff --git a/qemu-options.hx b/qemu-options.hx
index 10b9568..3a5bfde 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -592,6 +592,43 @@ qemu-system-i386 -hda a -hdb b
 @end example
 ETEXI
 
+DEF("dirty-bitmap", HAS_ARG, QEMU_OPTION_dirty_bitmap,
+    "-dirty-bitmap name=name[,file=file][,file_id=file_id][,drive=@var{id}]\n"
+    "              [,granularity=granularity][,enabled=on|off]\n",
+    QEMU_ARCH_ALL)
+STEXI
+@item -dirty-bitmap @var{option}[,@var{option}[,@var{option}[,...]]]
+@findex -dirty-bitmap
+
+Define a dirty-bitmap. Valid options are:
+
+@table @option
+@item name=@var{name}
+The name of the bitmap. Should be unique per @var{file}/@var{drive} and per
+@var{for_drive}.
+@item file=@var{file}
+The separate qcow2 file for loading the bitmap @var{name} from it.
+@item file_id=@var{file_id}
+When specified with @var{file} option, then this @var{file} will be available
+through this @var{file_id} for other @option{-dirty-bitmap} options.
+When specified without @var{file} option, then it is a reference to @var{file},
+specified with another @option{-dirty-bitmap} option, and it will be used to
+load the bitmap from.
+@item drive=@var{drive}
+The drive to bind the bitmap to. It should be specified as @var{id} suboption
+of one of @option{-drive} options.
+If nor @var{file} neither @var{file_id} are specified, then the bitmap will be
+loaded from that drive (internal dirty bitmap).
+@item granularity=@var{granularity}
+Granularity (in bytes) for created dirty bitmap. If the bitmap is already
+exists in specified @var{file}/@var{file_id}/@var{device} it's granularity will
+not be changed but only checked (an error will be generated if this check
+fails).
+@item enabled=@var{enabled}
+Enabled flag for the bitmap. By default the bitmap will be enabled.
+@end table
+ETEXI
+
 DEF("mtdblock", HAS_ARG, QEMU_OPTION_mtdblock,
     "-mtdblock file  use 'file' as on-board Flash memory image\n",
     QEMU_ARCH_ALL)
diff --git a/vl.c b/vl.c
index a824a7d..092771e 100644
--- a/vl.c
+++ b/vl.c
@@ -1157,6 +1157,95 @@ static int cleanup_add_fd(QemuOpts *opts, void *opaque)
 #define MTD_OPTS ""
 #define SD_OPTS ""
 
+static int dirty_bitmap_func(QemuOpts *opts, void *opaque)
+{
+    Error *local_err = NULL;
+    Error **errp = &local_err;
+    BlockDriverState *file_bs = NULL, *for_bs = NULL;
+    BdrvDirtyBitmap *bitmap = NULL;
+
+    const char *name = qemu_opt_get(opts, "name");
+    const char *drive = qemu_opt_get(opts, "drive");
+    const char *file = qemu_opt_get(opts, "file");
+    const char *file_id = qemu_opt_get(opts, "file_id");
+
+    uint64_t granularity = qemu_opt_get_number(opts, "granularity", 0);
+    bool enabled = qemu_opt_get_bool(opts, "enabled", true);
+
+    if (name == NULL) {
+        error_setg(errp, "'name' option is necessary");
+        goto fail;
+    }
+
+    if (drive == NULL) {
+        error_setg(errp, "'drive' option is necessary");
+        goto fail;
+    }
+
+    for_bs = bdrv_lookup_bs(drive, NULL, errp);
+    if (for_bs == NULL) {
+        goto fail;
+    }
+
+    if (file != NULL) {
+        QDict *options = NULL;
+        if (file_id != NULL) {
+            options = qdict_new();
+            qdict_put(options, "node-name", qstring_from_str(file_id));
+        }
+
+        bdrv_open(&file_bs, file, NULL, options, 0, NULL, errp);
+        if (options) {
+            QDECREF(options);
+        }
+        if (file_bs == NULL) {
+            goto fail;
+        }
+    } else if (file_id != NULL) {
+        file_bs = bdrv_find_node(file_id);
+        if (file_bs == NULL) {
+            error_setg(errp, "node '%s' is not found", drive);
+            goto fail;
+        }
+    } else {
+        file_bs = for_bs;
+    }
+
+    if (granularity == 0) {
+        granularity = bdrv_get_default_bitmap_granularity(for_bs);
+    }
+
+    bitmap = bdrv_load_dirty_bitmap(for_bs, file_bs, granularity, name,
+                                    errp);
+    if (*errp != NULL) {
+        goto fail;
+    }
+
+    if (bitmap == NULL) {
+        /* bitmap is not found in file_bs */
+        bitmap = bdrv_create_dirty_bitmap(for_bs, granularity, name, errp);
+        if (!bitmap) {
+            goto fail;
+        }
+    }
+
+    bdrv_dirty_bitmap_set_file(bitmap, file_bs);
+
+    if (!enabled) {
+        bdrv_disable_dirty_bitmap(bitmap);
+    }
+
+    return 0;
+
+fail:
+    error_report("-dirty-bitmap: %s", error_get_pretty(local_err));
+    error_free(local_err);
+    if (file_bs != NULL) {
+        bdrv_close(file_bs);
+    }
+    return -1;
+}
+
 static int drive_init_func(QemuOpts *opts, void *opaque)
 {
     BlockInterfaceType *block_default_type = opaque;
@@ -2741,6 +2830,7 @@ int main(int argc, char **argv, char **envp)
     module_call_init(MODULE_INIT_QOM);
 
     qemu_add_opts(&qemu_drive_opts);
+    qemu_add_opts(&qemu_dirty_bitmap_opts);
     qemu_add_drive_opts(&qemu_legacy_drive_opts);
     qemu_add_drive_opts(&qemu_common_drive_opts);
     qemu_add_drive_opts(&qemu_drive_opts);
@@ -2881,6 +2971,11 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
 	        break;
+            case QEMU_OPTION_dirty_bitmap:
+                if (dirty_bitmap_def(optarg) == NULL) {
+                    exit(1);
+                }
+            break;
             case QEMU_OPTION_set:
                 if (qemu_set_option(optarg) != 0)
                     exit(1);
@@ -4205,6 +4300,11 @@ int main(int argc, char **argv, char **envp)
     default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
     default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
 
+    if (qemu_opts_foreach(qemu_find_opts("dirty-bitmap"), dirty_bitmap_func,
+                          NULL, 1) != 0) {
+        exit(1);
+    }
+
     if (qemu_opts_foreach(qemu_find_opts("numa"), numa_init_func,
                           NULL, 1) != 0) {
         exit(1);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 7/8] qmp: print dirty bitmap
  2015-01-13 17:02 [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC) Vladimir Sementsov-Ogievskiy
                   ` (5 preceding siblings ...)
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 6/8] qemu: command line option for dirty bitmaps Vladimir Sementsov-Ogievskiy
@ 2015-01-13 17:02 ` Vladimir Sementsov-Ogievskiy
  2015-01-27 15:53   ` Eric Blake
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 8/8] iotests: test internal persistent " Vladimir Sementsov-Ogievskiy
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-01-13 17:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, vsementsov, stefanha, pbonzini, den, jsnow

Adds qmp and hmp commands to print dirty bitmap. This is needed only for
testing persistent dirty bitmap feature.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
---
 block.c               | 33 +++++++++++++++++++++++++++++++++
 blockdev.c            | 13 +++++++++++++
 hmp-commands.hx       | 15 +++++++++++++++
 hmp.c                 |  8 ++++++++
 hmp.h                 |  1 +
 include/block/block.h |  1 +
 qapi-schema.json      |  3 ++-
 qapi/block-core.json  |  3 +++
 qmp-commands.hx       |  5 +++++
 9 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 77419e9..3e6dedf 100644
--- a/block.c
+++ b/block.c
@@ -5445,6 +5445,39 @@ int bdrv_store_dirty_bitmap(BdrvDirtyBitmap *bitmap)
     return res;
 }
 
+void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+    unsigned long a = 0, b = 0;
+
+    printf("bitmap '%s'\n", bitmap->name ? bitmap->name : "no name");
+    printf("enabled: %s\n", bitmap->enabled ? "true" : "false");
+    printf("size: %" PRId64 "\n", bitmap->size);
+    printf("granularity: %" PRId64 "\n", bitmap->granularity);
+    printf("dirty regions begin:\n");
+
+    while (true) {
+        for (a = b; a < bitmap->size && !hbitmap_get(bitmap->bitmap, a); ++a) {
+            ;
+        }
+        if (a >= bitmap->size) {
+            break;
+        }
+
+        for (b = a + 1;
+             b < bitmap->size && hbitmap_get(bitmap->bitmap, b);
+             ++b) {
+            ;
+        }
+
+        printf("%ld -> %ld\n", a, b - 1);
+        if (b >= bitmap->size) {
+            break;
+        }
+    }
+
+    printf("dirty regions end\n");
+}
+
 BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
                                           int granularity,
                                           const char *name,
diff --git a/blockdev.c b/blockdev.c
index 8a9be08..8b58c2e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2079,6 +2079,19 @@ void qmp_block_dirty_bitmap_add(const char *node_ref, const char *name,
     aio_context_release(aio_context);
 }
 
+void qmp_block_dirty_bitmap_print(const char *node_ref, const char *name,
+                                  Error **errp)
+{
+    BdrvDirtyBitmap *bitmap;
+
+    bitmap = block_dirty_bitmap_lookup(node_ref, name, NULL, errp);
+    if (!bitmap) {
+        return;
+    }
+
+    bdrv_print_dirty_bitmap(bitmap);
+}
+
 void qmp_block_dirty_bitmap_remove(const char *node_ref, const char *name,
                                    Error **errp)
 {
diff --git a/hmp-commands.hx b/hmp-commands.hx
index e37bc8b..a9be506 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -58,6 +58,21 @@ Quit the emulator.
 ETEXI
 
     {
+        .name       = "print_dirty_bitmap",
+        .args_type  = "device:B,bitmap:s",
+        .params     = "device bitmap",
+        .help       = "print dirty bitmap",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd = hmp_print_dirty_bitmap,
+    },
+
+STEXI
+@item print_dirty_bitmap device_id bitmap_name
+@findex print_dirty_bitmap
+Print dirty bitmap meta information and dirty regions.
+ETEXI
+
+    {
         .name       = "block_resize",
         .args_type  = "device:B,size:o",
         .params     = "device size",
diff --git a/hmp.c b/hmp.c
index 63b19c7..a269145 100644
--- a/hmp.c
+++ b/hmp.c
@@ -782,6 +782,14 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
     qapi_free_TPMInfoList(info_list);
 }
 
+void hmp_print_dirty_bitmap(Monitor *mon, const QDict *qdict)
+{
+    const char *device = qdict_get_str(qdict, "device");
+    const char *name = qdict_get_str(qdict, "bitmap");
+
+    qmp_block_dirty_bitmap_print(device, name, NULL);
+}
+
 void hmp_quit(Monitor *mon, const QDict *qdict)
 {
     monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index 4bb5dca..6bbbc33 100644
--- a/hmp.h
+++ b/hmp.h
@@ -19,6 +19,7 @@
 #include "qapi-types.h"
 #include "qapi/qmp/qdict.h"
 
+void hmp_print_dirty_bitmap(Monitor *mon, const QDict *qdict);
 void hmp_info_name(Monitor *mon, const QDict *qdict);
 void hmp_info_version(Monitor *mon, const QDict *qdict);
 void hmp_info_kvm(Monitor *mon, const QDict *qdict);
diff --git a/include/block/block.h b/include/block/block.h
index f36557f..7188791 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -466,6 +466,7 @@ BdrvDirtyBitmap *bdrv_load_dirty_bitmap(BlockDriverState *bs,
                                         const char *name,
                                         Error **errp);
 int bdrv_store_dirty_bitmap(BdrvDirtyBitmap *bitmap);
+void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 
 void bdrv_enable_copy_on_read(BlockDriverState *bs);
 void bdrv_disable_copy_on_read(BlockDriverState *bs);
diff --git a/qapi-schema.json b/qapi-schema.json
index 85f55d9..1475f69 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1263,7 +1263,8 @@
        'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
        'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
        'block-dirty-bitmap-enable': 'BlockDirtyBitmap',
-       'block-dirty-bitmap-disable': 'BlockDirtyBitmap'
+       'block-dirty-bitmap-disable': 'BlockDirtyBitmap',
+       'block-dirty-bitmap-print': 'BlockDirtyBitmap'
    } }
 
 ##
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1892b50..3e1edb1 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -982,6 +982,9 @@
 {'command': 'block-dirty-bitmap-disable',
   'data': 'BlockDirtyBitmap' }
 
+{'command': 'block-dirty-bitmap-print',
+  'data': 'BlockDirtyBitmap' }
+
 ##
 # @block_set_io_throttle:
 #
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 479d4f5..8065715 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1222,6 +1222,11 @@ EQMP
         .args_type  = "node-ref:B,name:s",
         .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_disable,
     },
+    {
+        .name       = "block-dirty-bitmap-print",
+        .args_type  = "node-ref:B,name:s",
+        .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_print,
+    },
 
 SQMP
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 8/8] iotests: test internal persistent dirty bitmap
  2015-01-13 17:02 [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC) Vladimir Sementsov-Ogievskiy
                   ` (6 preceding siblings ...)
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 7/8] qmp: print dirty bitmap Vladimir Sementsov-Ogievskiy
@ 2015-01-13 17:02 ` Vladimir Sementsov-Ogievskiy
  2015-01-27 11:06 ` [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC) Vladimir Sementsov-Ogievskiy
  2015-02-04 15:07 ` Vladimir Sementsov-Ogievskiy
  9 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-01-13 17:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, vsementsov, stefanha, pbonzini, den, jsnow

The test performs several vm reloads with checking and updating dirty
bitmap.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
---
 tests/qemu-iotests/115     | 96 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/115.out | 64 +++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 161 insertions(+)
 create mode 100755 tests/qemu-iotests/115
 create mode 100644 tests/qemu-iotests/115.out

diff --git a/tests/qemu-iotests/115 b/tests/qemu-iotests/115
new file mode 100755
index 0000000..ef853b1
--- /dev/null
+++ b/tests/qemu-iotests/115
@@ -0,0 +1,96 @@
+#!/bin/bash
+#
+# Persistent dirty bitmap test
+#
+# Performs several vm reloads with checking and updating dirty bitmap.
+#
+# Copyright (C) 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=vsementsov@parallels.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1    # failure is the default!
+
+_cleanup()
+{
+    _cleanup_qemu
+    _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt qcow2
+_supported_os Linux
+
+size=1G
+_make_test_img $size
+
+qemu_comm_method="monitor"
+
+launch() {
+    _launch_qemu -drive file="${TEST_IMG}",id=disk\
+        -dirty-bitmap name=bitmap,drive=disk
+    echo
+    echo START VM
+}
+
+print_bitmap() {
+    _send_qemu_cmd $QEMU_HANDLE 'print_dirty_bitmap disk bitmap'\
+        "dirty regions end" | sed 's/^/    /'
+}
+
+write() {
+    _send_qemu_cmd $QEMU_HANDLE 'qemu-io disk "write ' $@ '"' "(qemu)"
+    echo
+    echo write $@
+}
+
+quit() {
+    _send_qemu_cmd $QEMU_HANDLE 'qemu-io disk flush' "(qemu)"
+    _send_qemu_cmd $QEMU_HANDLE 'quit' ""
+    echo QUIT
+    wait ${QEMU_PID[$QEMU_HANDLE]} 2>/dev/null
+}
+
+{
+    launch
+    print_bitmap
+    write 50m 1m
+    print_bitmap
+    quit
+
+    launch
+    print_bitmap
+    write 0m 10m
+    write 700m 200m
+    print_bitmap
+    quit
+
+    launch
+    print_bitmap
+    quit
+} | sed '/(qemu)/d'
+
+status=0
diff --git a/tests/qemu-iotests/115.out b/tests/qemu-iotests/115.out
new file mode 100644
index 0000000..d570c9f
--- /dev/null
+++ b/tests/qemu-iotests/115.out
@@ -0,0 +1,64 @@
+QA output created by 115
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
+
+START VM
+QEMU X.Y.Z monitor - type 'help' for more information
+    bitmap 'bitmap'
+    enabled: true
+    size: 2097152
+    granularity: 65536
+    dirty regions begin:
+    dirty regions end
+
+write 50m 1m
+wrote 1048576/1048576 bytes at offset 52428800
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+    bitmap 'bitmap'
+    enabled: true
+    size: 2097152
+    granularity: 65536
+    dirty regions begin:
+    102400 -> 104447
+    dirty regions end
+QUIT
+
+START VM
+QEMU X.Y.Z monitor - type 'help' for more information
+    bitmap 'bitmap'
+    enabled: true
+    size: 2097152
+    granularity: 65536
+    dirty regions begin:
+    102400 -> 104447
+    dirty regions end
+
+write 0m 10m
+wrote 10485760/10485760 bytes at offset 0
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+write 700m 200m
+wrote 209715200/209715200 bytes at offset 734003200
+200 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+    bitmap 'bitmap'
+    enabled: true
+    size: 2097152
+    granularity: 65536
+    dirty regions begin:
+    0 -> 20479
+    102400 -> 104447
+    1433600 -> 1843199
+    dirty regions end
+QUIT
+
+START VM
+QEMU X.Y.Z monitor - type 'help' for more information
+    bitmap 'bitmap'
+    enabled: true
+    size: 2097152
+    granularity: 65536
+    dirty regions begin:
+    0 -> 20479
+    102400 -> 104447
+    1433600 -> 1843199
+    dirty regions end
+QUIT
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index a4742c6..77377b3 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -115,3 +115,4 @@
 111 rw auto quick
 113 rw auto quick
 114 rw auto quick
+115 rw auto quick
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC)
  2015-01-13 17:02 [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC) Vladimir Sementsov-Ogievskiy
                   ` (7 preceding siblings ...)
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 8/8] iotests: test internal persistent " Vladimir Sementsov-Ogievskiy
@ 2015-01-27 11:06 ` Vladimir Sementsov-Ogievskiy
  2015-02-04 15:07 ` Vladimir Sementsov-Ogievskiy
  9 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-01-27 11:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, stefanha, pbonzini, den, jsnow

ping

Best regards,
Vladimir

On 13.01.2015 20:02, Vladimir Sementsov-Ogievskiy wrote:
> The bitmaps are saved into qcow2 file format. It provides both
> 'internal' and 'external' dirty bitmaps feature:
>   - for qcow2 drives we can store bitmaps in the same file
>   - for other formats we can store bitmaps in the separate qcow2 file
>
> QCow2 header is extended by fields 'nb_dirty_bitmaps' and
> 'dirty_bitmaps_offset' like with snapshots.
>
> Proposed command line syntax is the following:
>
> -dirty-bitmap [option1=val1][,option2=val2]...
>      Available options are:
>      name         The name for the bitmap (necessary).
>
>      file         The file to load the bitmap from.
>
>      file_id      When specified with 'file' option, then this file will
>                   be available through this id for other -dirty-bitmap
>                   options when specified without 'file' option, then it
>                   is a reference to 'file', specified with another
>                   -dirty-bitmap option, and it will be used to load the
>                   bitmap from.
>
>      drive        The drive to bind the bitmap to. It should be specified
>                   as 'id' suboption of one of -drive options. If nor
>                   'file' neither 'file_id' are specified, then the bitmap
>                   will be loaded from that drive (internal dirty bitmap).
>
>      granularity  The granularity for the bitmap. Not necessary, the
>                   default value may be used.
>
>      enabled      on|off. Default is 'on'. Disabled bitmaps are not
>                   changing regardless of writes to corresponding drive.
>
> Examples:
>
> qemu -drive file=a.qcow2,id=disk -dirty-bitmap name=b,drive=disk
> qemu -drive file=a.raw,id=disk \
>       -dirty-bitmap name=b,drive=disk,file=b.qcow2,enabled=off
>
> Vladimir Sementsov-Ogievskiy (8):
>    spec: add qcow2-dirty-bitmaps specification
>    hbitmap: store / restore
>    qcow2: add dirty-bitmaps feature
>    block: store persistent dirty bitmaps
>    block: add bdrv_load_dirty_bitmap
>    qemu: command line option for dirty bitmaps
>    qmp: print dirty bitmap
>    iotests: test internal persistent dirty bitmap
>
>   block.c                    | 113 ++++++++++
>   block/Makefile.objs        |   2 +-
>   block/qcow2-dirty-bitmap.c | 514 +++++++++++++++++++++++++++++++++++++++++++++
>   block/qcow2.c              |  26 +++
>   block/qcow2.h              |  48 +++++
>   blockdev.c                 |  51 +++++
>   docs/specs/qcow2.txt       |  59 ++++++
>   hmp-commands.hx            |  15 ++
>   hmp.c                      |   8 +
>   hmp.h                      |   1 +
>   include/block/block.h      |   9 +
>   include/block/block_int.h  |  10 +
>   include/qemu/hbitmap.h     |  49 +++++
>   include/sysemu/blockdev.h  |   1 +
>   include/sysemu/sysemu.h    |   1 +
>   qapi-schema.json           |   3 +-
>   qapi/block-core.json       |   3 +
>   qemu-options.hx            |  37 ++++
>   qmp-commands.hx            |   5 +
>   tests/qemu-iotests/115     |  96 +++++++++
>   tests/qemu-iotests/115.out |  64 ++++++
>   tests/qemu-iotests/group   |   1 +
>   util/hbitmap.c             |  87 ++++++++
>   vl.c                       | 100 +++++++++
>   24 files changed, 1301 insertions(+), 2 deletions(-)
>   create mode 100644 block/qcow2-dirty-bitmap.c
>   create mode 100755 tests/qemu-iotests/115
>   create mode 100644 tests/qemu-iotests/115.out
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification Vladimir Sementsov-Ogievskiy
@ 2015-01-27 15:39   ` Eric Blake
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2015-01-27 15:39 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-devel
  Cc: kwolf, den, jsnow, stefanha, pbonzini

[-- Attachment #1: Type: text/plain, Size: 2279 bytes --]

On 01/13/2015 10:02 AM, Vladimir Sementsov-Ogievskiy wrote:
> Persistent dirty bitmaps will be saved into qcow2 files. It may be used
> as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
> other drives (there may be qcow2 file with zero disk size but with
> several dirty bitmaps for other drives).
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
> ---
>  docs/specs/qcow2.txt | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
> index 121dfc8..b29de40 100644
> --- a/docs/specs/qcow2.txt
> +++ b/docs/specs/qcow2.txt
> @@ -116,6 +116,13 @@ in the description of a field.
>                      Length of the header structure in bytes. For version 2
>                      images, the length is always assumed to be 72 bytes.
>  
> +        104 - 107:  nb_dirty_bitmaps
> +                    Number of dirty bitmaps contained in the image
> +
> +        108 - 115:  dirty_bitmaps_offset
> +                    Offset into the image file at which the dirty bitmaps table
> +                    starts. Must be aligned to a cluster boundary.

NACK.  You cannot add new fields directly into the header without
bumping the version number; but that is overkill.  Instead, stick the
dirty bitmaps into a new header extension.  The existing fields of the
header are sufficient to record that a new extension is present.  Also,
you need to decide whether dirty bitmaps are an ignorable feature or a
mandatory feature, and set the appropriate feature bit in the header
flag fields (thus when an older qemu that doesn't know about dirty
bitmaps opens the file, it will either reject the open due to not
knowing how to keep dirty bitmaps sane, or will accept the open but
clear the ignorable bit so that the next time a new qemu opens the file
it will know that the dirty bitmap is no longer reliable).

> +=== Cluster mapping ===
> +
> +Dirty bitmaps are stored using a ONE-level structure for the mapping of
> +bitmaps to host clusters. There only an L1 table.

s/There/There is/

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 7/8] qmp: print dirty bitmap
  2015-01-13 17:02 ` [Qemu-devel] [PATCH 7/8] qmp: print dirty bitmap Vladimir Sementsov-Ogievskiy
@ 2015-01-27 15:53   ` Eric Blake
  2015-01-27 16:29     ` Markus Armbruster
  2015-01-30  9:06     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 2 replies; 18+ messages in thread
From: Eric Blake @ 2015-01-27 15:53 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-devel
  Cc: kwolf, den, jsnow, stefanha, pbonzini

[-- Attachment #1: Type: text/plain, Size: 2211 bytes --]

On 01/13/2015 10:02 AM, Vladimir Sementsov-Ogievskiy wrote:
> Adds qmp and hmp commands to print dirty bitmap. This is needed only for
> testing persistent dirty bitmap feature.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
> ---

> +++ b/block.c
> @@ -5445,6 +5445,39 @@ int bdrv_store_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>      return res;
>  }
>  
> +void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap)
> +{
> +    unsigned long a = 0, b = 0;
> +
> +    printf("bitmap '%s'\n", bitmap->name ? bitmap->name : "no name");
> +    printf("enabled: %s\n", bitmap->enabled ? "true" : "false");
> +    printf("size: %" PRId64 "\n", bitmap->size);
> +    printf("granularity: %" PRId64 "\n", bitmap->granularity);
> +    printf("dirty regions begin:\n");

> +++ b/blockdev.c
> @@ -2079,6 +2079,19 @@ void qmp_block_dirty_bitmap_add(const char *node_ref, const char *name,
>      aio_context_release(aio_context);
>  }
>  
> +void qmp_block_dirty_bitmap_print(const char *node_ref, const char *name,
> +                                  Error **errp)
> +{
> +    BdrvDirtyBitmap *bitmap;
> +
> +    bitmap = block_dirty_bitmap_lookup(node_ref, name, NULL, errp);
> +    if (!bitmap) {
> +        return;
> +    }
> +
> +    bdrv_print_dirty_bitmap(bitmap);

Eww.  This assumes that stdout is usable.  But that is not the case for
QMP commands.  It would be better to package up the output in a
structured format and return it to the caller, and let the caller decide
how to print it.

> +++ b/qapi/block-core.json
> @@ -982,6 +982,9 @@
>  {'command': 'block-dirty-bitmap-disable',
>    'data': 'BlockDirtyBitmap' }
>  
> +{'command': 'block-dirty-bitmap-print',
> +  'data': 'BlockDirtyBitmap' }

Missing documentation, if we even want this command.  As mentioned
above, this should return ALL of the information necessary for the
client to then decide how to print the information, rather than directly
printing information to stdout itself.  And it might be better to name
this command in the 'query-' namespace.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 7/8] qmp: print dirty bitmap
  2015-01-27 15:53   ` Eric Blake
@ 2015-01-27 16:29     ` Markus Armbruster
  2015-01-30  9:06     ` Vladimir Sementsov-Ogievskiy
  1 sibling, 0 replies; 18+ messages in thread
From: Markus Armbruster @ 2015-01-27 16:29 UTC (permalink / raw)
  To: Eric Blake
  Cc: kwolf, qemu-devel, Vladimir Sementsov-Ogievskiy, stefanha,
	pbonzini, den, jsnow

Eric Blake <eblake@redhat.com> writes:

> On 01/13/2015 10:02 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Adds qmp and hmp commands to print dirty bitmap. This is needed only for
>> testing persistent dirty bitmap feature.
>> 
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
>> ---
>
>> +++ b/block.c
>> @@ -5445,6 +5445,39 @@ int bdrv_store_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>>      return res;
>>  }
>>  
>> +void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>> +{
>> +    unsigned long a = 0, b = 0;
>> +
>> +    printf("bitmap '%s'\n", bitmap->name ? bitmap->name : "no name");
>> +    printf("enabled: %s\n", bitmap->enabled ? "true" : "false");
>> +    printf("size: %" PRId64 "\n", bitmap->size);
>> +    printf("granularity: %" PRId64 "\n", bitmap->granularity);
>> +    printf("dirty regions begin:\n");
>
>> +++ b/blockdev.c
>> @@ -2079,6 +2079,19 @@ void qmp_block_dirty_bitmap_add(const char *node_ref, const char *name,
>>      aio_context_release(aio_context);
>>  }
>>  
>> +void qmp_block_dirty_bitmap_print(const char *node_ref, const char *name,
>> +                                  Error **errp)
>> +{
>> +    BdrvDirtyBitmap *bitmap;
>> +
>> +    bitmap = block_dirty_bitmap_lookup(node_ref, name, NULL, errp);
>> +    if (!bitmap) {
>> +        return;
>> +    }
>> +
>> +    bdrv_print_dirty_bitmap(bitmap);
>
> Eww.  This assumes that stdout is usable.  But that is not the case for
> QMP commands.  It would be better to package up the output in a
> structured format and return it to the caller, and let the caller decide
> how to print it.

This isn't a case of "would be better", actually.

HMP commands may print to the *monitor*.  Printing to stdout is wrong.

QMP commands return their result as a JSON object.  Printing results
anywhere is wrong.

We commonly implement the HMP command handler on top of the QMP command
handler.  The QMP command handler returns an object.  The QMP core sends
this object in JSON format to the QMP client.  The HMP command prints it
to the monitor in a suitable human-readable format.

>> +++ b/qapi/block-core.json
>> @@ -982,6 +982,9 @@
>>  {'command': 'block-dirty-bitmap-disable',
>>    'data': 'BlockDirtyBitmap' }
>>  
>> +{'command': 'block-dirty-bitmap-print',
>> +  'data': 'BlockDirtyBitmap' }
>
> Missing documentation, if we even want this command.  As mentioned
> above, this should return ALL of the information necessary for the
> client to then decide how to print the information, rather than directly
> printing information to stdout itself.  And it might be better to name
> this command in the 'query-' namespace.

Yup.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 7/8] qmp: print dirty bitmap
  2015-01-27 15:53   ` Eric Blake
  2015-01-27 16:29     ` Markus Armbruster
@ 2015-01-30  9:06     ` Vladimir Sementsov-Ogievskiy
  2015-01-30 17:51       ` Eric Blake
  1 sibling, 1 reply; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-01-30  9:06 UTC (permalink / raw)
  To: Eric Blake, qemu-devel; +Cc: kwolf, den, jsnow, stefanha, pbonzini

is it better to add qmp_query_dirty_bitmap with underlying 
bdrv_query_dirty_bitmap, or to modify (add dirty regions information) 
existing qmp_query_block/qmp_query_dirty_bitmapS?

Best regards,
Vladimir

On 27.01.2015 18:53, Eric Blake wrote:
> On 01/13/2015 10:02 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Adds qmp and hmp commands to print dirty bitmap. This is needed only for
>> testing persistent dirty bitmap feature.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
>> ---
>> +++ b/block.c
>> @@ -5445,6 +5445,39 @@ int bdrv_store_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>>       return res;
>>   }
>>   
>> +void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>> +{
>> +    unsigned long a = 0, b = 0;
>> +
>> +    printf("bitmap '%s'\n", bitmap->name ? bitmap->name : "no name");
>> +    printf("enabled: %s\n", bitmap->enabled ? "true" : "false");
>> +    printf("size: %" PRId64 "\n", bitmap->size);
>> +    printf("granularity: %" PRId64 "\n", bitmap->granularity);
>> +    printf("dirty regions begin:\n");
>> +++ b/blockdev.c
>> @@ -2079,6 +2079,19 @@ void qmp_block_dirty_bitmap_add(const char *node_ref, const char *name,
>>       aio_context_release(aio_context);
>>   }
>>   
>> +void qmp_block_dirty_bitmap_print(const char *node_ref, const char *name,
>> +                                  Error **errp)
>> +{
>> +    BdrvDirtyBitmap *bitmap;
>> +
>> +    bitmap = block_dirty_bitmap_lookup(node_ref, name, NULL, errp);
>> +    if (!bitmap) {
>> +        return;
>> +    }
>> +
>> +    bdrv_print_dirty_bitmap(bitmap);
> Eww.  This assumes that stdout is usable.  But that is not the case for
> QMP commands.  It would be better to package up the output in a
> structured format and return it to the caller, and let the caller decide
> how to print it.
>
>> +++ b/qapi/block-core.json
>> @@ -982,6 +982,9 @@
>>   {'command': 'block-dirty-bitmap-disable',
>>     'data': 'BlockDirtyBitmap' }
>>   
>> +{'command': 'block-dirty-bitmap-print',
>> +  'data': 'BlockDirtyBitmap' }
> Missing documentation, if we even want this command.  As mentioned
> above, this should return ALL of the information necessary for the
> client to then decide how to print the information, rather than directly
> printing information to stdout itself.  And it might be better to name
> this command in the 'query-' namespace.
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 7/8] qmp: print dirty bitmap
  2015-01-30  9:06     ` Vladimir Sementsov-Ogievskiy
@ 2015-01-30 17:51       ` Eric Blake
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2015-01-30 17:51 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-devel
  Cc: kwolf, den, jsnow, stefanha, pbonzini

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

On 01/30/2015 02:06 AM, Vladimir Sementsov-Ogievskiy wrote:
> is it better to add qmp_query_dirty_bitmap with underlying
> bdrv_query_dirty_bitmap, or to modify (add dirty regions information)
> existing qmp_query_block/qmp_query_dirty_bitmapS?

[please don't top-post on technical lists]

Extending an existing command may be reasonable, if the amount of
information being added is compact (if you are adding several kilobytes
of information, that might justify a new command, if only so that old
callers don't waste time receiving large amounts of data on the wire
just to sift through and discard it all to get at the older content they
cared about).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC)
  2015-01-13 17:02 [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC) Vladimir Sementsov-Ogievskiy
                   ` (8 preceding siblings ...)
  2015-01-27 11:06 ` [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC) Vladimir Sementsov-Ogievskiy
@ 2015-02-04 15:07 ` Vladimir Sementsov-Ogievskiy
  2015-02-04 15:20   ` John Snow
  9 siblings, 1 reply; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-02-04 15:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, stefanha, pbonzini, den, jsnow

On 13.01.2015 20:02, Vladimir Sementsov-Ogievskiy wrote:
> The bitmaps are saved into qcow2 file format. It provides both
> 'internal' and 'external' dirty bitmaps feature:
>   - for qcow2 drives we can store bitmaps in the same file
>   - for other formats we can store bitmaps in the separate qcow2 file
>
> QCow2 header is extended by fields 'nb_dirty_bitmaps' and
> 'dirty_bitmaps_offset' like with snapshots.
>
> Proposed command line syntax is the following:
>
> -dirty-bitmap [option1=val1][,option2=val2]...
>      Available options are:
>      name         The name for the bitmap (necessary).
>
>      file         The file to load the bitmap from.
>
>      file_id      When specified with 'file' option, then this file will
>                   be available through this id for other -dirty-bitmap
>                   options when specified without 'file' option, then it
>                   is a reference to 'file', specified with another
>                   -dirty-bitmap option, and it will be used to load the
>                   bitmap from.
>
>      drive        The drive to bind the bitmap to. It should be specified
>                   as 'id' suboption of one of -drive options. If nor
>                   'file' neither 'file_id' are specified, then the bitmap
>                   will be loaded from that drive (internal dirty bitmap).
>
>      granularity  The granularity for the bitmap. Not necessary, the
>                   default value may be used.
>
>      enabled      on|off. Default is 'on'. Disabled bitmaps are not
>                   changing regardless of writes to corresponding drive.
>
> Examples:
>
> qemu -drive file=a.qcow2,id=disk -dirty-bitmap name=b,drive=disk
> qemu -drive file=a.raw,id=disk \
>       -dirty-bitmap name=b,drive=disk,file=b.qcow2,enabled=off
>
> Vladimir Sementsov-Ogievskiy (8):
>    spec: add qcow2-dirty-bitmaps specification
>    hbitmap: store / restore
>    qcow2: add dirty-bitmaps feature
>    block: store persistent dirty bitmaps
>    block: add bdrv_load_dirty_bitmap
>    qemu: command line option for dirty bitmaps
>    qmp: print dirty bitmap
>    iotests: test internal persistent dirty bitmap
>
>   block.c                    | 113 ++++++++++
>   block/Makefile.objs        |   2 +-
>   block/qcow2-dirty-bitmap.c | 514 +++++++++++++++++++++++++++++++++++++++++++++
>   block/qcow2.c              |  26 +++
>   block/qcow2.h              |  48 +++++
>   blockdev.c                 |  51 +++++
>   docs/specs/qcow2.txt       |  59 ++++++
>   hmp-commands.hx            |  15 ++
>   hmp.c                      |   8 +
>   hmp.h                      |   1 +
>   include/block/block.h      |   9 +
>   include/block/block_int.h  |  10 +
>   include/qemu/hbitmap.h     |  49 +++++
>   include/sysemu/blockdev.h  |   1 +
>   include/sysemu/sysemu.h    |   1 +
>   qapi-schema.json           |   3 +-
>   qapi/block-core.json       |   3 +
>   qemu-options.hx            |  37 ++++
>   qmp-commands.hx            |   5 +
>   tests/qemu-iotests/115     |  96 +++++++++
>   tests/qemu-iotests/115.out |  64 ++++++
>   tests/qemu-iotests/group   |   1 +
>   util/hbitmap.c             |  87 ++++++++
>   vl.c                       | 100 +++++++++
>   24 files changed, 1301 insertions(+), 2 deletions(-)
>   create mode 100644 block/qcow2-dirty-bitmap.c
>   create mode 100755 tests/qemu-iotests/115
>   create mode 100644 tests/qemu-iotests/115.out
>

Ping. I've already done (locally):
1) using qcow2 header extension instead of changing the header
2) normal qmp query request instead of "print dirty bitmap"
  - thanks to Eric and Markus

Now I'm waiting for some comments on the concept and it's realization to 
roll v3.

-- 
Best regards,
Vladimir

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC)
  2015-02-04 15:07 ` Vladimir Sementsov-Ogievskiy
@ 2015-02-04 15:20   ` John Snow
  2015-02-04 15:40     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 18+ messages in thread
From: John Snow @ 2015-02-04 15:20 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-devel; +Cc: kwolf, den, stefanha, pbonzini



On 02/04/2015 10:07 AM, Vladimir Sementsov-Ogievskiy wrote:
> On 13.01.2015 20:02, Vladimir Sementsov-Ogievskiy wrote:
>> The bitmaps are saved into qcow2 file format. It provides both
>> 'internal' and 'external' dirty bitmaps feature:
>>   - for qcow2 drives we can store bitmaps in the same file
>>   - for other formats we can store bitmaps in the separate qcow2 file
>>
>> QCow2 header is extended by fields 'nb_dirty_bitmaps' and
>> 'dirty_bitmaps_offset' like with snapshots.
>>
>> Proposed command line syntax is the following:
>>
>> -dirty-bitmap [option1=val1][,option2=val2]...
>>      Available options are:
>>      name         The name for the bitmap (necessary).
>>
>>      file         The file to load the bitmap from.
>>
>>      file_id      When specified with 'file' option, then this file will
>>                   be available through this id for other -dirty-bitmap
>>                   options when specified without 'file' option, then it
>>                   is a reference to 'file', specified with another
>>                   -dirty-bitmap option, and it will be used to load the
>>                   bitmap from.
>>
>>      drive        The drive to bind the bitmap to. It should be specified
>>                   as 'id' suboption of one of -drive options. If nor
>>                   'file' neither 'file_id' are specified, then the bitmap
>>                   will be loaded from that drive (internal dirty bitmap).
>>
>>      granularity  The granularity for the bitmap. Not necessary, the
>>                   default value may be used.
>>
>>      enabled      on|off. Default is 'on'. Disabled bitmaps are not
>>                   changing regardless of writes to corresponding drive.
>>
>> Examples:
>>
>> qemu -drive file=a.qcow2,id=disk -dirty-bitmap name=b,drive=disk
>> qemu -drive file=a.raw,id=disk \
>>       -dirty-bitmap name=b,drive=disk,file=b.qcow2,enabled=off
>>
>> Vladimir Sementsov-Ogievskiy (8):
>>    spec: add qcow2-dirty-bitmaps specification
>>    hbitmap: store / restore
>>    qcow2: add dirty-bitmaps feature
>>    block: store persistent dirty bitmaps
>>    block: add bdrv_load_dirty_bitmap
>>    qemu: command line option for dirty bitmaps
>>    qmp: print dirty bitmap
>>    iotests: test internal persistent dirty bitmap
>>
>>   block.c                    | 113 ++++++++++
>>   block/Makefile.objs        |   2 +-
>>   block/qcow2-dirty-bitmap.c | 514
>> +++++++++++++++++++++++++++++++++++++++++++++
>>   block/qcow2.c              |  26 +++
>>   block/qcow2.h              |  48 +++++
>>   blockdev.c                 |  51 +++++
>>   docs/specs/qcow2.txt       |  59 ++++++
>>   hmp-commands.hx            |  15 ++
>>   hmp.c                      |   8 +
>>   hmp.h                      |   1 +
>>   include/block/block.h      |   9 +
>>   include/block/block_int.h  |  10 +
>>   include/qemu/hbitmap.h     |  49 +++++
>>   include/sysemu/blockdev.h  |   1 +
>>   include/sysemu/sysemu.h    |   1 +
>>   qapi-schema.json           |   3 +-
>>   qapi/block-core.json       |   3 +
>>   qemu-options.hx            |  37 ++++
>>   qmp-commands.hx            |   5 +
>>   tests/qemu-iotests/115     |  96 +++++++++
>>   tests/qemu-iotests/115.out |  64 ++++++
>>   tests/qemu-iotests/group   |   1 +
>>   util/hbitmap.c             |  87 ++++++++
>>   vl.c                       | 100 +++++++++
>>   24 files changed, 1301 insertions(+), 2 deletions(-)
>>   create mode 100644 block/qcow2-dirty-bitmap.c
>>   create mode 100755 tests/qemu-iotests/115
>>   create mode 100644 tests/qemu-iotests/115.out
>>
>
> Ping. I've already done (locally):
> 1) using qcow2 header extension instead of changing the header
> 2) normal qmp query request instead of "print dirty bitmap"
>   - thanks to Eric and Markus
>
> Now I'm waiting for some comments on the concept and it's realization to
> roll v3.
>

My apologies, I've been sick for a while. I'll get going on finishing 
review of these two series.

I assume you'd like to merge the bitmaps migration first?

--js

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC)
  2015-02-04 15:20   ` John Snow
@ 2015-02-04 15:40     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-02-04 15:40 UTC (permalink / raw)
  To: John Snow, qemu-devel; +Cc: kwolf, den, stefanha, pbonzini

On 04.02.2015 18:20, John Snow wrote:
>
>
> On 02/04/2015 10:07 AM, Vladimir Sementsov-Ogievskiy wrote:
>> On 13.01.2015 20:02, Vladimir Sementsov-Ogievskiy wrote:
>>> The bitmaps are saved into qcow2 file format. It provides both
>>> 'internal' and 'external' dirty bitmaps feature:
>>>   - for qcow2 drives we can store bitmaps in the same file
>>>   - for other formats we can store bitmaps in the separate qcow2 file
>>>
>>> QCow2 header is extended by fields 'nb_dirty_bitmaps' and
>>> 'dirty_bitmaps_offset' like with snapshots.
>>>
>>> Proposed command line syntax is the following:
>>>
>>> -dirty-bitmap [option1=val1][,option2=val2]...
>>>      Available options are:
>>>      name         The name for the bitmap (necessary).
>>>
>>>      file         The file to load the bitmap from.
>>>
>>>      file_id      When specified with 'file' option, then this file 
>>> will
>>>                   be available through this id for other -dirty-bitmap
>>>                   options when specified without 'file' option, then it
>>>                   is a reference to 'file', specified with another
>>>                   -dirty-bitmap option, and it will be used to load the
>>>                   bitmap from.
>>>
>>>      drive        The drive to bind the bitmap to. It should be 
>>> specified
>>>                   as 'id' suboption of one of -drive options. If nor
>>>                   'file' neither 'file_id' are specified, then the 
>>> bitmap
>>>                   will be loaded from that drive (internal dirty 
>>> bitmap).
>>>
>>>      granularity  The granularity for the bitmap. Not necessary, the
>>>                   default value may be used.
>>>
>>>      enabled      on|off. Default is 'on'. Disabled bitmaps are not
>>>                   changing regardless of writes to corresponding drive.
>>>
>>> Examples:
>>>
>>> qemu -drive file=a.qcow2,id=disk -dirty-bitmap name=b,drive=disk
>>> qemu -drive file=a.raw,id=disk \
>>>       -dirty-bitmap name=b,drive=disk,file=b.qcow2,enabled=off
>>>
>>> Vladimir Sementsov-Ogievskiy (8):
>>>    spec: add qcow2-dirty-bitmaps specification
>>>    hbitmap: store / restore
>>>    qcow2: add dirty-bitmaps feature
>>>    block: store persistent dirty bitmaps
>>>    block: add bdrv_load_dirty_bitmap
>>>    qemu: command line option for dirty bitmaps
>>>    qmp: print dirty bitmap
>>>    iotests: test internal persistent dirty bitmap
>>>
>>>   block.c                    | 113 ++++++++++
>>>   block/Makefile.objs        |   2 +-
>>>   block/qcow2-dirty-bitmap.c | 514
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>   block/qcow2.c              |  26 +++
>>>   block/qcow2.h              |  48 +++++
>>>   blockdev.c                 |  51 +++++
>>>   docs/specs/qcow2.txt       |  59 ++++++
>>>   hmp-commands.hx            |  15 ++
>>>   hmp.c                      |   8 +
>>>   hmp.h                      |   1 +
>>>   include/block/block.h      |   9 +
>>>   include/block/block_int.h  |  10 +
>>>   include/qemu/hbitmap.h     |  49 +++++
>>>   include/sysemu/blockdev.h  |   1 +
>>>   include/sysemu/sysemu.h    |   1 +
>>>   qapi-schema.json           |   3 +-
>>>   qapi/block-core.json       |   3 +
>>>   qemu-options.hx            |  37 ++++
>>>   qmp-commands.hx            |   5 +
>>>   tests/qemu-iotests/115     |  96 +++++++++
>>>   tests/qemu-iotests/115.out |  64 ++++++
>>>   tests/qemu-iotests/group   |   1 +
>>>   util/hbitmap.c             |  87 ++++++++
>>>   vl.c                       | 100 +++++++++
>>>   24 files changed, 1301 insertions(+), 2 deletions(-)
>>>   create mode 100644 block/qcow2-dirty-bitmap.c
>>>   create mode 100755 tests/qemu-iotests/115
>>>   create mode 100644 tests/qemu-iotests/115.out
>>>
>>
>> Ping. I've already done (locally):
>> 1) using qcow2 header extension instead of changing the header
>> 2) normal qmp query request instead of "print dirty bitmap"
>>   - thanks to Eric and Markus
>>
>> Now I'm waiting for some comments on the concept and it's realization to
>> roll v3.
>>
>
> My apologies, I've been sick for a while. I'll get going on finishing 
> review of these two series.
>
> I assume you'd like to merge the bitmaps migration first?
>
> --js

The sequence is not important. These series are necessary for making a 
complete backup solution - don't lose named dirty bitmaps on shutdown 
and migration. Shutdown is more often task, and dirty bitmaps migration 
was appeared as a subtask when we were discussed the format for saving 
dirty bitmaps. But they both are needed, then, ok, let's start with 
migration.

-- 
Best regards,
Vladimir

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-02-04 15:40 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-13 17:02 [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC) Vladimir Sementsov-Ogievskiy
2015-01-13 17:02 ` [Qemu-devel] [PATCH 1/8] spec: add qcow2-dirty-bitmaps specification Vladimir Sementsov-Ogievskiy
2015-01-27 15:39   ` Eric Blake
2015-01-13 17:02 ` [Qemu-devel] [PATCH 2/8] hbitmap: store / restore Vladimir Sementsov-Ogievskiy
2015-01-13 17:02 ` [Qemu-devel] [PATCH 3/8] qcow2: add dirty-bitmaps feature Vladimir Sementsov-Ogievskiy
2015-01-13 17:02 ` [Qemu-devel] [PATCH 4/8] block: store persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
2015-01-13 17:02 ` [Qemu-devel] [PATCH 5/8] block: add bdrv_load_dirty_bitmap Vladimir Sementsov-Ogievskiy
2015-01-13 17:02 ` [Qemu-devel] [PATCH 6/8] qemu: command line option for dirty bitmaps Vladimir Sementsov-Ogievskiy
2015-01-13 17:02 ` [Qemu-devel] [PATCH 7/8] qmp: print dirty bitmap Vladimir Sementsov-Ogievskiy
2015-01-27 15:53   ` Eric Blake
2015-01-27 16:29     ` Markus Armbruster
2015-01-30  9:06     ` Vladimir Sementsov-Ogievskiy
2015-01-30 17:51       ` Eric Blake
2015-01-13 17:02 ` [Qemu-devel] [PATCH 8/8] iotests: test internal persistent " Vladimir Sementsov-Ogievskiy
2015-01-27 11:06 ` [Qemu-devel] [PATCH 0/8] block: persistent dirty bitmaps (RFC) Vladimir Sementsov-Ogievskiy
2015-02-04 15:07 ` Vladimir Sementsov-Ogievskiy
2015-02-04 15:20   ` John Snow
2015-02-04 15:40     ` Vladimir Sementsov-Ogievskiy

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.