On 2014-11-25 at 20:46, John Snow wrote:
From: Fam Zheng <f...@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 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}'
Thanks, this helps. :-)
Signed-off-by: Fam Zheng <f...@redhat.com>
Signed-off-by: John Snow <js...@redhat.com>
---
block.c | 19 ++++++++++++++++
block/mirror.c | 10 +-------
blockdev.c | 63
+++++++++++++++++++++++++++++++++++++++++++++++++++
include/block/block.h | 1 +
qapi/block-core.json | 58
+++++++++++++++++++++++++++++++++++++++++++++++
qmp-commands.hx | 49 +++++++++++++++++++++++++++++++++++++++
6 files changed, 191 insertions(+), 9 deletions(-)
diff --git a/block.c b/block.c
index f94b753..a940345 100644
--- a/block.c
+++ b/block.c
@@ -5385,6 +5385,25 @@ int bdrv_get_dirty(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, int64_t sector
}
}
+#define BDB_MIN_DEF_GRANULARITY 4096
+#define BDB_MAX_DEF_GRANULARITY 65536
+#define BDB_DEFAULT_GRANULARITY BDB_MAX_DEF_GRANULARITY
You mean this is the default for the default? ;-)
+
+int64_t bdrv_dbm_calc_def_granularity(BlockDriverState *bs)
You may want to make this a uint64_t so it's clear that this function
does not return errors.
+{
+ BlockDriverInfo bdi;
+ int64_t granularity;
+
+ if (bdrv_get_info(bs, &bdi) >= 0 && bdi.cluster_size != 0) {
+ granularity = MAX(BDB_MIN_DEF_GRANULARITY, bdi.cluster_size);
+ granularity = MIN(BDB_MAX_DEF_GRANULARITY, granularity);
+ } else {
+ granularity = BDB_DEFAULT_GRANULARITY;
+ }
+
+ return granularity;
+}
+
void bdrv_dirty_iter_init(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
{
diff --git a/block/mirror.c b/block/mirror.c
index 858e4ff..3633632 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -664,15 +664,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_dbm_calc_def_granularity(target);
Maybe you should note this replacement in the commit message.
}
assert ((granularity & (granularity - 1)) == 0);
diff --git a/blockdev.c b/blockdev.c
index 57910b8..e2fe687 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1810,6 +1810,69 @@ 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 *device, const char *name,
+ bool has_granularity, int64_t
granularity,
+ Error **errp)
+{
+ BlockDriverState *bs;
+ Error *local_err = NULL;
+
+ if (!device) {
+ error_setg(errp, "Device to add dirty bitmap to must not be
null");
+ return;
+ }
I don't know if checking for that case makes sense, but of course it
won't hurt. But...[1]
+
+ bs = bdrv_lookup_bs(device, NULL, &local_err);
Fair enough, I'd still like blk_by_name() and blk_bs() more
(bdrv_lookup_bs() uses blk_bs(blk_by_name()) for the device name just as
bdrv_find() did), but this is at least not a completely trivial wrapper.
+ if (!bs) {
+ error_propagate(errp, local_err);
Simply calling bdrv_lookup_bs(device, NULL, errp); suffices, no need for
a local Error object and error_propagate(). But I'm fine with it either
way.
