Re: [Qemu-block] [PATCH v2 4/6] qmp: transaction support for block-dirty-bitmap-enable/disable

[Vladimir Sementsov-Ogievskiy]

16.01.2018 15:54, Vladimir Sementsov-Ogievskiy wrote:

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  qapi/transaction.json |  4 +++
  blockdev.c| 79 +++
  2 files changed, 83 insertions(+)

diff --git a/qapi/transaction.json b/qapi/transaction.json
index bd312792da..b643d848f8 100644
--- a/qapi/transaction.json
+++ b/qapi/transaction.json
@@ -46,6 +46,8 @@
  # - @abort: since 1.6
  # - @block-dirty-bitmap-add: since 2.5
  # - @block-dirty-bitmap-clear: since 2.5
+# - @block-dirty-bitmap-enable: since 2.12
+# - @block-dirty-bitmap-disable: since 2.12
  # - @blockdev-backup: since 2.3
  # - @blockdev-snapshot: since 2.5
  # - @blockdev-snapshot-internal-sync: since 1.7
@@ -59,6 +61,8 @@
 'abort': 'Abort',
 'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
 'block-dirty-bitmap-clear': 'BlockDirtyBitmap',
+   'block-dirty-bitmap-enable': 'BlockDirtyBitmap',
+   'block-dirty-bitmap-disable': 'BlockDirtyBitmap',
 'blockdev-backup': 'BlockdevBackup',
 'blockdev-snapshot': 'BlockdevSnapshot',
 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
diff --git a/blockdev.c b/blockdev.c
index 997a6f514c..d338363d78 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1962,6 +1962,7 @@ typedef struct BlockDirtyBitmapState {
  AioContext *aio_context;
  HBitmap *backup;
  bool prepared;
+bool was_enabled;
  } BlockDirtyBitmapState;
  
  static void block_dirty_bitmap_add_prepare(BlkActionState *common,

@@ -2069,6 +2070,74 @@ static void 
block_dirty_bitmap_clear_clean(BlkActionState *common)
  }
  }
  
+static void block_dirty_bitmap_enable_prepare(BlkActionState *common,

+  Error **errp)
+{
+BlockDirtyBitmap *action;
+BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+
+if (action_check_completion_mode(common, errp) < 0) {
+return;
+}
+
+action = common->action->u.block_dirty_bitmap_enable.data;
+state->bitmap = block_dirty_bitmap_lookup(action->node,
+  action->name,
+  &state->bs,
+  errp);
+if (!state->bitmap) {
+return;
+}
+
+state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
+bdrv_enable_dirty_bitmap(state->bitmap);
+}
+
+static void block_dirty_bitmap_enable_abort(BlkActionState *common)
+{
+BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+
+if (!state->was_enabled) {
+bdrv_disable_dirty_bitmap(state->bitmap);
+}
+}
+
+static void block_dirty_bitmap_disable_prepare(BlkActionState *common,
+   Error **errp)
+{
+BlockDirtyBitmap *action;
+BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+
+if (action_check_completion_mode(common, errp) < 0) {
+return;
+}
+
+action = common->action->u.block_dirty_bitmap_disable.data;
+state->bitmap = block_dirty_bitmap_lookup(action->node,
+  action->name,
+  &state->bs,
+  errp);
+if (!state->bitmap) {
+return;
+}
+
+state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
+bdrv_disable_dirty_bitmap(state->bitmap);


hm. actually, I just make it like _clear is made. But now I doubt,
why do we need disable here? we can disable in commit, and do not need
state->was_enabled..

and for _clear, we not to clear bitmap in commit? and then, we do not need
bdrv_undo_clear_dirty_bitmap at all?

or what do I miss? if nothing, I'll respin the series including _clear 
refactoring.



+}
+
+static void block_dirty_bitmap_disable_abort(BlkActionState *common)
+{
+BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+
+if (state->was_enabled) {
+bdrv_enable_dirty_bitmap(state->bitmap);
+}
+}
+
  static void abort_prepare(BlkActionState *common, Error **errp)
  {
  error_setg(errp, "Transaction aborted using Abort action");
@@ -2130,6 +2199,16 @@ static const BlkActionOps actions[] = {
  .commit = block_dirty_bitmap_clear_commit,
  .abort = block_dirty_bitmap_clear_abort,
  .clean = block_dirty_bitmap_clear_clean,
+},
+[TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ENABLE] = {
+.instance_size = sizeof(BlockDirtyBitmapState),
+.prepare = block_dirty_bitmap_enable_prepare,
+.abort = block_dirty_bitmap_enable_abort,
+},
+[TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_DISABLE] = {
+

[Qemu-block] [PATCH v2 4/6] qmp: transaction support for block-dirty-bitmap-enable/disable

[Vladimir Sementsov-Ogievskiy]

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 qapi/transaction.json |  4 +++
 blockdev.c| 79 +++
 2 files changed, 83 insertions(+)

diff --git a/qapi/transaction.json b/qapi/transaction.json
index bd312792da..b643d848f8 100644
--- a/qapi/transaction.json
+++ b/qapi/transaction.json
@@ -46,6 +46,8 @@
 # - @abort: since 1.6
 # - @block-dirty-bitmap-add: since 2.5
 # - @block-dirty-bitmap-clear: since 2.5
+# - @block-dirty-bitmap-enable: since 2.12
+# - @block-dirty-bitmap-disable: since 2.12
 # - @blockdev-backup: since 2.3
 # - @blockdev-snapshot: since 2.5
 # - @blockdev-snapshot-internal-sync: since 1.7
@@ -59,6 +61,8 @@
'abort': 'Abort',
'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
'block-dirty-bitmap-clear': 'BlockDirtyBitmap',
+   'block-dirty-bitmap-enable': 'BlockDirtyBitmap',
+   'block-dirty-bitmap-disable': 'BlockDirtyBitmap',
'blockdev-backup': 'BlockdevBackup',
'blockdev-snapshot': 'BlockdevSnapshot',
'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
diff --git a/blockdev.c b/blockdev.c
index 997a6f514c..d338363d78 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1962,6 +1962,7 @@ typedef struct BlockDirtyBitmapState {
 AioContext *aio_context;
 HBitmap *backup;
 bool prepared;
+bool was_enabled;
 } BlockDirtyBitmapState;
 
 static void block_dirty_bitmap_add_prepare(BlkActionState *common,
@@ -2069,6 +2070,74 @@ static void 
block_dirty_bitmap_clear_clean(BlkActionState *common)
 }
 }
 
+static void block_dirty_bitmap_enable_prepare(BlkActionState *common,
+  Error **errp)
+{
+BlockDirtyBitmap *action;
+BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+
+if (action_check_completion_mode(common, errp) < 0) {
+return;
+}
+
+action = common->action->u.block_dirty_bitmap_enable.data;
+state->bitmap = block_dirty_bitmap_lookup(action->node,
+  action->name,
+  &state->bs,
+  errp);
+if (!state->bitmap) {
+return;
+}
+
+state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
+bdrv_enable_dirty_bitmap(state->bitmap);
+}
+
+static void block_dirty_bitmap_enable_abort(BlkActionState *common)
+{
+BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+
+if (!state->was_enabled) {
+bdrv_disable_dirty_bitmap(state->bitmap);
+}
+}
+
+static void block_dirty_bitmap_disable_prepare(BlkActionState *common,
+   Error **errp)
+{
+BlockDirtyBitmap *action;
+BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+
+if (action_check_completion_mode(common, errp) < 0) {
+return;
+}
+
+action = common->action->u.block_dirty_bitmap_disable.data;
+state->bitmap = block_dirty_bitmap_lookup(action->node,
+  action->name,
+  &state->bs,
+  errp);
+if (!state->bitmap) {
+return;
+}
+
+state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
+bdrv_disable_dirty_bitmap(state->bitmap);
+}
+
+static void block_dirty_bitmap_disable_abort(BlkActionState *common)
+{
+BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+
+if (state->was_enabled) {
+bdrv_enable_dirty_bitmap(state->bitmap);
+}
+}
+
 static void abort_prepare(BlkActionState *common, Error **errp)
 {
 error_setg(errp, "Transaction aborted using Abort action");
@@ -2130,6 +2199,16 @@ static const BlkActionOps actions[] = {
 .commit = block_dirty_bitmap_clear_commit,
 .abort = block_dirty_bitmap_clear_abort,
 .clean = block_dirty_bitmap_clear_clean,
+},
+[TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ENABLE] = {
+.instance_size = sizeof(BlockDirtyBitmapState),
+.prepare = block_dirty_bitmap_enable_prepare,
+.abort = block_dirty_bitmap_enable_abort,
+},
+[TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_DISABLE] = {
+.instance_size = sizeof(BlockDirtyBitmapState),
+.prepare = block_dirty_bitmap_disable_prepare,
+.abort = block_dirty_bitmap_disable_abort,
 }
 };
 
-- 
2.11.1