On 2015-02-20 at 18:07, John Snow wrote:
This adds four 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.
For starting a new incremental backup chain, users can also chain
together a bitmap clear and a full block backup.
Signed-off-by: Fam Zheng <f...@redhat.com>
Signed-off-by: John Snow <js...@redhat.com>
---
blockdev.c | 170 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
qapi-schema.json | 10 +++-
2 files changed, 179 insertions(+), 1 deletion(-)
diff --git a/blockdev.c b/blockdev.c
index 8422e94..7fbcb98 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1675,6 +1675,153 @@ static void blockdev_backup_clean(BlkTransactionState
*common)
}
}
+typedef struct BlockDirtyBitmapState {
+ BlkTransactionState common;
+ BdrvDirtyBitmap *bitmap;
+ BlockDriverState *bs;
+ AioContext *aio_context;
+ bool prepared;
+} BlockDirtyBitmapState;
+
+static void block_dirty_bitmap_add_prepare(BlkTransactionState *common,
+ Error **errp)
+{
+ BlockDirtyBitmapAdd *action;
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+
+ action = common->action->block_dirty_bitmap_add;
+ /* AIO context taken within qmp_block_dirty_bitmap_add */
+ qmp_block_dirty_bitmap_add(action->node, action->name,
+ action->has_granularity, action->granularity,
+ errp);
+
+ if (*errp == NULL) {
You don't know whether errp is NULL or not; please use a local Error *
object (and error_propagate()).
+ state->prepared = true;
+ }
+}
+
+static void block_dirty_bitmap_add_abort(BlkTransactionState *common)
+{
+ BlockDirtyBitmapAdd *action;
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+
+ 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.
+ */
+ if (state->prepared) {
+ qmp_block_dirty_bitmap_remove(action->node, action->name, NULL);
Considering that I agree that this cannot fail, I propose you use
&error_abort as the errp parameter. I'm leaving the decision up to you.
+ }
+}
+
+/**
+ * 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,
+ action->name,
+ &bs,
+ errp);
+ if (!state->bitmap) {
+ return;
+ }
+
+ if (bdrv_dirty_bitmap_frozen(state->bitmap)) {
+ error_setg(errp, "Cannot modify a frozen 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 block_dirty_bitmap_clear_prepare(BlkTransactionState *common,
+ Error **errp)
+{
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+ BlockDirtyBitmap *action;
+
+ action = common->action->block_dirty_bitmap_clear;
+ state->bitmap = block_dirty_bitmap_lookup(action->node,
+ action->name,
+ &state->bs,
+ errp);
+ if (!state->bitmap) {
+ return;
+ }
+
+ if (bdrv_dirty_bitmap_frozen(state->bitmap)) {
+ error_setg(errp, "Cannot modify a frozen bitmap");
+ return;
+ } else if (!bdrv_dirty_bitmap_enabled(state->bitmap)) {
+ error_setg(errp, "Cannot clear a disabled bitmap");
+ return;
+ }
+
+ /* AioContext is released in .clean() */
+ state->aio_context = bdrv_get_aio_context(state->bs);
+ aio_context_acquire(state->aio_context);
+}
+
+static void block_dirty_bitmap_clear_commit(BlkTransactionState *common)
+{
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+ bdrv_clear_dirty_bitmap(state->bitmap);
+}
+
+static void block_dirty_bitmap_clear_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");
@@ -1715,6 +1862,29 @@ static const BdrvActionOps actions[] = {
.abort = internal_snapshot_abort,
.clean = internal_snapshot_clean,
},
+ [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ADD] = {
+ .instance_size = sizeof(BlkTransactionState),
Probably because you're writing to a different offset in the structure
now than before, this doesn't segfault any more for me (but valgrind
does reveal an "invalid write"; and inserting a "state->bitmap = (void
*)42;" does make it segfault again); but this should still be
sizeof(BlockDirtyBitmapState).
Max
+ .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,
+ },
+ [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_CLEAR] = {
+ .instance_size = sizeof(BlockDirtyBitmapState),
+ .prepare = block_dirty_bitmap_clear_prepare,
+ .commit = block_dirty_bitmap_clear_commit,
+ .clean = block_dirty_bitmap_clear_clean,
+ }
};
/*
diff --git a/qapi-schema.json b/qapi-schema.json
index e16f8eb..0c31203 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1332,6 +1332,10 @@
# abort since 1.6
# blockdev-snapshot-internal-sync since 1.7
# blockdev-backup since 2.3
+# block-dirty-bitmap-add since 2.3
+# block-dirty-bitmap-enable since 2.3
+# block-dirty-bitmap-disable since 2.3
+# block-dirty-bitmap-clear since 2.3
##
{ 'union': 'TransactionAction',
'data': {
@@ -1339,7 +1343,11 @@
'drive-backup': 'DriveBackup',
'blockdev-backup': 'BlockdevBackup',
'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',
+ 'block-dirty-bitmap-clear': 'BlockDirtyBitmap'
} }
##