+}
+
+static void block_dirty_bitmap_add_abort(BlkTransactionState *common)
+{
+ BlockDirtyBitmapAdd *action;
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
Indentation is one space off.
+ 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.
+ * We will use the presence of the bitmap field to check that we
succeeded
+ * in the first place, though.
+ */
+ if (state->bitmap) {
+ qmp_block_dirty_bitmap_remove(action->node, action->name, NULL);
[1] As said above, state->bitmap will be non-NULL if the bitmap existed
before. Thus, this may remove a pre-existing bitmap which has not been
added by this transaction. Case in point:
$ x86_64-softmmu/qemu-system-x86_64 -drive
if=none,id=drv,file=test.qcow2 -qmp stdio -machine accel=qtest -display
none -nodefaults
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 2},
"package": ""}, "capabilities": []}}
{'execute':'qmp_capabilities'}
{"return": {}}
{'execute':'query-block'}
{"return": [{"device": "drv", "locked": false, "removable": true,
"inserted": {"iops_rd": 0, "detect_zeroes": "off", "image":
{"virtual-size": 67108864, "filename": "test.qcow2", "cluster-size":
65536, "format": "qcow2", "actual-size": 200704, "format-specific":
{"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false,
"corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false,
"backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0,
"write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0,
"cache": {"no-flush": false, "direct": false, "writeback": true},
"file": "test.qcow2", "encryption_key_missing": false}, "tray_open":
false, "type": "unknown"}]}
{'execute':'transaction','arguments':{'actions':[{'type':'block-dirty-bitmap-add','data':{'node':'drv','name':'foo'}}]}}
{"return": {}}
{'execute':'query-block'}
{"return": [{"device": "drv", "locked": false, "removable": true,
"inserted": {"iops_rd": 0, "detect_zeroes": "off", "image":
{"virtual-size": 67108864, "filename": "test.qcow2", "cluster-size":
65536, "format": "qcow2", "actual-size": 200704, "format-specific":
{"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false,
"corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false,
"backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0,
"write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0,
"cache": {"no-flush": false, "direct": false, "writeback": true},
"file": "test.qcow2", "encryption_key_missing": false}, "tray_open":
false, "dirty-bitmaps": [{"name": "foo", "granularity": 65536, "count":
0}], "type": "unknown"}]}
{'execute':'transaction','arguments':{'actions':[{'type':'block-dirty-bitmap-add','data':{'node':'drv','name':'foo'}}]}}
{"error": {"class": "GenericError", "desc": "Bitmap already exists: foo"}}
{'execute':'query-block'}
{"return": [{"device": "drv", "locked": false, "removable": true,
"inserted": {"iops_rd": 0, "detect_zeroes": "off", "image":
{"virtual-size": 67108864, "filename": "test.qcow2", "cluster-size":
65536, "format": "qcow2", "actual-size": 200704, "format-specific":
{"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false,
"corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false,
"backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0,
"write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0,
"cache": {"no-flush": false, "direct": false, "writeback": true},
"file": "test.qcow2", "encryption_key_missing": false}, "tray_open":
false, "type": "unknown"}]}
No dirty-bitmaps any more for drv.
(You'll want to heed [2] first before trying this, or you'll get a
segfault in the very first transaction)
I see two ways to solve this: As I said, you are free to call
qmp_block_dirty_bitmap_remove() even if creating the bitmap failed;
there is only one case where you must not call that function, and that
is if the bitmap existed before the transaction. In that case, you
should be calling block_dirty_bitmap_lookup() before
qmp_block_dirty_bitmap_add(), and if it succeeds you should not be
calling qmp_block_dirty_bitmap_remove(); if it fails you can call
qmp_block_dirty_bitmap_remove() (although I personally find that ugly
because you may be calling that function knowing that it will fail).
The other way is to evaluate the error returned by
qmp_block_dirty_bitmap_add() (pass it to a local Error pointer, remember
whether that was non-NULL, propagate it to errp). If there was an error,
do not call qmp_block_dirty_bitmap_remove() in
block_dirtybitmap_add_abort().
Max
+ }
+}
+
+/**
+ * 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),
[2] This should be sizeof(BlockDirtyBitmapState).
+ .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'
} }
##
