[RFC qemu 6/6] mirror: move some checks to QMP

[Fabian Grünbichler]

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 <address@hidden>
---
 block/mirror.c             | 28 +++------------
 blockdev.c                 | 29 +++++++++++++++
 tests/qemu-iotests/284.out | 72 +++++++++++++++++++-------------------
 3 files changed, 70 insertions(+), 59 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index d6aca2874e..7a3373fca7 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1562,31 +1562,13 @@ static BlockJob *mirror_start_job(
     Error *local_err = NULL;
     int ret;
 
-    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 23df9f76ba..87bde0a4f4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3847,7 +3847,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 (!has_bitmap) {
+            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 (has_bitmap) {
+        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;
diff --git a/tests/qemu-iotests/284.out b/tests/qemu-iotests/284.out
index 9b7408b6d6..06a2e29058 100644
--- a/tests/qemu-iotests/284.out
+++ b/tests/qemu-iotests/284.out
@@ -2681,45 +2681,45 @@ qemu_img compare "TEST_DIR/PID-img" 
"TEST_DIR/PID-fmirror3" ==> Identical, OK!
 -- Sync mode incremental tests --
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap-mode": "on-success", 
"device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", 
"sync": "incremental", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode 
without a bitmap"}}
+{"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name 
for 'incremental' sync mode"}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap-mode": "always", 
"device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", 
"sync": "incremental", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode 
without a bitmap"}}
+{"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name 
for 'incremental' sync mode"}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap-mode": "never", "device": 
"drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": 
"incremental", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode 
without a bitmap"}}
+{"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name 
for 'incremental' sync mode"}}
 
 {"execute": "blockdev-mirror", "arguments": {"device": "drive0", 
"filter-node-name": "mirror-top", "job-id": "api_job", "sync": "incremental", 
"target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Sync mode 'incremental' not 
supported"}}
+{"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name 
for 'incremental' sync mode"}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", 
"bitmap-mode": "on-success", "device": "drive0", "filter-node-name": 
"mirror-top", "job-id": "api_job", "sync": "incremental", "target": 
"mirror_target"}}
 {"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not 
found"}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", 
"bitmap-mode": "always", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "incremental", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not 
found"}}
+{"error": {"class": "GenericError", "desc": "Bitmap sync mode must be 
'on-success' when using sync mode 'incremental'"}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", 
"bitmap-mode": "never", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "incremental", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not 
found"}}
+{"error": {"class": "GenericError", "desc": "Bitmap sync mode must be 
'on-success' when using sync mode 'incremental'"}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", "device": 
"drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": 
"incremental", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "bitmap-mode must be specified if 
a bitmap is provided"}}
+{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not 
found"}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", 
"bitmap-mode": "always", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "incremental", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Sync mode 'incremental' not 
supported"}}
+{"error": {"class": "GenericError", "desc": "Bitmap sync mode must be 
'on-success' when using sync mode 'incremental'"}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", 
"bitmap-mode": "never", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "incremental", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Sync mode 'incremental' not 
supported"}}
+{"error": {"class": "GenericError", "desc": "Bitmap sync mode must be 
'on-success' when using sync mode 'incremental'"}}
 
 -- Sync mode bitmap tests --
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap-mode": "on-success", 
"device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", 
"sync": "bitmap", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode 
without a bitmap"}}
+{"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name 
for 'bitmap' sync mode"}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap-mode": "always", 
"device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", 
"sync": "bitmap", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode 
without a bitmap"}}
+{"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name 
for 'bitmap' sync mode"}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap-mode": "never", "device": 
"drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": 
"bitmap", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode 
without a bitmap"}}
+{"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name 
for 'bitmap' sync mode"}}
 
 {"execute": "blockdev-mirror", "arguments": {"device": "drive0", 
"filter-node-name": "mirror-top", "job-id": "api_job", "sync": "bitmap", 
"target": "mirror_target"}}
 {"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name 
for 'bitmap' sync mode"}}
@@ -2751,28 +2751,28 @@ qemu_img compare "TEST_DIR/PID-img" 
"TEST_DIR/PID-fmirror3" ==> Identical, OK!
 {"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode 
without a bitmap"}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", 
"bitmap-mode": "on-success", "device": "drive0", "filter-node-name": 
"mirror-top", "job-id": "api_job", "sync": "full", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not 
found"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", 
"bitmap-mode": "always", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "full", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not 
found"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", 
"bitmap-mode": "never", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "full", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not 
found"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", "device": 
"drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": 
"full", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "bitmap-mode must be specified if 
a bitmap is provided"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", 
"bitmap-mode": "on-success", "device": "drive0", "filter-node-name": 
"mirror-top", "job-id": "api_job", "sync": "full", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "sync mode 'full' is not 
compatible with bitmaps"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", 
"bitmap-mode": "always", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "full", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "sync mode 'full' is not 
compatible with bitmaps"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", 
"bitmap-mode": "never", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "full", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "sync mode 'full' is not 
compatible with bitmaps"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", "device": 
"drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": 
"full", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "bitmap-mode must be specified if 
a bitmap is provided"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 -- Sync mode top tests --
 
@@ -2786,28 +2786,28 @@ qemu_img compare "TEST_DIR/PID-img" 
"TEST_DIR/PID-fmirror3" ==> Identical, OK!
 {"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode 
without a bitmap"}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", 
"bitmap-mode": "on-success", "device": "drive0", "filter-node-name": 
"mirror-top", "job-id": "api_job", "sync": "top", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not 
found"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", 
"bitmap-mode": "always", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "top", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not 
found"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", 
"bitmap-mode": "never", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "top", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not 
found"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", "device": 
"drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "top", 
"target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "bitmap-mode must be specified if 
a bitmap is provided"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", 
"bitmap-mode": "on-success", "device": "drive0", "filter-node-name": 
"mirror-top", "job-id": "api_job", "sync": "top", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "sync mode 'full' is not 
compatible with bitmaps"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", 
"bitmap-mode": "always", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "top", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "sync mode 'full' is not 
compatible with bitmaps"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", 
"bitmap-mode": "never", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "top", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "sync mode 'full' is not 
compatible with bitmaps"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", "device": 
"drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "top", 
"target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "bitmap-mode must be specified if 
a bitmap is provided"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported 
with bitmap."}}
 
 -- Sync mode none tests --
 
@@ -2821,26 +2821,26 @@ qemu_img compare "TEST_DIR/PID-img" 
"TEST_DIR/PID-fmirror3" ==> Identical, OK!
 {"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode 
without a bitmap"}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", 
"bitmap-mode": "on-success", "device": "drive0", "filter-node-name": 
"mirror-top", "job-id": "api_job", "sync": "none", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not 
found"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", 
"bitmap-mode": "always", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "none", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not 
found"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", 
"bitmap-mode": "never", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "none", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not 
found"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", "device": 
"drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": 
"none", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "bitmap-mode must be specified if 
a bitmap is provided"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", 
"bitmap-mode": "on-success", "device": "drive0", "filter-node-name": 
"mirror-top", "job-id": "api_job", "sync": "none", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "sync mode 'none' is not 
compatible with bitmaps"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", 
"bitmap-mode": "always", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "none", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "sync mode 'none' is not 
compatible with bitmaps"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", 
"bitmap-mode": "never", "device": "drive0", "filter-node-name": "mirror-top", 
"job-id": "api_job", "sync": "none", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "sync mode 'none' is not 
compatible with bitmaps"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported 
with bitmap."}}
 
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", "device": 
"drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": 
"none", "target": "mirror_target"}}
-{"error": {"class": "GenericError", "desc": "bitmap-mode must be specified if 
a bitmap is provided"}}
+{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported 
with bitmap."}}
 
-- 
2.20.1