diff mbox

[v4,01/14] qapi: Add transaction support to block-dirty-bitmap operations

Message ID 1438238370-16212-2-git-send-email-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng July 30, 2015, 6:39 a.m. UTC 
From: John Snow <jsnow@redhat.com>

This adds two qmp commands to transactions.

block-dirty-bitmap-add allows you to create a bitmap simultaneously
alongside a new full backup to accomplish a clean synchronization
point.

block-dirty-bitmap-clear allows you to reset a bitmap back to as-if
it were new, which can also be used alongside a full backup to
accomplish a clean synchronization point.

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c                   |  19 +++++++-
 blockdev.c                | 114 +++++++++++++++++++++++++++++++++++++++++++++-
 docs/bitmaps.md           |   6 +--
 include/block/block.h     |   1 -
 include/block/block_int.h |   3 ++
 qapi-schema.json          |   6 ++-
 6 files changed, 139 insertions(+), 10 deletions(-)

Comments

Max Reitz Aug. 3, 2015, 4:49 p.m. UTC | #1 
On 30.07.2015 08:39, Fam Zheng wrote:
> From: John Snow <jsnow@redhat.com>
>
> This adds two qmp commands to transactions.
>
> block-dirty-bitmap-add allows you to create a bitmap simultaneously
> alongside a new full backup to accomplish a clean synchronization
> point.
>
> block-dirty-bitmap-clear allows you to reset a bitmap back to as-if
> it were new, which can also be used alongside a full backup to
> accomplish a clean synchronization point.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   block.c                   |  19 +++++++-
>   blockdev.c                | 114 +++++++++++++++++++++++++++++++++++++++++++++-
>   docs/bitmaps.md           |   6 +--
>   include/block/block.h     |   1 -
>   include/block/block_int.h |   3 ++
>   qapi-schema.json          |   6 ++-
>   6 files changed, 139 insertions(+), 10 deletions(-)
>
> diff --git a/block.c b/block.c
> index d088ee0..cee68c2 100644
> --- a/block.c
> +++ b/block.c
> @@ -3566,10 +3566,25 @@ void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>       hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
>   }
>
> -void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap)
> +void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)
>   {
>       assert(bdrv_dirty_bitmap_enabled(bitmap));
> -    hbitmap_reset_all(bitmap->bitmap);
> +    if (!out) {
> +        hbitmap_reset_all(bitmap->bitmap);
> +    } else {
> +        HBitmap *backup = bitmap->bitmap;
> +        bitmap->bitmap = hbitmap_alloc(bitmap->size,
> +                                       hbitmap_granularity(backup));
> +        *out = backup;
> +    }
> +}
> +
> +void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in)
> +{
> +    HBitmap *tmp = bitmap->bitmap;
> +    assert(bdrv_dirty_bitmap_enabled(bitmap));
> +    bitmap->bitmap = in;
> +    hbitmap_free(tmp);
>   }
>
>   void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
> diff --git a/blockdev.c b/blockdev.c
> index 62a4586..b80439b 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1708,6 +1708,106 @@ static void blockdev_backup_clean(BlkTransactionState *common)
>       }
>   }
>
> +typedef struct BlockDirtyBitmapState {
> +    BlkTransactionState common;
> +    BdrvDirtyBitmap *bitmap;
> +    BlockDriverState *bs;
> +    AioContext *aio_context;
> +    HBitmap *backup;
> +    bool prepared;
> +} BlockDirtyBitmapState;
> +
> +static void block_dirty_bitmap_add_prepare(BlkTransactionState *common,
> +                                           Error **errp)
> +{
> +    Error *local_err = NULL;
> +    BlockDirtyBitmapAdd *action;
> +    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
> +                                             common, common);
> +
> +    action = common->action->block_dirty_bitmap_add;
> +    /* AIO context taken and released within qmp_block_dirty_bitmap_add */
> +    qmp_block_dirty_bitmap_add(action->node, action->name,
> +                               action->has_granularity, action->granularity,
> +                               &local_err);
> +
> +    if (!local_err) {
> +        state->prepared = true;
> +    } else {
> +        error_propagate(errp, local_err);
> +    }
> +}
> +
> +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 be able to fail: IF the bitmap was added via .prepare(),
> +     * then the node reference and bitmap name must have been valid.
> +     */
> +    if (state->prepared) {
> +        qmp_block_dirty_bitmap_remove(action->node, action->name, &error_abort);
> +    }
> +}
> +
> +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,
> +                                              &state->aio_context,
> +                                              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;
> +    }
> +
> +    bdrv_clear_dirty_bitmap(state->bitmap, &state->backup);
> +    /* AioContext is released in .clean() */
> +}
> +
> +static void block_dirty_bitmap_clear_abort(BlkTransactionState *common)
> +{
> +    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
> +                                             common, common);
> +
> +    bdrv_undo_clear_dirty_bitmap(state->bitmap, state->backup);
> +}
> +
> +static void block_dirty_bitmap_clear_commit(BlkTransactionState *common)
> +{
> +    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
> +                                             common, common);
> +
> +    hbitmap_free(state->backup);
> +}
> +
> +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");
> @@ -1748,6 +1848,18 @@ static const BdrvActionOps actions[] = {
>           .abort = internal_snapshot_abort,
>           .clean = internal_snapshot_clean,
>       },
> +    [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ADD] = {
> +        .instance_size = sizeof(BlockDirtyBitmapState),
> +        .prepare = block_dirty_bitmap_add_prepare,
> +        .abort = block_dirty_bitmap_add_abort,
> +    },
> +    [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_CLEAR] = {
> +        .instance_size = sizeof(BlockDirtyBitmapState),
> +        .prepare = block_dirty_bitmap_clear_prepare,
> +        .commit = block_dirty_bitmap_clear_commit,
> +        .abort = block_dirty_bitmap_clear_abort,
> +        .clean = block_dirty_bitmap_clear_clean,
> +    }
>   };
>
>   /*
> @@ -2131,7 +2243,7 @@ void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
>           goto out;
>       }
>
> -    bdrv_clear_dirty_bitmap(bitmap);
> +    bdrv_clear_dirty_bitmap(bitmap, NULL);
>
>    out:
>       aio_context_release(aio_context);
> diff --git a/docs/bitmaps.md b/docs/bitmaps.md
> index fa87f07..9fd8ea6 100644
> --- a/docs/bitmaps.md
> +++ b/docs/bitmaps.md
> @@ -97,11 +97,7 @@ which is included at the end of this document.
>   }
>   ```
>
> -## Transactions (Not yet implemented)
> -
> -* Transactional commands are forthcoming in a future version,
> -  and are not yet available for use. This section serves as
> -  documentation of intent for their design and usage.
> +## Transactions
>
>   ### Justification
>
> diff --git a/include/block/block.h b/include/block/block.h
> index 37916f7..b1116e9 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -503,7 +503,6 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>                              int64_t cur_sector, int nr_sectors);
>   void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>                                int64_t cur_sector, int nr_sectors);
> -void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap);
>   void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
>   void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
>   int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 14ad4c3..281d790 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -664,4 +664,7 @@ void blk_dev_resize_cb(BlockBackend *blk);
>
>   void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
>
> +void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out);
> +void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in);
> +
>   #endif /* BLOCK_INT_H */
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 4342a08..83de942 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1493,6 +1493,8 @@
>   # abort since 1.6
>   # blockdev-snapshot-internal-sync since 1.7
>   # blockdev-backup since 2.3
> +# block-dirty-bitmap-add since 2.4
> +# block-dirty-bitmap-clear since 2.4

*2.5, and keep my R-b.

Max

>   ##
>   { 'union': 'TransactionAction',
>     'data': {
> @@ -1500,7 +1502,9 @@
>          '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-clear': 'BlockDirtyBitmap'
>      } }
>
>   ##
>
Fam Zheng Sept. 7, 2015, 6:32 a.m. UTC | #2 
On Mon, 08/03 18:49, Max Reitz wrote:
> *2.5, and keep my R-b.

Sure! Thanks.

Fam
diff mbox

Patch

diff --git a/block.c b/block.c
index d088ee0..cee68c2 100644
--- a/block.c
+++ b/block.c
@@ -3566,10 +3566,25 @@  void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
     hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
 }
 
-void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)
 {
     assert(bdrv_dirty_bitmap_enabled(bitmap));
-    hbitmap_reset_all(bitmap->bitmap);
+    if (!out) {
+        hbitmap_reset_all(bitmap->bitmap);
+    } else {
+        HBitmap *backup = bitmap->bitmap;
+        bitmap->bitmap = hbitmap_alloc(bitmap->size,
+                                       hbitmap_granularity(backup));
+        *out = backup;
+    }
+}
+
+void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in)
+{
+    HBitmap *tmp = bitmap->bitmap;
+    assert(bdrv_dirty_bitmap_enabled(bitmap));
+    bitmap->bitmap = in;
+    hbitmap_free(tmp);
 }
 
 void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
diff --git a/blockdev.c b/blockdev.c
index 62a4586..b80439b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1708,6 +1708,106 @@  static void blockdev_backup_clean(BlkTransactionState *common)
     }
 }
 
+typedef struct BlockDirtyBitmapState {
+    BlkTransactionState common;
+    BdrvDirtyBitmap *bitmap;
+    BlockDriverState *bs;
+    AioContext *aio_context;
+    HBitmap *backup;
+    bool prepared;
+} BlockDirtyBitmapState;
+
+static void block_dirty_bitmap_add_prepare(BlkTransactionState *common,
+                                           Error **errp)
+{
+    Error *local_err = NULL;
+    BlockDirtyBitmapAdd *action;
+    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+                                             common, common);
+
+    action = common->action->block_dirty_bitmap_add;
+    /* AIO context taken and released within qmp_block_dirty_bitmap_add */
+    qmp_block_dirty_bitmap_add(action->node, action->name,
+                               action->has_granularity, action->granularity,
+                               &local_err);
+
+    if (!local_err) {
+        state->prepared = true;
+    } else {
+        error_propagate(errp, local_err);
+    }
+}
+
+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 be able to fail: IF the bitmap was added via .prepare(),
+     * then the node reference and bitmap name must have been valid.
+     */
+    if (state->prepared) {
+        qmp_block_dirty_bitmap_remove(action->node, action->name, &error_abort);
+    }
+}
+
+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,
+                                              &state->aio_context,
+                                              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;
+    }
+
+    bdrv_clear_dirty_bitmap(state->bitmap, &state->backup);
+    /* AioContext is released in .clean() */
+}
+
+static void block_dirty_bitmap_clear_abort(BlkTransactionState *common)
+{
+    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+                                             common, common);
+
+    bdrv_undo_clear_dirty_bitmap(state->bitmap, state->backup);
+}
+
+static void block_dirty_bitmap_clear_commit(BlkTransactionState *common)
+{
+    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+                                             common, common);
+
+    hbitmap_free(state->backup);
+}
+
+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");
@@ -1748,6 +1848,18 @@  static const BdrvActionOps actions[] = {
         .abort = internal_snapshot_abort,
         .clean = internal_snapshot_clean,
     },
+    [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ADD] = {
+        .instance_size = sizeof(BlockDirtyBitmapState),
+        .prepare = block_dirty_bitmap_add_prepare,
+        .abort = block_dirty_bitmap_add_abort,
+    },
+    [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_CLEAR] = {
+        .instance_size = sizeof(BlockDirtyBitmapState),
+        .prepare = block_dirty_bitmap_clear_prepare,
+        .commit = block_dirty_bitmap_clear_commit,
+        .abort = block_dirty_bitmap_clear_abort,
+        .clean = block_dirty_bitmap_clear_clean,
+    }
 };
 
 /*
@@ -2131,7 +2243,7 @@  void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
         goto out;
     }
 
-    bdrv_clear_dirty_bitmap(bitmap);
+    bdrv_clear_dirty_bitmap(bitmap, NULL);
 
  out:
     aio_context_release(aio_context);
diff --git a/docs/bitmaps.md b/docs/bitmaps.md
index fa87f07..9fd8ea6 100644
--- a/docs/bitmaps.md
+++ b/docs/bitmaps.md
@@ -97,11 +97,7 @@  which is included at the end of this document.
 }
 ```
 
-## Transactions (Not yet implemented)
-
-* Transactional commands are forthcoming in a future version,
-  and are not yet available for use. This section serves as
-  documentation of intent for their design and usage.
+## Transactions
 
 ### Justification
 
diff --git a/include/block/block.h b/include/block/block.h
index 37916f7..b1116e9 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -503,7 +503,6 @@  void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
                            int64_t cur_sector, int nr_sectors);
 void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
                              int64_t cur_sector, int nr_sectors);
-void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
 void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
 int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 14ad4c3..281d790 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -664,4 +664,7 @@  void blk_dev_resize_cb(BlockBackend *blk);
 
 void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
 
+void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out);
+void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in);
+
 #endif /* BLOCK_INT_H */
diff --git a/qapi-schema.json b/qapi-schema.json
index 4342a08..83de942 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1493,6 +1493,8 @@ 
 # abort since 1.6
 # blockdev-snapshot-internal-sync since 1.7
 # blockdev-backup since 2.3
+# block-dirty-bitmap-add since 2.4
+# block-dirty-bitmap-clear since 2.4
 ##
 { 'union': 'TransactionAction',
   'data': {
@@ -1500,7 +1502,9 @@ 
        '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-clear': 'BlockDirtyBitmap'
    } }
 
 ##