* [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC)
@ 2014-12-23 1:12 John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 01/13] block: fix spoiling all dirty bitmaps by mirror and migration John Snow
` (13 more replies)
0 siblings, 14 replies; 22+ messages in thread
From: John Snow @ 2014-12-23 1:12 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, John Snow, armbru, mreitz, vsementsov, stefanha
This is v10 of the in-memory part of the incremental backup
feature.
There are some remaining issues for which I am requesting
commentary, please see below under the "RFC" section for the
lingering questions I am aware of as of this revision, the
broad overview below, and the detailed changelog.
Enough changes have been made that most Reviewed-By lines from
previous iterations have been removed. (Sorry!)
This series was originally authored by Fam Zheng;
his cover letter is included below.
~John Snow
=================================================================
This is the in memory part of the incremental backup feature.
With the added commands, we can create a bitmap on a block
backend, from which point of time all the writes are tracked by
the bitmap, marking sectors as dirty. Later, we call drive-backup
and pass the bitmap to it, to do an incremental backup.
See the last patch which adds some tests for this use case.
Fam
=================================================================
For convenience, this patchset is available on github:
https://github.com/jnsnow/qemu/commits/dbm-backup
v10 (Overview):
- I've included one of Vladimir's bitmap fixes as patch #1.
- QMP commands and transactions are now protected via
aio_context functions.
- QMP commands use "node-ref" as a parameter name now. Reasoning
is thus: Either a "device name" or a "node name" can be used to
reference a BDS, which is the key item we are actually acting
on with these bitmap commands. Thus, I refer to this unified
parameter as a "Node Reference," or "node-ref."
We could also argue for "backend-ref" or "device-ref" for the
reverse semantics: where we accept a unified parameter, but we
intend to resolve it to the BlockBackend instead of resolving
the parameter given to the BlockDriverState.
Or, we could use "reference" for both cases, and to match the
existing BlockdevRef command.
- All QMP commands added are now per-node via a unified parameter
name. Though backup only operates on "devices," you are free to
create bitmaps for any arbitrary node you wish. It is obviously
only useful for the root node, currently.
- Bitmap Sync modes (CONSUME, RESET) are removed. See below
(changelog, RFC questions) for more details.
- Code to recover the bitmap after a failure has been added,
but I have some major questions about drive_backup guarantees.
See RFC questions.
v10 (Detailed Changelog):
(1/13) New Patch:
- Included Vladimir Sementsov-Ogievskiy's patch that clarifies
the semantics of the bitmap primitives.
(3/13):
- Edited function names for consistency (Stefanha)
- Removed compile-time constants (Stefanha)
- Acquire aio_context for bitmap add/remove (Stefanha)
- Documented optional granularity for bitmap-add in
qmp-commands.hx file (Eric Blake)
- block_dirty_bitmap_lookup moved forward to this patch to allow
more re-use. (Stefanha)
- Fixed a problem where the block_dirty_bitmap_lookup didn't
always set an error if it returned NULL.
- Added an optional return BDS lookup parameter to
bdrv_dirty_bitmap_lookup.
- Renamed "device" to "node-ref" for block_dirty_bitmap_lookup,
adjusted calls to bdrv_lookup_bs() to reflect unified
parameter usage.
- qmp_block_dirty_bitmap_{add,remove} now reference arbitrary
node names via @node-ref.
(4/13):
- Default granularity and granularity getters are both
now uint64_t (Stefanha)
(5/13):
- Added documentation to warn about the necessity of updating
the hbitmap deep copy. (Stefanha)
(6/13)
- Renamed bdrv_reset_dirty_bitmap to bdrv_clear_dirty_bitmap
to be consistent with Vladimir's patches.
- Removed const qualifier for bdrv_copy_dirty_bitmap,
to accommodate patch 8.
(7/13) New Patch:
- Added an hbitmap_merge operation to join two bitmaps together.
(8/13) New Patch:
- Added bdrv_reclaim_dirty_bitmap() to allow us to "roll back" a
bitmap into the bitmap that it was spawned from. This will let
us maintain an accurate account of dirty sectors even after a
failure.
- This adds an "originator" pointer to the BdrvDirtyBitmap and
is why "const" was removed for copy.
(9/13):
- QMP semantics changes as outlined for Patch #3.
- Enable/Disable now protected by aio_context (Stefanha)
(10/13):
- Add coroutine_fn annotation to block backup helper. (Stefanha)
- Removed sync_bitmap_gran from BackupBlockJob, just use the
getter on the BdrvDirtyBitmap instead. (Stefanha)
- bdrv_dirty_iter_set was renamed to bdrv_set_dirty_iter.
- Calls to bdrv_reset_dirty_bitmap are modified to
bdrv_clear_dirty_bitmap to reflect the rename in patch #6.
- Bitmap usage modes (RESET vs. CONSUME) has been deleted, for
the reason of targeting a simpler core usage first before
targeting optimizations. CONSUME is particularly problematic
in the case of errors; so this mode is omitted for now.
- Adjusted error message to use MirrorSyncMode enum, not
(incorrectly) the BitmapSyncMode enum.
- In the event of a failure, the sync_bitmap is now merged back
into the original bitmap so that we do not lose any dirty
bitmap information needlessly.
(11/13):
- Changed block_dirty_bitmap_add_abort to use
qmp_block_dirty_bitmap_remove:
- Old code used bdrv_lookup_bs to acquire bitmap and bs
anyway, and ignored the failure case.
- New code relies on the same information, and with a NULL
errp pointer we will similarly ignore the failure case.
prepare() should have properly vetted this information
before this point anyway.
This new code is also now protected via aio_context through
the use of the QMP commands.
- More code re-use of block_dirty_bitmap_lookup to get both the
bs and bitmap pointers.
- aio_context protection and cleanup in new abort methods.
(13/13):
- Modified test for new parameter names.
v9:
- Edited commit message, for English embetterment (02/10)
- Rebased on top of stefanha/block-next (06,08/10)
- Adjusted error message and line length (07/10)
v8:
- Changed int64_t return for bdrv_dbm_calc_def_granularity to uint64_t (2/10)
- Updated commit message (2/10)
- Removed redundant check for null in device parameter (2/10)
- Removed comment cruft. (2/10)
- Removed redundant local_err propagation (several)
- Updated commit message (3/10)
- Fix HBitmap copy loop index (4/10)
- Remove redundant ternary (5/10)
- Shift up the block_dirty_bitmap_lookup function (6/10)
- Error messages cleanup (7/10)
- Add an assertion to explain the re-use of .prepare() for two transactions.
(8/10)
- Removed BDS argument from bitmap enable/disable helper; it was unused. (8/10)
v7: (highlights)
- First version being authored by jsnow
- Addressed most list feedback from V6, many small changes.
All feedback was either addressed on-list (as a wontfix) or patched.
- Replaced all error_set with error_setg
- Replaced all bdrv_find with bdrv_lookup_bs()
- Adjusted the max granularity to share a common function with
backup/mirror that attempts to "guess" a reasonable default.
It clamps between [4K,64K] currently.
- The BdrvDirtyBitmap object now counts granularity exclusively in
bytes to match its interface.
It leaves the sector granularity concerns to HBitmap.
- Reworked the backup loop to utilize the hbitmap iterator.
There are some extra concerns to handle arrhythmic cases where the
granularity of the bitmap does not match the backup cluster size.
This iteration works best when it does match, but it's not a
deal-breaker if it doesn't -- it just gets less efficient.
- Reworked the transactional functions so that abort() wouldn't "undo"
a redundant command. They now have been split into a prepare and a
commit function (with state) and do not provide an abort command.
- Added a block_dirty_bitmap_lookup(device, name, errp) function to
shorten a few of the commands added in this series, particularly
qmp_enable, qmp_disable, and the transaction preparations.
v6: Re-send of v5.
v5: Rebase to master.
v4: Last version tailored by Fam Zheng.
=================================================================
RFC Questions:
(1) Return values for QMP documentation.
Versions 1-9 indicate that a QMP command will return
"DeviceNotFound," but this seems to be related to the old
error_set implementation instead of the newer(?) error_setg.
How should docs/writing-qmp-commands.txt and this
documentation be updated accordingly?
(2) qmp_drive_backup coherency
Does qmp_drive_backup promise a /coherent/ backup?
This has implications for how we handle the failure mode
for sync bitmaps in block/backup; those implications are
examined below.
== MODE: CONSUME ==
The "CONSUME" BitmapUseMode would take the BdrvDirtyBitmap
and copy the sectors indicated by the bitmap, then discard
the bitmap. This means that from the time of the start of the
backup, nobody is keeping track of new writes.
In the failure case, this means that either:
(A) Backups are coherent: We cannot retry the operation,
because we simply do not know which bits may have been
altered from the time we started the backup.
The usefulness of the bitmap is lost on errors.
(B) Backups are not necessarily coherent: We can retry the
operation by just keeping this bitmap around and retrying
the operation. Additional bits may have been changed
since, but we are only interested in producing some
differential, without regard to coherency. The usefulness
of this is uncertain, because we do not have the needed
information to flush the remaining differential to
another image, so we can never produce a differential that
forms a coherent backing chain.
For the above reasons, I have omitted the CONSUME mode
from v10 of the patchset entirely.
== MODE: RESET ==
The "RESET" BitmapUseMode, now the only mode, makes a copy of
the BdrvDirtyBitmap for use during backup and clears the
original bitmap. This way we are tracking new writes during
backup.
In the failure case, this means that either for coherent or
incoherent snapshots, we should be able to take the list of
sectors we originally meant to copy and merge them back into
the list of sectors that have become modified since we began
copying them. In either the coherent or incoherent case, this
should be sufficient for maintaining data integrity and
allowing for retries on backup failure.
In the incoherent case, this will allow us to flush remaining
dirty clusters to disk to produce a coherent incremental
backup chain when we go to close the drive.
In this mode, a backup retry will then attempt to copy all
clusters modified since the last successful backup, because
modified sectors will accumulate in the bitmap between each
failure.
== EFFICIENCY ==
If backups are not guaranteed or expected to be coherent; we
may leave too many bits set. For example, say we copy
clusters [0 - 100]. Let's say that cluster 75 is one that is
marked dirty.
As we begin our backup, we make a copy of the original bitmap
and clear all bits in the original.
During our backup, as we copy cluster 25, a write occurs that
marks cluster 75 dirty in the original bitmap. Now it is
marked as dirty in both bitmaps.
If drive backups are not coherent, by the time we arrive at
cluster 75, it includes the newest data that marked the bit
dirty, but no bitmaps have cluster 75 marked clean.
Assuming cluster 75 is not written to again before the next
incremental backup is requested, when we go to do our backup
we will copy cluster 75 needlessly.
This scenario only happens if incremental backups are NOT
guaranteed to be self-consistent. If they are, there is no
problem. If they are not, the existing code can be optimized
by marking sectors as clean in the original bitmap as they
are copied out.
== What the heck? ==
The big RFC question here is "Are backups expected to be
self-consistent?" because depending on the answer, we may
need to treat bitmaps different during backup or upon error
after a backup.
Fam Zheng (8):
qapi: Add optional field "name" to block dirty bitmap
qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
block: Introduce bdrv_dirty_bitmap_granularity()
hbitmap: Add hbitmap_copy
qmp: Add block-dirty-bitmap-enable and block-dirty-bitmap-disable
qapi: Add transaction support to block-dirty-bitmap-{add, enable,
disable}
qmp: Add dirty bitmap 'enabled' field in query-block
qemu-iotests: Add tests for drive-backup sync=dirty-bitmap
John Snow (4):
block: Add bdrv_copy_dirty_bitmap and bdrv_clear_dirty_bitmap
hbitmap: add hbitmap_merge
block: add bdrv_reclaim_dirty_bitmap
qmp: Add support of "dirty-bitmap" sync mode for drive-backup
Vladimir Sementsov-Ogievskiy (1):
block: fix spoiling all dirty bitmaps by mirror and migration
block.c | 157 ++++++++++++++++++++++++--
block/backup.c | 119 ++++++++++++++++----
block/mirror.c | 27 +++--
blockdev.c | 254 +++++++++++++++++++++++++++++++++++++++++-
hmp.c | 3 +-
include/block/block.h | 25 ++++-
include/block/block_int.h | 2 +
include/qemu/hbitmap.h | 19 ++++
migration/block.c | 7 +-
qapi-schema.json | 5 +-
qapi/block-core.json | 101 ++++++++++++++++-
qmp-commands.hx | 68 ++++++++++-
tests/qemu-iotests/056 | 33 +++++-
tests/qemu-iotests/056.out | 4 +-
tests/qemu-iotests/iotests.py | 8 ++
util/hbitmap.c | 48 ++++++++
16 files changed, 814 insertions(+), 66 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v10 01/13] block: fix spoiling all dirty bitmaps by mirror and migration
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
@ 2014-12-23 1:12 ` John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 02/13] qapi: Add optional field "name" to block dirty bitmap John Snow
` (12 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: John Snow @ 2014-12-23 1:12 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, famz, John Snow, armbru, mreitz, vsementsov, stefanha,
Denis V. Lunev
From: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
Mirror and migration use dirty bitmaps for their purposes, and since
commit [block: per caller dirty bitmap] they use their own bitmaps, not
the global one. But they use old functions bdrv_set_dirty and
bdrv_reset_dirty, which change all dirty bitmaps.
Named dirty bitmaps series by Fam and Snow are affected: mirroring and
migration will spoil all (not related to this mirroring or migration)
named dirty bitmaps.
This patch fixes this by adding bdrv_set_dirty_bitmap and
bdrv_reset_dirty_bitmap, which change concrete bitmap. Also, to prevent
such mistakes in future, old functions bdrv_(set,reset)_dirty are made
static, for internal block usage.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
CC: John Snow <jsnow@redhat.com>
CC: Fam Zheng <famz@redhat.com>
CC: Denis V. Lunev <den@openvz.org>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 23 ++++++++++++++++++++---
block/mirror.c | 11 +++++++----
include/block/block.h | 6 ++++--
migration/block.c | 5 +++--
4 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/block.c b/block.c
index 4165d42..6c1b49a 100644
--- a/block.c
+++ b/block.c
@@ -97,6 +97,10 @@ static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
static QLIST_HEAD(, BlockDriver) bdrv_drivers =
QLIST_HEAD_INITIALIZER(bdrv_drivers);
+static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
+ int nr_sectors);
+static void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
+ int nr_sectors);
/* If non-zero, use only whitelisted block drivers */
static int use_bdrv_whitelist;
@@ -5389,8 +5393,20 @@ void bdrv_dirty_iter_init(BlockDriverState *bs,
hbitmap_iter_init(hbi, bitmap->bitmap, 0);
}
-void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
- int nr_sectors)
+void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+ int64_t cur_sector, int nr_sectors)
+{
+ hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
+}
+
+void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+ int64_t cur_sector, int nr_sectors)
+{
+ hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
+}
+
+static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
+ int nr_sectors)
{
BdrvDirtyBitmap *bitmap;
QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
@@ -5398,7 +5414,8 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
}
}
-void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors)
+static void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
+ int nr_sectors)
{
BdrvDirtyBitmap *bitmap;
QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
diff --git a/block/mirror.c b/block/mirror.c
index 2c6dd2a..9019d1b 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -128,7 +128,8 @@ static void mirror_write_complete(void *opaque, int ret)
BlockDriverState *source = s->common.bs;
BlockErrorAction action;
- bdrv_set_dirty(source, op->sector_num, op->nb_sectors);
+ bdrv_set_dirty_bitmap(source, s->dirty_bitmap, op->sector_num,
+ op->nb_sectors);
action = mirror_error_action(s, false, -ret);
if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) {
s->ret = ret;
@@ -145,7 +146,8 @@ static void mirror_read_complete(void *opaque, int ret)
BlockDriverState *source = s->common.bs;
BlockErrorAction action;
- bdrv_set_dirty(source, op->sector_num, op->nb_sectors);
+ bdrv_set_dirty_bitmap(source, s->dirty_bitmap, op->sector_num,
+ op->nb_sectors);
action = mirror_error_action(s, true, -ret);
if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) {
s->ret = ret;
@@ -286,7 +288,8 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
next_sector += sectors_per_chunk;
}
- bdrv_reset_dirty(source, sector_num, nb_sectors);
+ bdrv_reset_dirty_bitmap(source, s->dirty_bitmap, sector_num,
+ nb_sectors);
/* Copy the dirty cluster. */
s->in_flight++;
@@ -442,7 +445,7 @@ static void coroutine_fn mirror_run(void *opaque)
assert(n > 0);
if (ret == 1) {
- bdrv_set_dirty(bs, sector_num, n);
+ bdrv_set_dirty_bitmap(bs, s->dirty_bitmap, sector_num, n);
sector_num = next;
} else {
sector_num += n;
diff --git a/include/block/block.h b/include/block/block.h
index 6e7275d..a77ed30 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -434,8 +434,10 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
-void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
-void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
+void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+ int64_t cur_sector, int nr_sectors);
+void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+ int64_t cur_sector, int nr_sectors);
void bdrv_dirty_iter_init(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
diff --git a/migration/block.c b/migration/block.c
index 74d9eb1..a0f908c 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -303,7 +303,7 @@ static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState *bmds)
blk->aiocb = bdrv_aio_readv(bs, cur_sector, &blk->qiov,
nr_sectors, blk_mig_read_cb, blk);
- bdrv_reset_dirty(bs, cur_sector, nr_sectors);
+ bdrv_reset_dirty_bitmap(bs, bmds->dirty_bitmap, cur_sector, nr_sectors);
qemu_mutex_unlock_iothread();
bmds->cur_sector = cur_sector + nr_sectors;
@@ -496,7 +496,8 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,
g_free(blk);
}
- bdrv_reset_dirty(bmds->bs, sector, nr_sectors);
+ bdrv_reset_dirty_bitmap(bmds->bs, bmds->dirty_bitmap, sector,
+ nr_sectors);
break;
}
sector += BDRV_SECTORS_PER_DIRTY_CHUNK;
--
1.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v10 02/13] qapi: Add optional field "name" to block dirty bitmap
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 01/13] block: fix spoiling all dirty bitmaps by mirror and migration John Snow
@ 2014-12-23 1:12 ` John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 03/13] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove John Snow
` (11 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: John Snow @ 2014-12-23 1:12 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, John Snow, armbru, mreitz, vsementsov, stefanha
From: Fam Zheng <famz@redhat.com>
This field will be set for user created dirty bitmap. Also pass in an
error pointer to bdrv_create_dirty_bitmap, so when a name is already
taken on this BDS, it can report an error message. This is not global
check, two BDSes can have dirty bitmap with a common name.
Implemented bdrv_find_dirty_bitmap to find a dirty bitmap by name, will
be used later when other QMP commands want to reference dirty bitmap by
name.
Add bdrv_dirty_bitmap_make_anon. This unsets the name of dirty bitmap.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block.c | 32 +++++++++++++++++++++++++++++++-
block/mirror.c | 2 +-
include/block/block.h | 7 ++++++-
migration/block.c | 2 +-
qapi/block-core.json | 4 +++-
5 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/block.c b/block.c
index 6c1b49a..bfeae6b 100644
--- a/block.c
+++ b/block.c
@@ -53,6 +53,7 @@
struct BdrvDirtyBitmap {
HBitmap *bitmap;
+ char *name;
QLIST_ENTRY(BdrvDirtyBitmap) list;
};
@@ -5323,7 +5324,28 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
return true;
}
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
+BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name)
+{
+ BdrvDirtyBitmap *bm;
+
+ assert(name);
+ QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
+ if (bm->name && !strcmp(name, bm->name)) {
+ return bm;
+ }
+ }
+ return NULL;
+}
+
+void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+{
+ g_free(bitmap->name);
+ bitmap->name = NULL;
+}
+
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
+ int granularity,
+ const char *name,
Error **errp)
{
int64_t bitmap_size;
@@ -5331,6 +5353,10 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
assert((granularity & (granularity - 1)) == 0);
+ if (name && bdrv_find_dirty_bitmap(bs, name)) {
+ error_setg(errp, "Bitmap already exists: %s", name);
+ return NULL;
+ }
granularity >>= BDRV_SECTOR_BITS;
assert(granularity);
bitmap_size = bdrv_nb_sectors(bs);
@@ -5341,6 +5367,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
}
bitmap = g_new0(BdrvDirtyBitmap, 1);
bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
+ bitmap->name = g_strdup(name);
QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
return bitmap;
}
@@ -5352,6 +5379,7 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
if (bm == bitmap) {
QLIST_REMOVE(bitmap, list);
hbitmap_free(bitmap->bitmap);
+ g_free(bitmap->name);
g_free(bitmap);
return;
}
@@ -5370,6 +5398,8 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
info->count = bdrv_get_dirty_count(bs, bm);
info->granularity =
((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
+ info->has_name = !!bm->name;
+ info->name = g_strdup(bm->name);
entry->value = info;
*plist = entry;
plist = &entry->next;
diff --git a/block/mirror.c b/block/mirror.c
index 9019d1b..d819952 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -702,7 +702,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
s->granularity = granularity;
s->buf_size = MAX(buf_size, granularity);
- s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, errp);
+ s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
if (!s->dirty_bitmap) {
return;
}
diff --git a/include/block/block.h b/include/block/block.h
index a77ed30..bf767af 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -429,8 +429,13 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov);
struct HBitmapIter;
typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
+ int granularity,
+ const char *name,
Error **errp);
+BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
+ const char *name);
+void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
diff --git a/migration/block.c b/migration/block.c
index a0f908c..4eeb72a 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -319,7 +319,7 @@ static int set_dirty_tracking(void)
QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
bmds->dirty_bitmap = bdrv_create_dirty_bitmap(bmds->bs, BLOCK_SIZE,
- NULL);
+ NULL, NULL);
if (!bmds->dirty_bitmap) {
ret = -errno;
goto fail;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 6e8db15..d176324 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -322,6 +322,8 @@
#
# Block dirty bitmap information.
#
+# @name: #optional the name of the dirty bitmap (Since 2.3)
+#
# @count: number of dirty bytes according to the dirty bitmap
#
# @granularity: granularity of the dirty bitmap in bytes (since 1.4)
@@ -329,7 +331,7 @@
# Since: 1.3
##
{ 'type': 'BlockDirtyInfo',
- 'data': {'count': 'int', 'granularity': 'int'} }
+ 'data': {'*name': 'str', 'count': 'int', 'granularity': 'int'} }
##
# @BlockInfo:
--
1.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v10 03/13] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 01/13] block: fix spoiling all dirty bitmaps by mirror and migration John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 02/13] qapi: Add optional field "name" to block dirty bitmap John Snow
@ 2014-12-23 1:12 ` John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 04/13] block: Introduce bdrv_dirty_bitmap_granularity() John Snow
` (10 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: John Snow @ 2014-12-23 1:12 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, John Snow, armbru, mreitz, vsementsov, stefanha
From: Fam Zheng <famz@redhat.com>
The new command pair is added to manage user created dirty bitmap. The
dirty bitmap's name is mandatory and must be unique for the same device,
but different devices can have bitmaps with the same names.
The granularity is an optional field. If it is not specified, we will
choose a default granularity based on the cluster size if available,
clamped to between 4K and 64K to mirror how the 'mirror' code was
already choosing granularity. If we do not have cluster size info
available, we choose 64K. This code has been factored out into a helper
shared with block/mirror.
This patch also introduces the 'block_dirty_bitmap_lookup' helper,
which takes a device name and a dirty bitmap name and validates the
lookup, returning NULL and setting errp if there is a problem with
either field. This helper will be re-used in future patches in this
series.
The types added to block-core.json will be re-used in future patches
in this series, see:
'qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable}'
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 20 ++++++++++
block/mirror.c | 10 +----
blockdev.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/block/block.h | 1 +
qapi/block-core.json | 55 +++++++++++++++++++++++++++
qmp-commands.hx | 51 +++++++++++++++++++++++++
6 files changed, 228 insertions(+), 9 deletions(-)
diff --git a/block.c b/block.c
index bfeae6b..3eb77ee 100644
--- a/block.c
+++ b/block.c
@@ -5417,6 +5417,26 @@ int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector
}
}
+/**
+ * Chooses a default granularity based on the existing cluster size,
+ * but clamped between [4K, 64K]. Defaults to 64K in the case that there
+ * is no cluster size information available.
+ */
+uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
+{
+ BlockDriverInfo bdi;
+ uint64_t granularity;
+
+ if (bdrv_get_info(bs, &bdi) >= 0 && bdi.cluster_size != 0) {
+ granularity = MAX(4096, bdi.cluster_size);
+ granularity = MIN(65536, granularity);
+ } else {
+ granularity = 65536;
+ }
+
+ return granularity;
+}
+
void bdrv_dirty_iter_init(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
{
diff --git a/block/mirror.c b/block/mirror.c
index d819952..fc545f1 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -667,15 +667,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
MirrorBlockJob *s;
if (granularity == 0) {
- /* Choose the default granularity based on the target file's cluster
- * size, clamped between 4k and 64k. */
- BlockDriverInfo bdi;
- if (bdrv_get_info(target, &bdi) >= 0 && bdi.cluster_size != 0) {
- granularity = MAX(4096, bdi.cluster_size);
- granularity = MIN(65536, granularity);
- } else {
- granularity = 65536;
- }
+ granularity = bdrv_get_default_bitmap_granularity(target);
}
assert ((granularity & (granularity - 1)) == 0);
diff --git a/blockdev.c b/blockdev.c
index 5651a8e..95251c7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1173,6 +1173,48 @@ out_aio_context:
return NULL;
}
+/**
+ * Return a dirty bitmap (if present), after validating
+ * the node reference and bitmap names. Returns NULL on error,
+ * including when the BDS and/or bitmap is not found.
+ */
+static BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node_ref,
+ const char *name,
+ BlockDriverState **pbs,
+ Error **errp)
+{
+ BlockDriverState *bs;
+ BdrvDirtyBitmap *bitmap;
+
+ if (!node_ref) {
+ error_setg(errp, "Node reference cannot be NULL");
+ return NULL;
+ }
+ if (!name) {
+ error_setg(errp, "Bitmap name cannot be NULL");
+ return NULL;
+ }
+
+ bs = bdrv_lookup_bs(node_ref, node_ref, errp);
+ if (!bs) {
+ error_setg(errp, "Node reference '%s' not found", node_ref);
+ return NULL;
+ }
+
+ /* If caller provided a BDS*, provide the result of that lookup, too. */
+ if (pbs) {
+ *pbs = bs;
+ }
+
+ bitmap = bdrv_find_dirty_bitmap(bs, name);
+ if (!bitmap) {
+ error_setg(errp, "Dirty bitmap not found: %s", name);
+ return NULL;
+ }
+
+ return bitmap;
+}
+
/* New and old BlockDriverState structs for atomic group operations */
typedef struct BlkTransactionState BlkTransactionState;
@@ -1894,6 +1936,64 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
aio_context_release(aio_context);
}
+void qmp_block_dirty_bitmap_add(const char *node_ref, const char *name,
+ bool has_granularity, int64_t granularity,
+ Error **errp)
+{
+ AioContext *aio_context;
+ BlockDriverState *bs;
+
+ if (!name || name[0] == '\0') {
+ error_setg(errp, "Bitmap name cannot be empty");
+ return;
+ }
+
+ bs = bdrv_lookup_bs(node_ref, node_ref, errp);
+ if (!bs) {
+ return;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ if (has_granularity) {
+ if (granularity < 512 || !is_power_of_2(granularity)) {
+ error_setg(errp, "Granularity must be power of 2 "
+ "and at least 512");
+ goto out;
+ }
+ } else {
+ /* Default to cluster size, if available: */
+ granularity = bdrv_get_default_bitmap_granularity(bs);
+ }
+
+ bdrv_create_dirty_bitmap(bs, granularity, name, errp);
+
+ out:
+ aio_context_release(aio_context);
+}
+
+void qmp_block_dirty_bitmap_remove(const char *node_ref, const char *name,
+ Error **errp)
+{
+ AioContext *aio_context;
+ BlockDriverState *bs;
+ BdrvDirtyBitmap *bitmap;
+
+ bitmap = block_dirty_bitmap_lookup(node_ref, name, &bs, errp);
+ if (!bitmap || !bs) {
+ return;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ bdrv_dirty_bitmap_make_anon(bs, bitmap);
+ bdrv_release_dirty_bitmap(bs, bitmap);
+
+ aio_context_release(aio_context);
+}
+
int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
const char *id = qdict_get_str(qdict, "id");
diff --git a/include/block/block.h b/include/block/block.h
index bf767af..44dd6ca 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -438,6 +438,7 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
+uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index d176324..f79d165 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -894,6 +894,61 @@
'*on-target-error': 'BlockdevOnError' } }
##
+# @BlockDirtyBitmap
+#
+# @node-ref: name of device/node-name which the bitmap is tracking
+#
+# @name: name of the dirty bitmap
+#
+# Since 2.3
+##
+{ 'type': 'BlockDirtyBitmap',
+ 'data': { 'node-ref': 'str', 'name': 'str' } }
+
+##
+# @BlockDirtyBitmapAdd
+#
+# @node-ref: name of device/node-name which the bitmap is tracking
+#
+# @name: name of the dirty bitmap
+#
+# @granularity: #optional the bitmap granularity, default is 64k for
+# block-dirty-bitmap-add
+#
+# Since 2.3
+##
+{ 'type': 'BlockDirtyBitmapAdd',
+ 'data': { 'node-ref': 'str', 'name': 'str', '*granularity': 'int' } }
+
+##
+# @block-dirty-bitmap-add
+#
+# Create a dirty bitmap with a name on the node
+#
+# Returns: nothing on success
+# If @node-ref is not a valid block device, DeviceNotFound
+# If @name is already taken, GenericError with an explanation
+#
+# Since 2.3
+##
+{'command': 'block-dirty-bitmap-add',
+ 'data': 'BlockDirtyBitmapAdd' }
+
+##
+# @block-dirty-bitmap-remove
+#
+# Remove a dirty bitmap on the node
+#
+# Returns: nothing on success
+# If @node-ref is not a valid block device, DeviceNotFound
+# If @name is not found, GenericError with an explanation
+#
+# Since 2.3
+##
+{'command': 'block-dirty-bitmap-remove',
+ 'data': 'BlockDirtyBitmap' }
+
+##
# @block_set_io_throttle:
#
# Change I/O throttle limits for a block drive.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6945d30..4ffca8a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1202,6 +1202,57 @@ Example:
EQMP
{
+ .name = "block-dirty-bitmap-add",
+ .args_type = "node-ref:B,name:s,granularity:i?",
+ .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_add,
+ },
+ {
+ .name = "block-dirty-bitmap-remove",
+ .args_type = "node-ref:B,name:s",
+ .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_remove,
+ },
+
+SQMP
+
+block-dirty-bitmap-add
+----------------------
+Since 2.3
+
+Create a dirty bitmap with a name on the device, and start tracking the writes.
+
+Arguments:
+
+- "node-ref": device/node on which to create dirty bitmap (json-string)
+- "name": name of the new dirty bitmap (json-string)
+- "granularity": granularity to track writes with (int, optional)
+
+Example:
+
+-> { "execute": "block-dirty-bitmap-add", "arguments": { "node-ref": "drive0",
+ "name": "bitmap0" } }
+<- { "return": {} }
+
+block-dirty-bitmap-remove
+-------------------------
+Since 2.3
+
+Stop write tracking and remove the dirty bitmap that was created with
+block-dirty-bitmap-add.
+
+Arguments:
+
+- "node-ref": device/node on which to remove dirty bitmap (json-string)
+- "name": name of the dirty bitmap to remove (json-string)
+
+Example:
+
+-> { "execute": "block-dirty-bitmap-remove", "arguments": { "node-ref": "drive0",
+ "name": "bitmap0" } }
+<- { "return": {} }
+
+EQMP
+
+ {
.name = "blockdev-snapshot-sync",
.args_type = "device:s?,node-name:s?,snapshot-file:s,snapshot-node-name:s?,format:s?,mode:s?",
.mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot_sync,
--
1.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v10 04/13] block: Introduce bdrv_dirty_bitmap_granularity()
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
` (2 preceding siblings ...)
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 03/13] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove John Snow
@ 2014-12-23 1:12 ` John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 05/13] hbitmap: Add hbitmap_copy John Snow
` (9 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: John Snow @ 2014-12-23 1:12 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, John Snow, armbru, mreitz, vsementsov, stefanha
From: Fam Zheng <famz@redhat.com>
This returns the granularity (in bytes) of dirty bitmap,
which matches the QMP interface and the existing query
interface.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 9 +++++++--
include/block/block.h | 2 ++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 3eb77ee..a1d9e88 100644
--- a/block.c
+++ b/block.c
@@ -5396,8 +5396,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
BlockDirtyInfo *info = g_new0(BlockDirtyInfo, 1);
BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
info->count = bdrv_get_dirty_count(bs, bm);
- info->granularity =
- ((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
+ info->granularity = bdrv_dirty_bitmap_granularity(bs, bm);
info->has_name = !!bm->name;
info->name = g_strdup(bm->name);
entry->value = info;
@@ -5437,6 +5436,12 @@ uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
return granularity;
}
+uint64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap)
+{
+ return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
+}
+
void bdrv_dirty_iter_init(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
{
diff --git a/include/block/block.h b/include/block/block.h
index 44dd6ca..c7402e7 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -439,6 +439,8 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
+uint64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
--
1.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v10 05/13] hbitmap: Add hbitmap_copy
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
` (3 preceding siblings ...)
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 04/13] block: Introduce bdrv_dirty_bitmap_granularity() John Snow
@ 2014-12-23 1:12 ` John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 06/13] block: Add bdrv_copy_dirty_bitmap and bdrv_clear_dirty_bitmap John Snow
` (8 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: John Snow @ 2014-12-23 1:12 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, John Snow, armbru, mreitz, vsementsov, stefanha
From: Fam Zheng <famz@redhat.com>
This makes a deep copy of an HBitmap.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
include/qemu/hbitmap.h | 8 ++++++++
util/hbitmap.c | 20 ++++++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 550d7ce..b645cfc 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -65,6 +65,14 @@ struct HBitmapIter {
HBitmap *hbitmap_alloc(uint64_t size, int granularity);
/**
+ * hbitmap_copy:
+ * @bitmap: The original bitmap to copy.
+ *
+ * Copy a HBitmap.
+ */
+HBitmap *hbitmap_copy(const HBitmap *bitmap);
+
+/**
* hbitmap_empty:
* @hb: HBitmap to operate on.
*
diff --git a/util/hbitmap.c b/util/hbitmap.c
index ab13971..033ad58 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -90,6 +90,10 @@ struct HBitmap {
* bitmap will still allocate HBITMAP_LEVELS arrays.
*/
unsigned long *levels[HBITMAP_LEVELS];
+
+ /* NB: If additional objects that require a deep copy are added to
+ * this structure, the hbitmap_copy function should be updated accordingly.
+ */
};
/* Advance hbi to the next nonzero word and return it. hbi->pos
@@ -395,3 +399,19 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
hb->levels[0][0] |= 1UL << (BITS_PER_LONG - 1);
return hb;
}
+
+HBitmap *hbitmap_copy(const HBitmap *bitmap)
+{
+ int i;
+ uint64_t size;
+ HBitmap *hb = g_memdup(bitmap, sizeof(HBitmap));
+
+ size = bitmap->size;
+ for (i = HBITMAP_LEVELS - 1; i >= 0; i--) {
+ size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+ hb->levels[i] = g_memdup(bitmap->levels[i],
+ size * sizeof(unsigned long));
+ }
+
+ return hb;
+}
--
1.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v10 06/13] block: Add bdrv_copy_dirty_bitmap and bdrv_clear_dirty_bitmap
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
` (4 preceding siblings ...)
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 05/13] hbitmap: Add hbitmap_copy John Snow
@ 2014-12-23 1:12 ` John Snow
2014-12-30 13:47 ` Vladimir Sementsov-Ogievskiy
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 07/13] hbitmap: add hbitmap_merge John Snow
` (7 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: John Snow @ 2014-12-23 1:12 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, John Snow, armbru, mreitz, vsementsov, stefanha
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 39 +++++++++++++++++++++++++++++++++++----
include/block/block.h | 4 ++++
2 files changed, 39 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index a1d9e88..f9e0767 100644
--- a/block.c
+++ b/block.c
@@ -53,6 +53,8 @@
struct BdrvDirtyBitmap {
HBitmap *bitmap;
+ int64_t size;
+ int64_t granularity;
char *name;
QLIST_ENTRY(BdrvDirtyBitmap) list;
};
@@ -5343,6 +5345,21 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
bitmap->name = NULL;
}
+BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap,
+ const char *name)
+{
+ BdrvDirtyBitmap *new_bitmap;
+
+ new_bitmap = g_malloc0(sizeof(BdrvDirtyBitmap));
+ new_bitmap->bitmap = hbitmap_copy(bitmap->bitmap);
+ new_bitmap->size = bitmap->size;
+ new_bitmap->granularity = bitmap->granularity;
+ new_bitmap->name = g_strdup(name);
+ QLIST_INSERT_HEAD(&bs->dirty_bitmaps, new_bitmap, list);
+ return new_bitmap;
+}
+
BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
int granularity,
const char *name,
@@ -5350,6 +5367,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
{
int64_t bitmap_size;
BdrvDirtyBitmap *bitmap;
+ int sector_granularity;
assert((granularity & (granularity - 1)) == 0);
@@ -5357,8 +5375,8 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
error_setg(errp, "Bitmap already exists: %s", name);
return NULL;
}
- granularity >>= BDRV_SECTOR_BITS;
- assert(granularity);
+ sector_granularity = granularity >> BDRV_SECTOR_BITS;
+ assert(sector_granularity);
bitmap_size = bdrv_nb_sectors(bs);
if (bitmap_size < 0) {
error_setg_errno(errp, -bitmap_size, "could not get length of device");
@@ -5366,7 +5384,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
return NULL;
}
bitmap = g_new0(BdrvDirtyBitmap, 1);
- bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
+ bitmap->size = bitmap_size;
+ bitmap->granularity = granularity;
+ bitmap->bitmap = hbitmap_alloc(bitmap->size, ffs(sector_granularity) - 1);
bitmap->name = g_strdup(name);
QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
return bitmap;
@@ -5439,7 +5459,9 @@ uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
uint64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap)
{
- return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
+ g_assert(BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap) ==
+ bitmap->granularity);
+ return bitmap->granularity;
}
void bdrv_dirty_iter_init(BlockDriverState *bs,
@@ -5460,6 +5482,15 @@ void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
}
+/**
+ * Effectively, reset the hbitmap from bits [0, size)
+ * Synonymous with bdrv_reset_dirty_bitmap(bs, bitmap, 0, bitmap->size)
+ */
+void bdrv_clear_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+{
+ hbitmap_reset(bitmap->bitmap, 0, bitmap->size);
+}
+
static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
int nr_sectors)
{
diff --git a/include/block/block.h b/include/block/block.h
index c7402e7..e964abd 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -436,6 +436,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
const char *name);
void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap,
+ const char *name);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
@@ -446,6 +449,7 @@ void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
+void bdrv_clear_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_dirty_iter_init(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
--
1.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v10 07/13] hbitmap: add hbitmap_merge
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
` (5 preceding siblings ...)
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 06/13] block: Add bdrv_copy_dirty_bitmap and bdrv_clear_dirty_bitmap John Snow
@ 2014-12-23 1:12 ` John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 08/13] block: add bdrv_reclaim_dirty_bitmap John Snow
` (6 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: John Snow @ 2014-12-23 1:12 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, John Snow, armbru, mreitz, vsementsov, stefanha
We add a bitmap merge operation to assist in error cases
where we wish to combine two bitmaps together.
This is algorithmically O(bits) provided HBITMAP_LEVELS remains
constant. For a full bitmap on a 64bit machine:
sum(bits/64^k, k, 0, HBITMAP_LEVELS) ~= 1.01587 * bits
We may be able to improve running speed for particularly sparse
bitmaps by using iterators, but the running time for dense maps
will be worse.
We present the simpler solution first, and we can refine it later
if needed.
Signed-off-by: John Snow <jsnow@redhat.com>
---
include/qemu/hbitmap.h | 11 +++++++++++
util/hbitmap.c | 28 ++++++++++++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index b645cfc..c48c50a 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -73,6 +73,17 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity);
HBitmap *hbitmap_copy(const HBitmap *bitmap);
/**
+ * hbitmap_merge:
+ * @a: The bitmap to store the result in.
+ * @b: The bitmap to merge into @a.
+ *
+ * Merge two bitmaps together.
+ * A := A (BITOR) B.
+ * B is left unmodified.
+ */
+bool hbitmap_merge(HBitmap *a, const HBitmap *b);
+
+/**
* hbitmap_empty:
* @hb: HBitmap to operate on.
*
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 033ad58..f400dcb 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -415,3 +415,31 @@ HBitmap *hbitmap_copy(const HBitmap *bitmap)
return hb;
}
+
+/**
+ * Given HBitmaps A and B, let A := A (BITOR) B.
+ * Bitmap B will not be modified.
+ */
+bool hbitmap_merge(HBitmap *a, const HBitmap *b)
+{
+ int i, j;
+ uint64_t size;
+
+ if ((a->size != b->size) || (a->granularity != b->granularity)) {
+ return false;
+ }
+
+ /* This merge is O(size), as BITS_PER_LONG and HBITMAP_LEVELS are constant.
+ * It may be possible to improve running times for sparsely populated maps
+ * by using hbitmap_iter_next, but this is suboptimal for dense maps.
+ */
+ size = a->size;
+ for (i = HBITMAP_LEVELS - 1; i >= 0; i--) {
+ size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+ for (j = 0; j < size; j++) {
+ a->levels[i][j] |= b->levels[i][j];
+ }
+ }
+
+ return true;
+}
--
1.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v10 08/13] block: add bdrv_reclaim_dirty_bitmap
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
` (6 preceding siblings ...)
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 07/13] hbitmap: add hbitmap_merge John Snow
@ 2014-12-23 1:12 ` John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 09/13] qmp: Add block-dirty-bitmap-enable and block-dirty-bitmap-disable John Snow
` (5 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: John Snow @ 2014-12-23 1:12 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, John Snow, armbru, mreitz, vsementsov, stefanha
For a synchronization bitmap that has been used for
a failed operation, we may wish to merge that bitmap
back with the bitmap we copied it from so that no
dirty bit tracking information is lost due to the
failed operation.
For this operation, I add bdrv_reclaim_dirty_bitmap,
which takes a BdrvDirtyBitmap associated with a failed
operation and merges the bitmap back with the BdrvDirtyBitmap
that spawned it, then frees this failed bitmap.
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 13 +++++++++++++
include/block/block.h | 2 ++
2 files changed, 15 insertions(+)
diff --git a/block.c b/block.c
index f9e0767..990b9eb 100644
--- a/block.c
+++ b/block.c
@@ -53,6 +53,7 @@
struct BdrvDirtyBitmap {
HBitmap *bitmap;
+ BdrvDirtyBitmap *originator;
int64_t size;
int64_t granularity;
char *name;
@@ -5352,6 +5353,7 @@ BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
BdrvDirtyBitmap *new_bitmap;
new_bitmap = g_malloc0(sizeof(BdrvDirtyBitmap));
+ new_bitmap->originator = bitmap;
new_bitmap->bitmap = hbitmap_copy(bitmap->bitmap);
new_bitmap->size = bitmap->size;
new_bitmap->granularity = bitmap->granularity;
@@ -5360,6 +5362,17 @@ BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
return new_bitmap;
}
+BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
+ BdrvDirtyBitmap *failed)
+{
+ BdrvDirtyBitmap *originator = failed->originator;
+ hbitmap_merge(originator->bitmap, failed->bitmap);
+ bdrv_release_dirty_bitmap(bs, failed);
+
+ return originator;
+}
+
+
BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
int granularity,
const char *name,
diff --git a/include/block/block.h b/include/block/block.h
index e964abd..e51ca45 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -439,6 +439,8 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap,
const char *name);
+BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
+ BdrvDirtyBitmap *failed);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
--
1.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v10 09/13] qmp: Add block-dirty-bitmap-enable and block-dirty-bitmap-disable
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
` (7 preceding siblings ...)
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 08/13] block: add bdrv_reclaim_dirty_bitmap John Snow
@ 2014-12-23 1:12 ` John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 10/13] qmp: Add support of "dirty-bitmap" sync mode for drive-backup John Snow
` (4 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: John Snow @ 2014-12-23 1:12 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, John Snow, armbru, mreitz, vsementsov, stefanha
From: Fam Zheng <famz@redhat.com>
This allows to put the dirty bitmap into a disabled state where no more
writes will be tracked.
It will be used before backup or writing to persistent file.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 19 ++++++++++++++++++-
blockdev.c | 40 ++++++++++++++++++++++++++++++++++++++++
include/block/block.h | 2 ++
qapi/block-core.json | 28 ++++++++++++++++++++++++++++
qmp-commands.hx | 10 ++++++++++
5 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 990b9eb..166a04f 100644
--- a/block.c
+++ b/block.c
@@ -57,6 +57,7 @@ struct BdrvDirtyBitmap {
int64_t size;
int64_t granularity;
char *name;
+ bool enabled;
QLIST_ENTRY(BdrvDirtyBitmap) list;
};
@@ -5401,6 +5402,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
bitmap->granularity = granularity;
bitmap->bitmap = hbitmap_alloc(bitmap->size, ffs(sector_granularity) - 1);
bitmap->name = g_strdup(name);
+ bitmap->enabled = true;
QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
return bitmap;
}
@@ -5419,6 +5421,16 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
}
}
+void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+ bitmap->enabled = false;
+}
+
+void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+ bitmap->enabled = true;
+}
+
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
{
BdrvDirtyBitmap *bm;
@@ -5486,7 +5498,9 @@ void bdrv_dirty_iter_init(BlockDriverState *bs,
void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors)
{
- hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
+ if (bitmap->enabled) {
+ hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
+ }
}
void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
@@ -5509,6 +5523,9 @@ static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
{
BdrvDirtyBitmap *bitmap;
QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
+ if (!bitmap->enabled) {
+ continue;
+ }
hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
}
}
diff --git a/blockdev.c b/blockdev.c
index 95251c7..118cb6c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1994,6 +1994,46 @@ void qmp_block_dirty_bitmap_remove(const char *node_ref, const char *name,
aio_context_release(aio_context);
}
+void qmp_block_dirty_bitmap_enable(const char *node_ref, const char *name,
+ Error **errp)
+{
+ AioContext *aio_context;
+ BdrvDirtyBitmap *bitmap;
+ BlockDriverState *bs;
+
+ bitmap = block_dirty_bitmap_lookup(node_ref, name, &bs, errp);
+ if (!bitmap) {
+ return;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ bdrv_enable_dirty_bitmap(bitmap);
+
+ aio_context_release(aio_context);
+}
+
+void qmp_block_dirty_bitmap_disable(const char *node_ref, const char *name,
+ Error **errp)
+{
+ AioContext *aio_context;
+ BdrvDirtyBitmap *bitmap;
+ BlockDriverState *bs;
+
+ bitmap = block_dirty_bitmap_lookup(node_ref, name, &bs, errp);
+ if (!bitmap) {
+ return;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ bdrv_disable_dirty_bitmap(bitmap);
+
+ aio_context_release(aio_context);
+}
+
int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
const char *id = qdict_get_str(qdict, "id");
diff --git a/include/block/block.h b/include/block/block.h
index e51ca45..8b18f7c 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_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
+void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
uint64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f79d165..3e863b9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -949,6 +949,34 @@
'data': 'BlockDirtyBitmap' }
##
+# @block-dirty-bitmap-enable
+#
+# Enable a dirty bitmap on the device
+#
+# Returns: nothing on success
+# If @node-ref is not a valid block device, DeviceNotFound
+# If @name is not found, GenericError with an explaining message
+#
+# Since 2.3
+##
+{'command': 'block-dirty-bitmap-enable',
+ 'data': 'BlockDirtyBitmap' }
+
+##
+# @block-dirty-bitmap-disable
+#
+# Disable a dirty bitmap on the device
+#
+# Returns: nothing on success
+# If @node-ref is not a valid block device, DeviceNotFound
+# If @name is not found, GenericError with an explaining message
+#
+# Since 2.3
+##
+{'command': 'block-dirty-bitmap-disable',
+ 'data': 'BlockDirtyBitmap' }
+
+##
# @block_set_io_throttle:
#
# Change I/O throttle limits for a block drive.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4ffca8a..59be8eb 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1211,6 +1211,16 @@ EQMP
.args_type = "node-ref:B,name:s",
.mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_remove,
},
+ {
+ .name = "block-dirty-bitmap-enable",
+ .args_type = "node-ref:B,name:s",
+ .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_enable,
+ },
+ {
+ .name = "block-dirty-bitmap-disable",
+ .args_type = "node-ref:B,name:s",
+ .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_disable,
+ },
SQMP
--
1.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v10 10/13] qmp: Add support of "dirty-bitmap" sync mode for drive-backup
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
` (8 preceding siblings ...)
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 09/13] qmp: Add block-dirty-bitmap-enable and block-dirty-bitmap-disable John Snow
@ 2014-12-23 1:12 ` John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 11/13] qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable} John Snow
` (3 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: John Snow @ 2014-12-23 1:12 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, John Snow, armbru, mreitz, vsementsov, stefanha
For "dirty-bitmap" sync mode, the block job will iterate through the
given dirty bitmap to decide if a sector needs backup (backup all the
dirty clusters and skip clean ones), just as allocation conditions of
"top" sync mode.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 5 ++
block/backup.c | 119 ++++++++++++++++++++++++++++++++++++++--------
block/mirror.c | 4 ++
blockdev.c | 14 +++++-
hmp.c | 3 +-
include/block/block.h | 1 +
include/block/block_int.h | 2 +
qapi/block-core.json | 11 +++--
qmp-commands.hx | 7 +--
9 files changed, 136 insertions(+), 30 deletions(-)
diff --git a/block.c b/block.c
index 166a04f..3d1337b 100644
--- a/block.c
+++ b/block.c
@@ -5539,6 +5539,11 @@ static void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
}
}
+void bdrv_set_dirty_iter(HBitmapIter *hbi, int64_t offset)
+{
+ hbitmap_iter_init(hbi, hbi->hb, offset);
+}
+
int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
{
return hbitmap_count(bitmap->bitmap);
diff --git a/block/backup.c b/block/backup.c
index 792e655..f6ecb40 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -37,6 +37,8 @@ typedef struct CowRequest {
typedef struct BackupBlockJob {
BlockJob common;
BlockDriverState *target;
+ /* bitmap for sync=dirty-bitmap */
+ BdrvDirtyBitmap *sync_bitmap;
MirrorSyncMode sync_mode;
RateLimit limit;
BlockdevOnError on_source_error;
@@ -242,6 +244,31 @@ static void backup_complete(BlockJob *job, void *opaque)
g_free(data);
}
+static bool coroutine_fn yield_and_check(BackupBlockJob *job)
+{
+ if (block_job_is_cancelled(&job->common)) {
+ return true;
+ }
+
+ /* we need to yield so that qemu_aio_flush() returns.
+ * (without, VM does not reboot)
+ */
+ if (job->common.speed) {
+ uint64_t delay_ns = ratelimit_calculate_delay(&job->limit,
+ job->sectors_read);
+ job->sectors_read = 0;
+ block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, delay_ns);
+ } else {
+ block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, 0);
+ }
+
+ if (block_job_is_cancelled(&job->common)) {
+ return true;
+ }
+
+ return false;
+}
+
static void coroutine_fn backup_run(void *opaque)
{
BackupBlockJob *job = opaque;
@@ -254,13 +281,13 @@ static void coroutine_fn backup_run(void *opaque)
};
int64_t start, end;
int ret = 0;
+ bool error_is_read;
QLIST_INIT(&job->inflight_reqs);
qemu_co_rwlock_init(&job->flush_rwlock);
start = 0;
- end = DIV_ROUND_UP(job->common.len / BDRV_SECTOR_SIZE,
- BACKUP_SECTORS_PER_CLUSTER);
+ end = DIV_ROUND_UP(job->common.len, BACKUP_CLUSTER_SIZE);
job->bitmap = hbitmap_alloc(end, 0);
@@ -278,28 +305,44 @@ static void coroutine_fn backup_run(void *opaque)
qemu_coroutine_yield();
job->common.busy = true;
}
+ } else if (job->sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
+ /* Dirty Bitmap sync has a slightly different iteration method */
+ HBitmapIter hbi;
+ int64_t sector;
+ int64_t cluster;
+ bool polyrhythmic;
+
+ bdrv_dirty_iter_init(bs, job->sync_bitmap, &hbi);
+ /* Does the granularity happen to match our backup cluster size? */
+ polyrhythmic = (bdrv_dirty_bitmap_granularity(bs, job->sync_bitmap) != BACKUP_CLUSTER_SIZE);
+
+ /* Find the next dirty /sector/ and copy that /cluster/ */
+ while ((sector = hbitmap_iter_next(&hbi)) != -1) {
+ if (yield_and_check(job)) {
+ goto leave;
+ }
+ cluster = sector / BACKUP_SECTORS_PER_CLUSTER;
+
+ do {
+ ret = backup_do_cow(bs, cluster * BACKUP_SECTORS_PER_CLUSTER,
+ BACKUP_SECTORS_PER_CLUSTER, &error_is_read);
+ if ((ret < 0) &&
+ backup_error_action(job, error_is_read, -ret) ==
+ BLOCK_ERROR_ACTION_REPORT) {
+ goto leave;
+ }
+ } while (ret < 0);
+
+ /* Advance (or rewind) our iterator if we need to. */
+ if (polyrhythmic) {
+ bdrv_set_dirty_iter(&hbi,
+ (cluster + 1) * BACKUP_SECTORS_PER_CLUSTER);
+ }
+ }
} else {
/* Both FULL and TOP SYNC_MODE's require copying.. */
for (; start < end; start++) {
- bool error_is_read;
-
- if (block_job_is_cancelled(&job->common)) {
- break;
- }
-
- /* we need to yield so that qemu_aio_flush() returns.
- * (without, VM does not reboot)
- */
- if (job->common.speed) {
- uint64_t delay_ns = ratelimit_calculate_delay(
- &job->limit, job->sectors_read);
- job->sectors_read = 0;
- block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, delay_ns);
- } else {
- block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, 0);
- }
-
- if (block_job_is_cancelled(&job->common)) {
+ if (yield_and_check(job)) {
break;
}
@@ -351,12 +394,22 @@ static void coroutine_fn backup_run(void *opaque)
}
}
+leave:
notifier_with_return_remove(&before_write);
/* wait until pending backup_do_cow() calls have completed */
qemu_co_rwlock_wrlock(&job->flush_rwlock);
qemu_co_rwlock_unlock(&job->flush_rwlock);
+ if (job->sync_bitmap) {
+ if (ret < 0) {
+ /* Merges the sync_bitmap back into the bitmap it was split from,
+ * and frees this bitmap. */
+ bdrv_reclaim_dirty_bitmap(bs, job->sync_bitmap);
+ } else {
+ bdrv_release_dirty_bitmap(bs, job->sync_bitmap);
+ }
+ }
hbitmap_free(job->bitmap);
bdrv_iostatus_disable(target);
@@ -368,12 +421,14 @@ static void coroutine_fn backup_run(void *opaque)
void backup_start(BlockDriverState *bs, BlockDriverState *target,
int64_t speed, MirrorSyncMode sync_mode,
+ BdrvDirtyBitmap *sync_bitmap,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
BlockCompletionFunc *cb, void *opaque,
Error **errp)
{
int64_t len;
+ BdrvDirtyBitmap *original;
assert(bs);
assert(target);
@@ -386,6 +441,26 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
return;
}
+ if (sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
+ if (!sync_bitmap) {
+ error_setg(errp, "must provide a valid bitmap name for "
+ "\"dirty-bitmap\" sync mode");
+ return;
+ }
+
+ original = sync_bitmap;
+ sync_bitmap = bdrv_copy_dirty_bitmap(bs, original, NULL);
+ bdrv_clear_dirty_bitmap(bs, original);
+ bdrv_disable_dirty_bitmap(sync_bitmap);
+
+ } else if (sync_bitmap) {
+ error_setg(errp,
+ "a sync_bitmap was provided to backup_run, "
+ "but received an incompatible sync_mode (%s)",
+ MirrorSyncMode_lookup[sync_mode]);
+ return;
+ }
+
len = bdrv_getlength(bs);
if (len < 0) {
error_setg_errno(errp, -len, "unable to get length for '%s'",
@@ -403,6 +478,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
job->on_target_error = on_target_error;
job->target = target;
job->sync_mode = sync_mode;
+ job->sync_bitmap = sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP ?
+ sync_bitmap : NULL;
job->common.len = len;
job->common.co = qemu_coroutine_create(backup_run);
qemu_coroutine_enter(job->common.co, job);
diff --git a/block/mirror.c b/block/mirror.c
index fc545f1..427c688 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -717,6 +717,10 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
bool is_none_mode;
BlockDriverState *base;
+ if (mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
+ error_setg(errp, "Sync mode 'dirty-bitmap' not supported");
+ return;
+ }
is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
base = mode == MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL;
mirror_start_job(bs, target, replaces,
diff --git a/blockdev.c b/blockdev.c
index 118cb6c..0b38571 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1569,6 +1569,7 @@ static void drive_backup_prepare(BlkTransactionState *common, Error **errp)
backup->sync,
backup->has_mode, backup->mode,
backup->has_speed, backup->speed,
+ backup->has_bitmap, backup->bitmap,
backup->has_on_source_error, backup->on_source_error,
backup->has_on_target_error, backup->on_target_error,
&local_err);
@@ -2344,6 +2345,7 @@ void qmp_drive_backup(const char *device, const char *target,
enum MirrorSyncMode sync,
bool has_mode, enum NewImageMode mode,
bool has_speed, int64_t speed,
+ bool has_bitmap, const char *bitmap,
bool has_on_source_error, BlockdevOnError on_source_error,
bool has_on_target_error, BlockdevOnError on_target_error,
Error **errp)
@@ -2351,6 +2353,7 @@ void qmp_drive_backup(const char *device, const char *target,
BlockDriverState *bs;
BlockDriverState *target_bs;
BlockDriverState *source = NULL;
+ BdrvDirtyBitmap *bmap = NULL;
AioContext *aio_context;
BlockDriver *drv = NULL;
Error *local_err = NULL;
@@ -2446,7 +2449,16 @@ void qmp_drive_backup(const char *device, const char *target,
bdrv_set_aio_context(target_bs, aio_context);
- backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
+ if (has_bitmap) {
+ bmap = bdrv_find_dirty_bitmap(bs, bitmap);
+ if (!bmap) {
+ error_setg(errp, "Bitmap '%s' could not be found", bitmap);
+ goto out;
+ }
+ }
+
+ backup_start(bs, target_bs, speed, sync, bmap,
+ on_source_error, on_target_error,
block_job_cb, bs, &local_err);
if (local_err != NULL) {
bdrv_unref(target_bs);
diff --git a/hmp.c b/hmp.c
index 481be80..63b19c7 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1021,7 +1021,8 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
qmp_drive_backup(device, filename, !!format, format,
full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
- true, mode, false, 0, false, 0, false, 0, &err);
+ true, mode, false, 0, false, NULL,
+ false, 0, false, 0, &err);
hmp_handle_error(mon, &err);
}
diff --git a/include/block/block.h b/include/block/block.h
index 8b18f7c..cb1f28d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -456,6 +456,7 @@ void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
void bdrv_clear_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
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);
void bdrv_enable_copy_on_read(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 06a21dd..cb1e4a1 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -584,6 +584,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
* @target: Block device to write to.
* @speed: The maximum speed, in bytes per second, or 0 for unlimited.
* @sync_mode: What parts of the disk image should be copied to the destination.
+ * @sync_bitmap: The dirty bitmap if sync_mode is MIRROR_SYNC_MODE_DIRTY_BITMAP.
* @on_source_error: The action to take upon error reading from the source.
* @on_target_error: The action to take upon error writing to the target.
* @cb: Completion function for the job.
@@ -594,6 +595,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
*/
void backup_start(BlockDriverState *bs, BlockDriverState *target,
int64_t speed, MirrorSyncMode sync_mode,
+ BdrvDirtyBitmap *sync_bitmap,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
BlockCompletionFunc *cb, void *opaque,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 3e863b9..92c3a84 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -500,7 +500,7 @@
# Since: 1.3
##
{ 'enum': 'MirrorSyncMode',
- 'data': ['top', 'full', 'none'] }
+ 'data': ['top', 'full', 'none', 'dirty-bitmap'] }
##
# @BlockJobType:
@@ -675,14 +675,17 @@
# probe if @mode is 'existing', else the format of the source
#
# @sync: what parts of the disk image should be copied to the destination
-# (all the disk, only the sectors allocated in the topmost image, or
-# only new I/O).
+# (all the disk, only the sectors allocated in the topmost image, from a
+# dirty bitmap, or only new I/O).
#
# @mode: #optional whether and how QEMU should create a new image, default is
# 'absolute-paths'.
#
# @speed: #optional the maximum speed, in bytes per second
#
+# @bitmap: #optional the name of dirty bitmap if sync is "dirty-bitmap"
+# (Since 2.3)
+#
# @on-source-error: #optional the action to take on an error on the source,
# default 'report'. 'stop' and 'enospc' can only be used
# if the block device supports io-status (see BlockInfo).
@@ -700,7 +703,7 @@
{ 'type': 'DriveBackup',
'data': { 'device': 'str', 'target': 'str', '*format': 'str',
'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
- '*speed': 'int',
+ '*speed': 'int', '*bitmap': 'str',
'*on-source-error': 'BlockdevOnError',
'*on-target-error': 'BlockdevOnError' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 59be8eb..479d4f5 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1048,7 +1048,7 @@ EQMP
{
.name = "drive-backup",
.args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
- "on-source-error:s?,on-target-error:s?",
+ "bitmap:s?,on-source-error:s?,on-target-error:s?",
.mhandler.cmd_new = qmp_marshal_input_drive_backup,
},
@@ -1075,8 +1075,9 @@ Arguments:
(json-string, optional)
- "sync": what parts of the disk image should be copied to the destination;
possibilities include "full" for all the disk, "top" for only the sectors
- allocated in the topmost image, or "none" to only replicate new I/O
- (MirrorSyncMode).
+ allocated in the topmost image, "dirty-bitmap" for only the dirty sectors in
+ the bitmap, or "none" to only replicate new I/O (MirrorSyncMode).
+- "bitmap": dirty bitmap name for sync==dirty-bitmap
- "mode": whether and how QEMU should create a new image
(NewImageMode, optional, default 'absolute-paths')
- "speed": the maximum speed, in bytes per second (json-int, optional)
--
1.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v10 11/13] qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable}
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
` (9 preceding siblings ...)
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 10/13] qmp: Add support of "dirty-bitmap" sync mode for drive-backup John Snow
@ 2014-12-23 1:12 ` John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 12/13] qmp: Add dirty bitmap 'enabled' field in query-block John Snow
` (2 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: John Snow @ 2014-12-23 1:12 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, John Snow, armbru, mreitz, vsementsov, stefanha
From: Fam Zheng <famz@redhat.com>
This adds three qmp commands to transactions.
Users can stop a dirty bitmap, start backup of it, and start another
dirty bitmap atomically, so that the dirty bitmap is tracked
incrementally and we don't miss any write.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
blockdev.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
qapi-schema.json | 5 ++-
2 files changed, 104 insertions(+), 1 deletion(-)
diff --git a/blockdev.c b/blockdev.c
index 0b38571..209fedd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1602,6 +1602,89 @@ static void drive_backup_clean(BlkTransactionState *common)
}
}
+static void block_dirty_bitmap_add_prepare(BlkTransactionState *common,
+ Error **errp)
+{
+ BlockDirtyBitmapAdd *action;
+
+ action = common->action->block_dirty_bitmap_add;
+ qmp_block_dirty_bitmap_add(action->node_ref, action->name,
+ action->has_granularity, action->granularity,
+ errp);
+}
+
+static void block_dirty_bitmap_add_abort(BlkTransactionState *common)
+{
+ BlockDirtyBitmapAdd *action;
+
+ action = common->action->block_dirty_bitmap_add;
+ /* Should not fail meaningfully: IF the bitmap was added via .prepare(),
+ * then the node reference and bitmap name must have been valid.
+ * THUS: any failure here could only indicate the lack of a bitmap at all.
+ */
+ qmp_block_dirty_bitmap_remove(action->node_ref, action->name, NULL);
+}
+
+typedef struct BlockDirtyBitmapState {
+ BlkTransactionState common;
+ BdrvDirtyBitmap *bitmap;
+ AioContext *aio_context;
+} BlockDirtyBitmapState;
+
+/**
+ * Enable and Disable re-use the same preparation.
+ */
+static void block_dirty_bitmap_toggle_prepare(BlkTransactionState *common,
+ Error **errp)
+{
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+ BlockDirtyBitmap *action;
+ BlockDriverState *bs;
+
+ /* We may be used by either enable or disable;
+ * We use the "enable" member of the union here,
+ * but "disable" should be functionally equivalent: */
+ action = common->action->block_dirty_bitmap_enable;
+ assert(action == common->action->block_dirty_bitmap_disable);
+
+ state->bitmap = block_dirty_bitmap_lookup(action->node_ref,
+ action->name,
+ &bs,
+ errp);
+ if (!state->bitmap) {
+ return;
+ }
+
+ /* AioContext is released in .clean() */
+ state->aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(state->aio_context);
+}
+
+static void block_dirty_bitmap_enable_commit(BlkTransactionState *common)
+{
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+ bdrv_enable_dirty_bitmap(state->bitmap);
+}
+
+static void block_dirty_bitmap_disable_commit(BlkTransactionState *common)
+{
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+ bdrv_disable_dirty_bitmap(state->bitmap);
+}
+
+static void block_dirty_bitmap_toggle_clean(BlkTransactionState *common)
+{
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+
+ if (state->aio_context) {
+ aio_context_release(state->aio_context);
+ }
+}
+
static void abort_prepare(BlkTransactionState *common, Error **errp)
{
error_setg(errp, "Transaction aborted using Abort action");
@@ -1636,6 +1719,23 @@ static const BdrvActionOps actions[] = {
.abort = internal_snapshot_abort,
.clean = internal_snapshot_clean,
},
+ [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ADD] = {
+ .instance_size = sizeof(BlkTransactionState),
+ .prepare = block_dirty_bitmap_add_prepare,
+ .abort = block_dirty_bitmap_add_abort,
+ },
+ [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ENABLE] = {
+ .instance_size = sizeof(BlockDirtyBitmapState),
+ .prepare = block_dirty_bitmap_toggle_prepare,
+ .commit = block_dirty_bitmap_enable_commit,
+ .clean = block_dirty_bitmap_toggle_clean,
+ },
+ [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_DISABLE] = {
+ .instance_size = sizeof(BlockDirtyBitmapState),
+ .prepare = block_dirty_bitmap_toggle_prepare,
+ .commit = block_dirty_bitmap_disable_commit,
+ .clean = block_dirty_bitmap_toggle_clean,
+ },
};
/*
diff --git a/qapi-schema.json b/qapi-schema.json
index 563b4ad..85f55d9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1260,7 +1260,10 @@
'blockdev-snapshot-sync': 'BlockdevSnapshot',
'drive-backup': 'DriveBackup',
'abort': 'Abort',
- 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal'
+ 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
+ 'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
+ 'block-dirty-bitmap-enable': 'BlockDirtyBitmap',
+ 'block-dirty-bitmap-disable': 'BlockDirtyBitmap'
} }
##
--
1.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v10 12/13] qmp: Add dirty bitmap 'enabled' field in query-block
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
` (10 preceding siblings ...)
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 11/13] qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable} John Snow
@ 2014-12-23 1:12 ` John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 13/13] qemu-iotests: Add tests for drive-backup sync=dirty-bitmap John Snow
2014-12-23 11:48 ` [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) Vladimir Sementsov-Ogievskiy
13 siblings, 0 replies; 22+ messages in thread
From: John Snow @ 2014-12-23 1:12 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, John Snow, armbru, mreitz, vsementsov, stefanha
From: Fam Zheng <famz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 1 +
qapi/block-core.json | 5 ++++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 3d1337b..2466ba8 100644
--- a/block.c
+++ b/block.c
@@ -5444,6 +5444,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
info->granularity = bdrv_dirty_bitmap_granularity(bs, bm);
info->has_name = !!bm->name;
info->name = g_strdup(bm->name);
+ info->enabled = bm->enabled;
entry->value = info;
*plist = entry;
plist = &entry->next;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 92c3a84..1892b50 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -328,10 +328,13 @@
#
# @granularity: granularity of the dirty bitmap in bytes (since 1.4)
#
+# @enabled: whether the dirty bitmap is enabled (Since 2.3)
+#
# Since: 1.3
##
{ 'type': 'BlockDirtyInfo',
- 'data': {'*name': 'str', 'count': 'int', 'granularity': 'int'} }
+ 'data': {'*name': 'str', 'count': 'int', 'granularity': 'int',
+ 'enabled': 'bool'} }
##
# @BlockInfo:
--
1.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v10 13/13] qemu-iotests: Add tests for drive-backup sync=dirty-bitmap
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
` (11 preceding siblings ...)
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 12/13] qmp: Add dirty bitmap 'enabled' field in query-block John Snow
@ 2014-12-23 1:12 ` John Snow
2014-12-23 11:48 ` [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) Vladimir Sementsov-Ogievskiy
13 siblings, 0 replies; 22+ messages in thread
From: John Snow @ 2014-12-23 1:12 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, John Snow, armbru, mreitz, vsementsov, stefanha
From: Fam Zheng <famz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/056 | 33 ++++++++++++++++++++++++++++++---
tests/qemu-iotests/056.out | 4 ++--
tests/qemu-iotests/iotests.py | 8 ++++++++
3 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/tests/qemu-iotests/056 b/tests/qemu-iotests/056
index 54e4bd0..6f8aa9a 100755
--- a/tests/qemu-iotests/056
+++ b/tests/qemu-iotests/056
@@ -23,17 +23,17 @@
import time
import os
import iotests
-from iotests import qemu_img, qemu_io, create_image
+from iotests import qemu_img, qemu_img_map_assert, qemu_io, create_image
backing_img = os.path.join(iotests.test_dir, 'backing.img')
test_img = os.path.join(iotests.test_dir, 'test.img')
target_img = os.path.join(iotests.test_dir, 'target.img')
-class TestSyncModesNoneAndTop(iotests.QMPTestCase):
+class TestSyncModes(iotests.QMPTestCase):
image_len = 64 * 1024 * 1024 # MB
def setUp(self):
- create_image(backing_img, TestSyncModesNoneAndTop.image_len)
+ create_image(backing_img, TestSyncModes.image_len)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
qemu_io('-c', 'write -P0x41 0 512', test_img)
qemu_io('-c', 'write -P0xd5 1M 32k', test_img)
@@ -64,6 +64,33 @@ class TestSyncModesNoneAndTop(iotests.QMPTestCase):
self.assertTrue(iotests.compare_images(test_img, target_img),
'target image does not match source after backup')
+ def test_sync_dirty_bitmap_missing(self):
+ self.assert_no_active_block_jobs()
+ result = self.vm.qmp('drive-backup', device='drive0', sync='dirty-bitmap',
+ format=iotests.imgfmt, target=target_img)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+ def test_sync_dirty_bitmap_not_found(self):
+ self.assert_no_active_block_jobs()
+ result = self.vm.qmp('drive-backup', device='drive0', sync='dirty-bitmap',
+ bitmap='unknown',
+ format=iotests.imgfmt, target=target_img)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+ def test_sync_dirty_bitmap(self):
+ self.assert_no_active_block_jobs()
+ result = self.vm.qmp('block-dirty-bitmap-add', node_ref='drive0', name='bitmap0')
+ self.assert_qmp(result, 'return', {})
+ self.vm.hmp_qemu_io('drive0', 'write -P0x5a 0 512')
+ self.vm.hmp_qemu_io('drive0', 'write -P0x5a 48M 512')
+ result = self.vm.qmp('drive-backup', device='drive0', sync='dirty-bitmap',
+ bitmap='bitmap0',
+ format=iotests.imgfmt, target=target_img)
+ self.assert_qmp(result, 'return', {})
+ self.wait_until_completed(check_offset=False)
+ self.assert_no_active_block_jobs()
+ qemu_img_map_assert(target_img, [0, 0x3000000])
+
def test_cancel_sync_none(self):
self.assert_no_active_block_jobs()
diff --git a/tests/qemu-iotests/056.out b/tests/qemu-iotests/056.out
index fbc63e6..914e373 100644
--- a/tests/qemu-iotests/056.out
+++ b/tests/qemu-iotests/056.out
@@ -1,5 +1,5 @@
-..
+.....
----------------------------------------------------------------------
-Ran 2 tests
+Ran 5 tests
OK
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index f57f154..95bb959 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -55,6 +55,14 @@ def qemu_img_pipe(*args):
'''Run qemu-img and return its output'''
return subprocess.Popen(qemu_img_args + list(args), stdout=subprocess.PIPE).communicate()[0]
+def qemu_img_map_assert(img, offsets):
+ '''Run qemu-img map on img and check the mapped ranges'''
+ offs = []
+ for line in qemu_img_pipe('map', img).splitlines()[1:]:
+ offset, length, mapped, fname = line.split()
+ offs.append(int(offset, 16))
+ assert set(offs) == set(offsets), "mapped offsets in image '%s' not equal to '%s'" % (str(offs), str(offsets))
+
def qemu_io(*args):
'''Run qemu-io and return the stdout data'''
args = qemu_io_args + list(args)
--
1.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC)
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
` (12 preceding siblings ...)
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 13/13] qemu-iotests: Add tests for drive-backup sync=dirty-bitmap John Snow
@ 2014-12-23 11:48 ` Vladimir Sementsov-Ogievskiy
2014-12-23 16:23 ` John Snow
13 siblings, 1 reply; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2014-12-23 11:48 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: kwolf, famz, armbru, mreitz, stefanha
On 23.12.2014 04:12, John Snow wrote:
> For convenience, this patchset is available on github:
> https://github.com/jnsnow/qemu/commits/dbm-backup
- old version here.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC)
2014-12-23 11:48 ` [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) Vladimir Sementsov-Ogievskiy
@ 2014-12-23 16:23 ` John Snow
0 siblings, 0 replies; 22+ messages in thread
From: John Snow @ 2014-12-23 16:23 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: kwolf, famz, armbru, mreitz, stefanha
On 12/23/2014 06:48 AM, Vladimir Sementsov-Ogievskiy wrote:
> On 23.12.2014 04:12, John Snow wrote:
>> For convenience, this patchset is available on github:
>> https://github.com/jnsnow/qemu/commits/dbm-backup
> - old version here.
My apologies. I've corrected it now.
Thank you,
--John
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v10 06/13] block: Add bdrv_copy_dirty_bitmap and bdrv_clear_dirty_bitmap
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 06/13] block: Add bdrv_copy_dirty_bitmap and bdrv_clear_dirty_bitmap John Snow
@ 2014-12-30 13:47 ` Vladimir Sementsov-Ogievskiy
2015-01-10 3:25 ` John Snow
0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2014-12-30 13:47 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: kwolf, famz, armbru, mreitz, stefanha
I'm sorry if it was already discussed, but I think it is inconsistent to
have "size" in sectors and "granularity" in bytes in one structure. I've
misused these fields because of this in my current work.
At least, I think there should be comments about this.
Best regards,
Vladimir
On 23.12.2014 04:12, John Snow wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block.c | 39 +++++++++++++++++++++++++++++++++++----
> include/block/block.h | 4 ++++
> 2 files changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/block.c b/block.c
> index a1d9e88..f9e0767 100644
> --- a/block.c
> +++ b/block.c
> @@ -53,6 +53,8 @@
>
> struct BdrvDirtyBitmap {
> HBitmap *bitmap;
> + int64_t size;
> + int64_t granularity;
> char *name;
> QLIST_ENTRY(BdrvDirtyBitmap) list;
> };
> @@ -5343,6 +5345,21 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
> bitmap->name = NULL;
> }
>
> +BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
> + BdrvDirtyBitmap *bitmap,
> + const char *name)
> +{
> + BdrvDirtyBitmap *new_bitmap;
> +
> + new_bitmap = g_malloc0(sizeof(BdrvDirtyBitmap));
> + new_bitmap->bitmap = hbitmap_copy(bitmap->bitmap);
> + new_bitmap->size = bitmap->size;
> + new_bitmap->granularity = bitmap->granularity;
> + new_bitmap->name = g_strdup(name);
> + QLIST_INSERT_HEAD(&bs->dirty_bitmaps, new_bitmap, list);
> + return new_bitmap;
> +}
> +
> BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
> int granularity,
> const char *name,
> @@ -5350,6 +5367,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
> {
> int64_t bitmap_size;
> BdrvDirtyBitmap *bitmap;
> + int sector_granularity;
>
> assert((granularity & (granularity - 1)) == 0);
>
> @@ -5357,8 +5375,8 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
> error_setg(errp, "Bitmap already exists: %s", name);
> return NULL;
> }
> - granularity >>= BDRV_SECTOR_BITS;
> - assert(granularity);
> + sector_granularity = granularity >> BDRV_SECTOR_BITS;
> + assert(sector_granularity);
> bitmap_size = bdrv_nb_sectors(bs);
> if (bitmap_size < 0) {
> error_setg_errno(errp, -bitmap_size, "could not get length of device");
> @@ -5366,7 +5384,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
> return NULL;
> }
> bitmap = g_new0(BdrvDirtyBitmap, 1);
> - bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
> + bitmap->size = bitmap_size;
> + bitmap->granularity = granularity;
> + bitmap->bitmap = hbitmap_alloc(bitmap->size, ffs(sector_granularity) - 1);
> bitmap->name = g_strdup(name);
> QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
> return bitmap;
> @@ -5439,7 +5459,9 @@ uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
> uint64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
> BdrvDirtyBitmap *bitmap)
> {
> - return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
> + g_assert(BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap) ==
> + bitmap->granularity);
> + return bitmap->granularity;
> }
>
> void bdrv_dirty_iter_init(BlockDriverState *bs,
> @@ -5460,6 +5482,15 @@ void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
> hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
> }
>
> +/**
> + * Effectively, reset the hbitmap from bits [0, size)
> + * Synonymous with bdrv_reset_dirty_bitmap(bs, bitmap, 0, bitmap->size)
> + */
> +void bdrv_clear_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
> +{
> + hbitmap_reset(bitmap->bitmap, 0, bitmap->size);
> +}
> +
> static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
> int nr_sectors)
> {
> diff --git a/include/block/block.h b/include/block/block.h
> index c7402e7..e964abd 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -436,6 +436,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
> BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
> const char *name);
> void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
> +BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
> + BdrvDirtyBitmap *bitmap,
> + const char *name);
> void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
> BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
> uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
> @@ -446,6 +449,7 @@ void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
> int64_t cur_sector, int nr_sectors);
> void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
> int64_t cur_sector, int nr_sectors);
> +void bdrv_clear_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
> void bdrv_dirty_iter_init(BlockDriverState *bs,
> BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
> int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v10 06/13] block: Add bdrv_copy_dirty_bitmap and bdrv_clear_dirty_bitmap
2014-12-30 13:47 ` Vladimir Sementsov-Ogievskiy
@ 2015-01-10 3:25 ` John Snow
2015-01-13 9:54 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 22+ messages in thread
From: John Snow @ 2015-01-10 3:25 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: kwolf, famz, armbru, stefanha, mreitz
On 12/30/2014 08:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> I'm sorry if it was already discussed, but I think it is inconsistent to
> have "size" in sectors and "granularity" in bytes in one structure. I've
> misused these fields because of this in my current work.
>
> At least, I think there should be comments about this.
>
> Best regards,
> Vladimir
Looking at it now, I think it'd be worse to change it, because it
represents the "size" of the bitmap, as in the number of logical bits
for the interface of the bitmap. Since it is a sector bitmap, I think
this should remain in sectors, for now.
I really want to keep the granularity in bytes in this case, because I
want to match the existing interface, and size makes sense to me in sectors.
What I will do instead is just to document this quirk. Look out for V11 :)
--John
>
> On 23.12.2014 04:12, John Snow wrote:
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>> block.c | 39 +++++++++++++++++++++++++++++++++++----
>> include/block/block.h | 4 ++++
>> 2 files changed, 39 insertions(+), 4 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index a1d9e88..f9e0767 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -53,6 +53,8 @@
>> struct BdrvDirtyBitmap {
>> HBitmap *bitmap;
>> + int64_t size;
>> + int64_t granularity;
>> char *name;
>> QLIST_ENTRY(BdrvDirtyBitmap) list;
>> };
>> @@ -5343,6 +5345,21 @@ void
>> bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap
>> *bitmap)
>> bitmap->name = NULL;
>> }
>> +BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
>> + BdrvDirtyBitmap *bitmap,
>> + const char *name)
>> +{
>> + BdrvDirtyBitmap *new_bitmap;
>> +
>> + new_bitmap = g_malloc0(sizeof(BdrvDirtyBitmap));
>> + new_bitmap->bitmap = hbitmap_copy(bitmap->bitmap);
>> + new_bitmap->size = bitmap->size;
>> + new_bitmap->granularity = bitmap->granularity;
>> + new_bitmap->name = g_strdup(name);
>> + QLIST_INSERT_HEAD(&bs->dirty_bitmaps, new_bitmap, list);
>> + return new_bitmap;
>> +}
>> +
>> BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>> int granularity,
>> const char *name,
>> @@ -5350,6 +5367,7 @@ BdrvDirtyBitmap
>> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>> {
>> int64_t bitmap_size;
>> BdrvDirtyBitmap *bitmap;
>> + int sector_granularity;
>> assert((granularity & (granularity - 1)) == 0);
>> @@ -5357,8 +5375,8 @@ BdrvDirtyBitmap
>> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>> error_setg(errp, "Bitmap already exists: %s", name);
>> return NULL;
>> }
>> - granularity >>= BDRV_SECTOR_BITS;
>> - assert(granularity);
>> + sector_granularity = granularity >> BDRV_SECTOR_BITS;
>> + assert(sector_granularity);
>> bitmap_size = bdrv_nb_sectors(bs);
>> if (bitmap_size < 0) {
>> error_setg_errno(errp, -bitmap_size, "could not get length
>> of device");
>> @@ -5366,7 +5384,9 @@ BdrvDirtyBitmap
>> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>> return NULL;
>> }
>> bitmap = g_new0(BdrvDirtyBitmap, 1);
>> - bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
>> + bitmap->size = bitmap_size;
>> + bitmap->granularity = granularity;
>> + bitmap->bitmap = hbitmap_alloc(bitmap->size,
>> ffs(sector_granularity) - 1);
>> bitmap->name = g_strdup(name);
>> QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
>> return bitmap;
>> @@ -5439,7 +5459,9 @@ uint64_t
>> bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
>> uint64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
>> BdrvDirtyBitmap *bitmap)
>> {
>> - return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
>> + g_assert(BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap) ==
>> + bitmap->granularity);
>> + return bitmap->granularity;
>> }
>> void bdrv_dirty_iter_init(BlockDriverState *bs,
>> @@ -5460,6 +5482,15 @@ void bdrv_reset_dirty_bitmap(BlockDriverState
>> *bs, BdrvDirtyBitmap *bitmap,
>> hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
>> }
>> +/**
>> + * Effectively, reset the hbitmap from bits [0, size)
>> + * Synonymous with bdrv_reset_dirty_bitmap(bs, bitmap, 0, bitmap->size)
>> + */
>> +void bdrv_clear_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap
>> *bitmap)
>> +{
>> + hbitmap_reset(bitmap->bitmap, 0, bitmap->size);
>> +}
>> +
>> static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
>> int nr_sectors)
>> {
>> diff --git a/include/block/block.h b/include/block/block.h
>> index c7402e7..e964abd 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -436,6 +436,9 @@ BdrvDirtyBitmap
>> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>> BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
>> const char *name);
>> void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs,
>> BdrvDirtyBitmap *bitmap);
>> +BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
>> + BdrvDirtyBitmap *bitmap,
>> + const char *name);
>> void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap
>> *bitmap);
>> BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
>> uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
>> @@ -446,6 +449,7 @@ void bdrv_set_dirty_bitmap(BlockDriverState *bs,
>> BdrvDirtyBitmap *bitmap,
>> int64_t cur_sector, int nr_sectors);
>> void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap
>> *bitmap,
>> int64_t cur_sector, int nr_sectors);
>> +void bdrv_clear_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap
>> *bitmap);
>> void bdrv_dirty_iter_init(BlockDriverState *bs,
>> BdrvDirtyBitmap *bitmap, struct
>> HBitmapIter *hbi);
>> int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap
>> *bitmap);
>
>
--
—js
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v10 06/13] block: Add bdrv_copy_dirty_bitmap and bdrv_clear_dirty_bitmap
2015-01-10 3:25 ` John Snow
@ 2015-01-13 9:54 ` Vladimir Sementsov-Ogievskiy
2015-01-13 16:37 ` John Snow
0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-01-13 9:54 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: kwolf, famz, armbru, stefanha, mreitz
Hmm, can't find it in v11
Best regards,
Vladimir
On 10.01.2015 06:25, John Snow wrote:
>
>
> On 12/30/2014 08:47 AM, Vladimir Sementsov-Ogievskiy wrote:
>> I'm sorry if it was already discussed, but I think it is inconsistent to
>> have "size" in sectors and "granularity" in bytes in one structure. I've
>> misused these fields because of this in my current work.
>>
>> At least, I think there should be comments about this.
>>
>> Best regards,
>> Vladimir
>
> Looking at it now, I think it'd be worse to change it, because it
> represents the "size" of the bitmap, as in the number of logical bits
> for the interface of the bitmap. Since it is a sector bitmap, I think
> this should remain in sectors, for now.
>
> I really want to keep the granularity in bytes in this case, because I
> want to match the existing interface, and size makes sense to me in
> sectors.
>
> What I will do instead is just to document this quirk. Look out for
> V11 :)
>
> --John
>
>>
>> On 23.12.2014 04:12, John Snow wrote:
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> ---
>>> block.c | 39 +++++++++++++++++++++++++++++++++++----
>>> include/block/block.h | 4 ++++
>>> 2 files changed, 39 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/block.c b/block.c
>>> index a1d9e88..f9e0767 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -53,6 +53,8 @@
>>> struct BdrvDirtyBitmap {
>>> HBitmap *bitmap;
>>> + int64_t size;
>>> + int64_t granularity;
>>> char *name;
>>> QLIST_ENTRY(BdrvDirtyBitmap) list;
>>> };
>>> @@ -5343,6 +5345,21 @@ void
>>> bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap
>>> *bitmap)
>>> bitmap->name = NULL;
>>> }
>>> +BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
>>> + BdrvDirtyBitmap *bitmap,
>>> + const char *name)
>>> +{
>>> + BdrvDirtyBitmap *new_bitmap;
>>> +
>>> + new_bitmap = g_malloc0(sizeof(BdrvDirtyBitmap));
>>> + new_bitmap->bitmap = hbitmap_copy(bitmap->bitmap);
>>> + new_bitmap->size = bitmap->size;
>>> + new_bitmap->granularity = bitmap->granularity;
>>> + new_bitmap->name = g_strdup(name);
>>> + QLIST_INSERT_HEAD(&bs->dirty_bitmaps, new_bitmap, list);
>>> + return new_bitmap;
>>> +}
>>> +
>>> BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>>> int granularity,
>>> const char *name,
>>> @@ -5350,6 +5367,7 @@ BdrvDirtyBitmap
>>> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>>> {
>>> int64_t bitmap_size;
>>> BdrvDirtyBitmap *bitmap;
>>> + int sector_granularity;
>>> assert((granularity & (granularity - 1)) == 0);
>>> @@ -5357,8 +5375,8 @@ BdrvDirtyBitmap
>>> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>>> error_setg(errp, "Bitmap already exists: %s", name);
>>> return NULL;
>>> }
>>> - granularity >>= BDRV_SECTOR_BITS;
>>> - assert(granularity);
>>> + sector_granularity = granularity >> BDRV_SECTOR_BITS;
>>> + assert(sector_granularity);
>>> bitmap_size = bdrv_nb_sectors(bs);
>>> if (bitmap_size < 0) {
>>> error_setg_errno(errp, -bitmap_size, "could not get length
>>> of device");
>>> @@ -5366,7 +5384,9 @@ BdrvDirtyBitmap
>>> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>>> return NULL;
>>> }
>>> bitmap = g_new0(BdrvDirtyBitmap, 1);
>>> - bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
>>> + bitmap->size = bitmap_size;
>>> + bitmap->granularity = granularity;
>>> + bitmap->bitmap = hbitmap_alloc(bitmap->size,
>>> ffs(sector_granularity) - 1);
>>> bitmap->name = g_strdup(name);
>>> QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
>>> return bitmap;
>>> @@ -5439,7 +5459,9 @@ uint64_t
>>> bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
>>> uint64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
>>> BdrvDirtyBitmap *bitmap)
>>> {
>>> - return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
>>> + g_assert(BDRV_SECTOR_SIZE <<
>>> hbitmap_granularity(bitmap->bitmap) ==
>>> + bitmap->granularity);
>>> + return bitmap->granularity;
>>> }
>>> void bdrv_dirty_iter_init(BlockDriverState *bs,
>>> @@ -5460,6 +5482,15 @@ void bdrv_reset_dirty_bitmap(BlockDriverState
>>> *bs, BdrvDirtyBitmap *bitmap,
>>> hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
>>> }
>>> +/**
>>> + * Effectively, reset the hbitmap from bits [0, size)
>>> + * Synonymous with bdrv_reset_dirty_bitmap(bs, bitmap, 0,
>>> bitmap->size)
>>> + */
>>> +void bdrv_clear_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap
>>> *bitmap)
>>> +{
>>> + hbitmap_reset(bitmap->bitmap, 0, bitmap->size);
>>> +}
>>> +
>>> static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
>>> int nr_sectors)
>>> {
>>> diff --git a/include/block/block.h b/include/block/block.h
>>> index c7402e7..e964abd 100644
>>> --- a/include/block/block.h
>>> +++ b/include/block/block.h
>>> @@ -436,6 +436,9 @@ BdrvDirtyBitmap
>>> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>>> BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
>>> const char *name);
>>> void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs,
>>> BdrvDirtyBitmap *bitmap);
>>> +BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
>>> + BdrvDirtyBitmap *bitmap,
>>> + const char *name);
>>> void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap
>>> *bitmap);
>>> BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
>>> uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
>>> @@ -446,6 +449,7 @@ void bdrv_set_dirty_bitmap(BlockDriverState *bs,
>>> BdrvDirtyBitmap *bitmap,
>>> int64_t cur_sector, int nr_sectors);
>>> void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap
>>> *bitmap,
>>> int64_t cur_sector, int nr_sectors);
>>> +void bdrv_clear_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap
>>> *bitmap);
>>> void bdrv_dirty_iter_init(BlockDriverState *bs,
>>> BdrvDirtyBitmap *bitmap, struct
>>> HBitmapIter *hbi);
>>> int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap
>>> *bitmap);
>>
>>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v10 06/13] block: Add bdrv_copy_dirty_bitmap and bdrv_clear_dirty_bitmap
2015-01-13 9:54 ` Vladimir Sementsov-Ogievskiy
@ 2015-01-13 16:37 ` John Snow
2015-01-13 16:43 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 22+ messages in thread
From: John Snow @ 2015-01-13 16:37 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: kwolf, famz, armbru, stefanha, mreitz
On 01/13/2015 04:54 AM, Vladimir Sementsov-Ogievskiy wrote:
> Hmm, can't find it in v11
>
> Best regards,
> Vladimir
>
I changed approach and instead of copying bitmaps to be used with the
incremental backup, I lock them in place. So copy isn't used anymore in
my set, so the name of the patch changed.
--js
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v10 06/13] block: Add bdrv_copy_dirty_bitmap and bdrv_clear_dirty_bitmap
2015-01-13 16:37 ` John Snow
@ 2015-01-13 16:43 ` Vladimir Sementsov-Ogievskiy
2015-01-13 16:45 ` John Snow
0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2015-01-13 16:43 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: kwolf, famz, armbru, stefanha, mreitz
No, I'm saying about adding comment about using size in sectors and
granularity in bytes in BdrvDirtyBitmap structure.
On 10.01.2015 06:25, John Snow wrote:
......
> What I will do instead is just to document this quirk. Look out for V11
Best regards,
Vladimir
On 13.01.2015 19:37, John Snow wrote:
>
> On 01/13/2015 04:54 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Hmm, can't find it in v11
>>
>> Best regards,
>> Vladimir
>>
>
> I changed approach and instead of copying bitmaps to be used with the
> incremental backup, I lock them in place. So copy isn't used anymore
> in my set, so the name of the patch changed.
>
> --js
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v10 06/13] block: Add bdrv_copy_dirty_bitmap and bdrv_clear_dirty_bitmap
2015-01-13 16:43 ` Vladimir Sementsov-Ogievskiy
@ 2015-01-13 16:45 ` John Snow
0 siblings, 0 replies; 22+ messages in thread
From: John Snow @ 2015-01-13 16:45 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: kwolf, famz, armbru, stefanha, mreitz
On 01/13/2015 11:43 AM, Vladimir Sementsov-Ogievskiy wrote:
> No, I'm saying about adding comment about using size in sectors and
> granularity in bytes in BdrvDirtyBitmap structure.
I'm sorry, I misunderstood.
It's at the end of the series, as a fixup patch.
>
> On 10.01.2015 06:25, John Snow wrote:
> ......
>> What I will do instead is just to document this quirk. Look out for V11
>
> Best regards,
> Vladimir
>
> On 13.01.2015 19:37, John Snow wrote:
>>
>> On 01/13/2015 04:54 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> Hmm, can't find it in v11
>>>
>>> Best regards,
>>> Vladimir
>>>
>>
>> I changed approach and instead of copying bitmaps to be used with the
>> incremental backup, I lock them in place. So copy isn't used anymore
>> in my set, so the name of the patch changed.
>>
>> --js
>
--
—js
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2015-01-13 16:45 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-23 1:12 [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 01/13] block: fix spoiling all dirty bitmaps by mirror and migration John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 02/13] qapi: Add optional field "name" to block dirty bitmap John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 03/13] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 04/13] block: Introduce bdrv_dirty_bitmap_granularity() John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 05/13] hbitmap: Add hbitmap_copy John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 06/13] block: Add bdrv_copy_dirty_bitmap and bdrv_clear_dirty_bitmap John Snow
2014-12-30 13:47 ` Vladimir Sementsov-Ogievskiy
2015-01-10 3:25 ` John Snow
2015-01-13 9:54 ` Vladimir Sementsov-Ogievskiy
2015-01-13 16:37 ` John Snow
2015-01-13 16:43 ` Vladimir Sementsov-Ogievskiy
2015-01-13 16:45 ` John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 07/13] hbitmap: add hbitmap_merge John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 08/13] block: add bdrv_reclaim_dirty_bitmap John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 09/13] qmp: Add block-dirty-bitmap-enable and block-dirty-bitmap-disable John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 10/13] qmp: Add support of "dirty-bitmap" sync mode for drive-backup John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 11/13] qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable} John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 12/13] qmp: Add dirty bitmap 'enabled' field in query-block John Snow
2014-12-23 1:12 ` [Qemu-devel] [PATCH v10 13/13] qemu-iotests: Add tests for drive-backup sync=dirty-bitmap John Snow
2014-12-23 11:48 ` [Qemu-devel] [PATCH v10 00/13] block: Incremental backup series (RFC) Vladimir Sementsov-Ogievskiy
2014-12-23 16:23 ` John Snow
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).