[Qemu-devel] [RFC] dirty-bitmaps: add block-dirty-bitmap-persist command

[John Snow]

This is for the purpose of toggling on/off persistence on a bitmap.
This enables you to save a bitmap that was not persistent, but may
have already accumulated valuable data.

This is simply a QOL enhancement:
- Allows user to "upgrade" an existing bitmap to persistent
- Allows user to "downgrade" an existing bitmap to transient,
  removing it from storage without deleting the bitmap.

Signed-off-by: John Snow <address@hidden>
---

This is just an RFC because I'm not sure if I really want to pursue
adding this, but it was raised in a discussion I had recently that it
was a little annoying as an API design that persistence couldn't be
changed after addition, so I wanted to see how much code it would take
to address that.

(So this patch isn't really tested; just: "Hey, look!")

I don't like this patch because it exacerbates my perceived problems
with the "check if I can make it persistent, then toggle the flag"
model, where I prefer the "Just try to set it persistent and let it fail
if it cannot" model, but there were some issues with that patchset that
I want to revisit.

---

 blockdev.c           | 49 ++++++++++++++++++++++++++++++++++++++++++++
 qapi/block-core.json | 34 ++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 2d7e7be538..230442e921 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3095,6 +3095,55 @@ void qmp_block_dirty_bitmap_merge(const char *node, 
const char *target,
     do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
 }
 
+void qmp_block_dirty_bitmap_persist(const char *node, const char *name,
+                                    bool persist, Error **errp)
+{
+    BdrvDirtyBitmap *bitmap;
+    BlockDriverState *bs;
+    AioContext *aio_context = NULL;
+    Error *local_err = NULL;
+    bool persistent;
+
+    bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
+    if (!bitmap || !bs) {
+        return;
+    }
+
+    if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, errp)) {
+        return;
+    }
+
+    persistent = bdrv_dirty_bitmap_get_persistence(bitmap);
+
+    if (persist != persistent) {
+        aio_context = bdrv_get_aio_context(bs);
+        aio_context_acquire(aio_context);
+    }
+
+    if (!persist && persistent) {
+        bdrv_remove_persistent_dirty_bitmap(bs, name, &local_err);
+        if (local_err != NULL) {
+            error_propagate(errp, local_err);
+            goto out;
+        }
+    }
+
+    if (persist && !persistent) {
+        uint32_t granularity = bdrv_dirty_bitmap_granularity(bitmap);
+        if (!bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp)) {
+            goto out;
+        }
+    }
+
+    bdrv_dirty_bitmap_set_persistence(bitmap, persistent);
+
+ out:
+    if (aio_context) {
+        aio_context_release(aio_context);
+    }
+    return;
+}
+
 BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
                                                               const char *name,
                                                               Error **errp)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 3dbf23d874..9c0957f528 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2001,6 +2001,19 @@
   'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
             '*persistent': 'bool', '*autoload': 'bool', '*disabled': 'bool' } }
 
+##
+# @BlockDirtyBitmapPersist:
+#
+# @persist: True sets the specified bitmap as persistent.
+#           False will remove it from storage and mark it transient.
+#
+# Since: 4.2
+##
+{ 'struct': 'BlockDirtyBitmapPersist',
+  'base': 'BlockDirtyBitmap',
+  'data': { 'persist': 'bool' }
+}
+
 ##
 # @BlockDirtyBitmapMergeSource:
 #
@@ -2173,6 +2186,27 @@
       { 'command': 'block-dirty-bitmap-merge',
         'data': 'BlockDirtyBitmapMerge' }
 
+##
+# @block-dirty-bitmap-persist:
+#
+# Mark a dirty bitmap as either persistent or transient.
+#
+# Returns: nothing on success; including for no-op.
+#          GenericError with explanation if the operation did not succeed.
+#
+# Example:
+#
+# -> { "execute": "block-dirty-bitmap-persist",
+#      "arguments": { "node": "drive0",
+#                     "bitmap": "bitmap0",
+#                     "persist": true } }
+# <- { "return": {} }
+#
+# Since: 4.2
+##
+{ 'command': 'block-dirty-bitmap-persist',
+  'data': 'BlockDirtyBitmapPersist' }
+
 ##
 # @BlockDirtyBitmapSha256:
 #
-- 
2.21.0