1 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
2 From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
3 Date: Mon, 6 Apr 2020 12:17:08 +0200
4 Subject: [PATCH] mirror: move some checks to qmp
6 Content-Type: text/plain; charset=UTF-8
7 Content-Transfer-Encoding: 8bit
9 and assert the passing conditions in block/mirror.c. while incremental
10 mode was never available for drive-mirror, it makes the interface more
11 uniform w.r.t. backup block jobs.
13 Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
14 Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
16 block/mirror.c | 28 +++------------
17 blockdev.c | 29 +++++++++++++++
18 tests/qemu-iotests/384.out | 72 +++++++++++++++++++-------------------
19 3 files changed, 70 insertions(+), 59 deletions(-)
21 diff --git a/block/mirror.c b/block/mirror.c
22 index 8b3342f9ec..1d4ff0efad 100644
25 @@ -1634,31 +1634,13 @@ static BlockJob *mirror_start_job(
26 uint64_t target_perms, target_shared_perms;
29 - if (sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
30 - error_setg(errp, "Sync mode '%s' not supported",
31 - MirrorSyncMode_str(sync_mode));
33 - } else if (sync_mode == MIRROR_SYNC_MODE_BITMAP) {
35 - error_setg(errp, "Must provide a valid bitmap name for '%s'"
37 - MirrorSyncMode_str(sync_mode));
40 - } else if (bitmap) {
42 - "sync mode '%s' is not compatible with bitmaps",
43 - MirrorSyncMode_str(sync_mode));
46 + /* QMP interface protects us from these cases */
47 + assert(sync_mode != MIRROR_SYNC_MODE_INCREMENTAL);
48 + assert((bitmap && sync_mode == MIRROR_SYNC_MODE_BITMAP) ||
49 + (!bitmap && sync_mode != MIRROR_SYNC_MODE_BITMAP));
50 + assert(!(bitmap && granularity));
54 - error_setg(errp, "granularity (%d)"
55 - "cannot be specified when a bitmap is provided",
59 granularity = bdrv_dirty_bitmap_granularity(bitmap);
61 if (bitmap_mode != BITMAP_SYNC_MODE_NEVER) {
62 diff --git a/blockdev.c b/blockdev.c
63 index a57b0af2e7..ce62a9b439 100644
66 @@ -3029,7 +3029,36 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs,
67 sync = MIRROR_SYNC_MODE_FULL;
70 + if ((sync == MIRROR_SYNC_MODE_BITMAP) ||
71 + (sync == MIRROR_SYNC_MODE_INCREMENTAL)) {
72 + /* done before desugaring 'incremental' to print the right message */
74 + error_setg(errp, "Must provide a valid bitmap name for "
75 + "'%s' sync mode", MirrorSyncMode_str(sync));
80 + if (sync == MIRROR_SYNC_MODE_INCREMENTAL) {
81 + if (has_bitmap_mode &&
82 + bitmap_mode != BITMAP_SYNC_MODE_ON_SUCCESS) {
83 + error_setg(errp, "Bitmap sync mode must be '%s' "
84 + "when using sync mode '%s'",
85 + BitmapSyncMode_str(BITMAP_SYNC_MODE_ON_SUCCESS),
86 + MirrorSyncMode_str(sync));
89 + has_bitmap_mode = true;
90 + sync = MIRROR_SYNC_MODE_BITMAP;
91 + bitmap_mode = BITMAP_SYNC_MODE_ON_SUCCESS;
95 + if (sync != MIRROR_SYNC_MODE_BITMAP) {
96 + error_setg(errp, "Sync mode '%s' not supported with bitmap.",
97 + MirrorSyncMode_str(sync));
101 error_setg(errp, "Granularity and bitmap cannot both be set");
103 diff --git a/tests/qemu-iotests/384.out b/tests/qemu-iotests/384.out
104 index 9b7408b6d6..06a2e29058 100644
105 --- a/tests/qemu-iotests/384.out
106 +++ b/tests/qemu-iotests/384.out
107 @@ -2681,45 +2681,45 @@ qemu_img compare "TEST_DIR/PID-img" "TEST_DIR/PID-fmirror3" ==> Identical, OK!
108 -- Sync mode incremental tests --
110 {"execute": "blockdev-mirror", "arguments": {"bitmap-mode": "on-success", "device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "incremental", "target": "mirror_target"}}
111 -{"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode without a bitmap"}}
112 +{"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name for 'incremental' sync mode"}}
114 {"execute": "blockdev-mirror", "arguments": {"bitmap-mode": "always", "device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "incremental", "target": "mirror_target"}}
115 -{"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode without a bitmap"}}
116 +{"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name for 'incremental' sync mode"}}
118 {"execute": "blockdev-mirror", "arguments": {"bitmap-mode": "never", "device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "incremental", "target": "mirror_target"}}
119 -{"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode without a bitmap"}}
120 +{"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name for 'incremental' sync mode"}}
122 {"execute": "blockdev-mirror", "arguments": {"device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "incremental", "target": "mirror_target"}}
123 -{"error": {"class": "GenericError", "desc": "Sync mode 'incremental' not supported"}}
124 +{"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name for 'incremental' sync mode"}}
126 {"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"}}
127 {"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not found"}}
129 {"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"}}
130 -{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not found"}}
131 +{"error": {"class": "GenericError", "desc": "Bitmap sync mode must be 'on-success' when using sync mode 'incremental'"}}
133 {"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"}}
134 -{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not found"}}
135 +{"error": {"class": "GenericError", "desc": "Bitmap sync mode must be 'on-success' when using sync mode 'incremental'"}}
137 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", "device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "incremental", "target": "mirror_target"}}
138 -{"error": {"class": "GenericError", "desc": "bitmap-mode must be specified if a bitmap is provided"}}
139 +{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not found"}}
141 {"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"}}
142 -{"error": {"class": "GenericError", "desc": "Sync mode 'incremental' not supported"}}
143 +{"error": {"class": "GenericError", "desc": "Bitmap sync mode must be 'on-success' when using sync mode 'incremental'"}}
145 {"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"}}
146 -{"error": {"class": "GenericError", "desc": "Sync mode 'incremental' not supported"}}
147 +{"error": {"class": "GenericError", "desc": "Bitmap sync mode must be 'on-success' when using sync mode 'incremental'"}}
149 -- Sync mode bitmap tests --
151 {"execute": "blockdev-mirror", "arguments": {"bitmap-mode": "on-success", "device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "bitmap", "target": "mirror_target"}}
152 -{"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode without a bitmap"}}
153 +{"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name for 'bitmap' sync mode"}}
155 {"execute": "blockdev-mirror", "arguments": {"bitmap-mode": "always", "device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "bitmap", "target": "mirror_target"}}
156 -{"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode without a bitmap"}}
157 +{"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name for 'bitmap' sync mode"}}
159 {"execute": "blockdev-mirror", "arguments": {"bitmap-mode": "never", "device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "bitmap", "target": "mirror_target"}}
160 -{"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode without a bitmap"}}
161 +{"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name for 'bitmap' sync mode"}}
163 {"execute": "blockdev-mirror", "arguments": {"device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "bitmap", "target": "mirror_target"}}
164 {"error": {"class": "GenericError", "desc": "Must provide a valid bitmap name for 'bitmap' sync mode"}}
165 @@ -2751,28 +2751,28 @@ qemu_img compare "TEST_DIR/PID-img" "TEST_DIR/PID-fmirror3" ==> Identical, OK!
166 {"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode without a bitmap"}}
168 {"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"}}
169 -{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not found"}}
170 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
172 {"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"}}
173 -{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not found"}}
174 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
176 {"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"}}
177 -{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not found"}}
178 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
180 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", "device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "full", "target": "mirror_target"}}
181 -{"error": {"class": "GenericError", "desc": "bitmap-mode must be specified if a bitmap is provided"}}
182 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
184 {"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"}}
185 -{"error": {"class": "GenericError", "desc": "sync mode 'full' is not compatible with bitmaps"}}
186 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
188 {"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"}}
189 -{"error": {"class": "GenericError", "desc": "sync mode 'full' is not compatible with bitmaps"}}
190 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
192 {"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"}}
193 -{"error": {"class": "GenericError", "desc": "sync mode 'full' is not compatible with bitmaps"}}
194 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
196 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", "device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "full", "target": "mirror_target"}}
197 -{"error": {"class": "GenericError", "desc": "bitmap-mode must be specified if a bitmap is provided"}}
198 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
200 -- Sync mode top tests --
202 @@ -2786,28 +2786,28 @@ qemu_img compare "TEST_DIR/PID-img" "TEST_DIR/PID-fmirror3" ==> Identical, OK!
203 {"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode without a bitmap"}}
205 {"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"}}
206 -{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not found"}}
207 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
209 {"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"}}
210 -{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not found"}}
211 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
213 {"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"}}
214 -{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not found"}}
215 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
217 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", "device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "top", "target": "mirror_target"}}
218 -{"error": {"class": "GenericError", "desc": "bitmap-mode must be specified if a bitmap is provided"}}
219 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
221 {"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"}}
222 -{"error": {"class": "GenericError", "desc": "sync mode 'full' is not compatible with bitmaps"}}
223 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
225 {"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"}}
226 -{"error": {"class": "GenericError", "desc": "sync mode 'full' is not compatible with bitmaps"}}
227 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
229 {"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"}}
230 -{"error": {"class": "GenericError", "desc": "sync mode 'full' is not compatible with bitmaps"}}
231 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
233 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", "device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "top", "target": "mirror_target"}}
234 -{"error": {"class": "GenericError", "desc": "bitmap-mode must be specified if a bitmap is provided"}}
235 +{"error": {"class": "GenericError", "desc": "Sync mode 'full' not supported with bitmap."}}
237 -- Sync mode none tests --
239 @@ -2821,26 +2821,26 @@ qemu_img compare "TEST_DIR/PID-img" "TEST_DIR/PID-fmirror3" ==> Identical, OK!
240 {"error": {"class": "GenericError", "desc": "Cannot specify bitmap sync mode without a bitmap"}}
242 {"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"}}
243 -{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not found"}}
244 +{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported with bitmap."}}
246 {"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"}}
247 -{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not found"}}
248 +{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported with bitmap."}}
250 {"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"}}
251 -{"error": {"class": "GenericError", "desc": "Dirty bitmap 'bitmap404' not found"}}
252 +{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported with bitmap."}}
254 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap404", "device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "none", "target": "mirror_target"}}
255 -{"error": {"class": "GenericError", "desc": "bitmap-mode must be specified if a bitmap is provided"}}
256 +{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported with bitmap."}}
258 {"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"}}
259 -{"error": {"class": "GenericError", "desc": "sync mode 'none' is not compatible with bitmaps"}}
260 +{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported with bitmap."}}
262 {"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"}}
263 -{"error": {"class": "GenericError", "desc": "sync mode 'none' is not compatible with bitmaps"}}
264 +{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported with bitmap."}}
266 {"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"}}
267 -{"error": {"class": "GenericError", "desc": "sync mode 'none' is not compatible with bitmaps"}}
268 +{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported with bitmap."}}
270 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", "device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": "none", "target": "mirror_target"}}
271 -{"error": {"class": "GenericError", "desc": "bitmap-mode must be specified if a bitmap is provided"}}
272 +{"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported with bitmap."}}