diff mbox series

[RFC,3/4] mirror: move some checks to qmp

Message ID 20240216105513.309901-4-f.ebner@proxmox.com (mailing list archive)
State New, archived
Headers show
Series mirror: implement incremental and bitmap modes | expand

Commit Message

Fiona Ebner Feb. 16, 2024, 10:55 a.m. UTC 
From: Fabian Grünbichler <f.gruenbichler@proxmox.com>

and assert the passing conditions in block/mirror.c. while incremental
mode was never available for drive-mirror, it makes the interface more
uniform w.r.t. backup block jobs.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
[FE: rebase for 9.0]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 block/mirror.c | 28 +++++-----------------------
 blockdev.c     | 29 +++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 23 deletions(-)
diff mbox series

Patch

diff --git a/block/mirror.c b/block/mirror.c
index 84155b1f78..15d1c060eb 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1755,31 +1755,13 @@  static BlockJob *mirror_start_job(
 
     GLOBAL_STATE_CODE();
 
-    if (sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
-        error_setg(errp, "Sync mode '%s' not supported",
-                   MirrorSyncMode_str(sync_mode));
-        return NULL;
-    } else if (sync_mode == MIRROR_SYNC_MODE_BITMAP) {
-        if (!bitmap) {
-            error_setg(errp, "Must provide a valid bitmap name for '%s'"
-                       " sync mode",
-                       MirrorSyncMode_str(sync_mode));
-            return NULL;
-        }
-    } else if (bitmap) {
-        error_setg(errp,
-                   "sync mode '%s' is not compatible with bitmaps",
-                   MirrorSyncMode_str(sync_mode));
-        return NULL;
-    }
+    /* QMP interface protects us from these cases */
+    assert(sync_mode != MIRROR_SYNC_MODE_INCREMENTAL);
+    assert((bitmap && sync_mode == MIRROR_SYNC_MODE_BITMAP) ||
+           (!bitmap && sync_mode != MIRROR_SYNC_MODE_BITMAP));
+    assert(!(bitmap && granularity));
 
     if (bitmap) {
-        if (granularity) {
-            error_setg(errp, "granularity (%d)"
-                       "cannot be specified when a bitmap is provided",
-                       granularity);
-            return NULL;
-        }
         granularity = bdrv_dirty_bitmap_granularity(bitmap);
 
         if (bitmap_mode != BITMAP_SYNC_MODE_NEVER) {
diff --git a/blockdev.c b/blockdev.c
index aeb9fde9f3..519f408359 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2852,7 +2852,36 @@  static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs,
         sync = MIRROR_SYNC_MODE_FULL;
     }
 
+    if ((sync == MIRROR_SYNC_MODE_BITMAP) ||
+        (sync == MIRROR_SYNC_MODE_INCREMENTAL)) {
+        /* done before desugaring 'incremental' to print the right message */
+        if (!bitmap_name) {
+            error_setg(errp, "Must provide a valid bitmap name for "
+                       "'%s' sync mode", MirrorSyncMode_str(sync));
+            return;
+        }
+    }
+
+    if (sync == MIRROR_SYNC_MODE_INCREMENTAL) {
+        if (has_bitmap_mode &&
+            bitmap_mode != BITMAP_SYNC_MODE_ON_SUCCESS) {
+            error_setg(errp, "Bitmap sync mode must be '%s' "
+                       "when using sync mode '%s'",
+                       BitmapSyncMode_str(BITMAP_SYNC_MODE_ON_SUCCESS),
+                       MirrorSyncMode_str(sync));
+            return;
+        }
+        has_bitmap_mode = true;
+        sync = MIRROR_SYNC_MODE_BITMAP;
+        bitmap_mode = BITMAP_SYNC_MODE_ON_SUCCESS;
+    }
+
     if (bitmap_name) {
+        if (sync != MIRROR_SYNC_MODE_BITMAP) {
+            error_setg(errp, "Sync mode '%s' not supported with bitmap.",
+                       MirrorSyncMode_str(sync));
+            return;
+        }
         if (granularity) {
             error_setg(errp, "Granularity and bitmap cannot both be set");
             return;