From 5e771752a1ffba3a99d7d75b6d492b4a86b59e1b Mon Sep 17 00:00:00 2001
From: Sergio Lopez <slp@redhat.com>
Date: Fri, 8 Mar 2019 16:48:53 +0100
Subject: [PATCH 01/12] mirror: Confirm we're quiesced only if the job is
paused or cancelled
While child_job_drained_begin() calls to job_pause(), the job doesn't
actually transition between states until it runs again and reaches a
pause point. This means bdrv_drained_begin() may return with some jobs
using the node still having 'busy == true'.
As a consequence, block_job_detach_aio_context() may get into a
deadlock, waiting for the job to be actually paused, while the coroutine
servicing the job is yielding and doesn't get the opportunity to get
scheduled again. This situation can be reproduced by issuing a
'block-commit' immediately followed by a 'device_del'.
To ensure bdrv_drained_begin() only returns when the jobs have been
paused, we change mirror_drained_poll() to only confirm it's quiesced
when job->paused == true and there aren't any in-flight requests, except
if we reached that point by a drained section initiated by the
mirror/commit job itself.
The other block jobs shouldn't need any changes, as the default
drained_poll() behavior is to only confirm it's quiesced if the job is
not busy or completed.
Signed-off-by: Sergio Lopez <slp@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/mirror.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/block/mirror.c b/block/mirror.c
index 010fdafd79..eb9a4cdf56 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -80,6 +80,7 @@ typedef struct MirrorBlockJob {
bool initial_zeroing_ongoing;
int in_active_write_counter;
bool prepared;
+ bool in_drain;
} MirrorBlockJob;
typedef struct MirrorBDSOpaque {
@@ -683,6 +684,7 @@ static int mirror_exit_common(Job *job)
/* The mirror job has no requests in flight any more, but we need to
* drain potential other users of the BDS before changing the graph. */
+ assert(s->in_drain);
bdrv_drained_begin(target_bs);
bdrv_replace_node(to_replace, target_bs, &local_err);
bdrv_drained_end(target_bs);
@@ -721,6 +723,7 @@ static int mirror_exit_common(Job *job)
bs_opaque->job = NULL;
bdrv_drained_end(src);
+ s->in_drain = false;
bdrv_unref(mirror_top_bs);
bdrv_unref(src);
@@ -1004,10 +1007,12 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
*/
trace_mirror_before_drain(s, cnt);
+ s->in_drain = true;
bdrv_drained_begin(bs);
cnt = bdrv_get_dirty_count(s->dirty_bitmap);
if (cnt > 0 || mirror_flush(s) < 0) {
bdrv_drained_end(bs);
+ s->in_drain = false;
continue;
}
@@ -1055,6 +1060,7 @@ immediate_exit:
bdrv_dirty_iter_free(s->dbi);
if (need_drain) {
+ s->in_drain = true;
bdrv_drained_begin(bs);
}
@@ -1123,6 +1129,16 @@ static void coroutine_fn mirror_pause(Job *job)
static bool mirror_drained_poll(BlockJob *job)
{
MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+
+ /* If the job isn't paused nor cancelled, we can't be sure that it won't
+ * issue more requests. We make an exception if we've reached this point
+ * from one of our own drain sections, to avoid a deadlock waiting for
+ * ourselves.
+ */
+ if (!s->common.job.paused && !s->common.job.cancelled && !s->in_drain) {
+ return true;
+ }
+
return !!s->in_flight;
}
From a0cf83639c7adbdee08a5bac840c5aada019b8f3 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Wed, 13 Mar 2019 15:22:38 +0100
Subject: [PATCH 02/12] qcow2: Fix data file error condition in
qcow2_co_create()
We were trying to check whether bdrv_open_blockdev_ref() returned
success, but accidentally checked the wrong variable. Spotted by
Coverity (CID 1399703).
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
---
block/qcow2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 0dd77c6367..d507ee0686 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3073,7 +3073,7 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
goto out;
}
data_bs = bdrv_open_blockdev_ref(qcow2_opts->data_file, errp);
- if (bs == NULL) {
+ if (data_bs == NULL) {
ret = -EIO;
goto out;
}
From 1f46ab2e526d1fc0716c76dc78f2180fd68b09c1 Mon Sep 17 00:00:00 2001
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Date: Thu, 14 Mar 2019 11:52:21 +0300
Subject: [PATCH 03/12] qapi: fix block-latency-histogram-set description and
examples
There no @device parameter, only the @id one.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qapi/block-core.json | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 12c5e73551..7ccbfff9d0 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -565,7 +565,7 @@
#
# Manage read, write and flush latency histograms for the device.
#
-# If only @device parameter is specified, remove all present latency histograms
+# If only @id parameter is specified, remove all present latency histograms
# for the device. Otherwise, add/reset some of (or all) latency histograms.
#
# @id: The name or QOM path of the guest device.
@@ -597,7 +597,7 @@
# [0, 10), [10, 50), [50, 100), [100, +inf):
#
# -> { "execute": "block-latency-histogram-set",
-# "arguments": { "device": "drive0",
+# "arguments": { "id": "drive0",
# "boundaries": [10, 50, 100] } }
# <- { "return": {} }
#
@@ -605,7 +605,7 @@
# not changed (or not created):
#
# -> { "execute": "block-latency-histogram-set",
-# "arguments": { "device": "drive0",
+# "arguments": { "id": "drive0",
# "boundaries-write": [10, 50, 100] } }
# <- { "return": {} }
#
@@ -614,7 +614,7 @@
# write: [0, 1000), [1000, 5000), [5000, +inf)
#
# -> { "execute": "block-latency-histogram-set",
-# "arguments": { "device": "drive0",
+# "arguments": { "id": "drive0",
# "boundaries": [10, 50, 100],
# "boundaries-write": [1000, 5000] } }
# <- { "return": {} }
@@ -622,7 +622,7 @@
# Example: remove all latency histograms:
#
# -> { "execute": "block-latency-histogram-set",
-# "arguments": { "device": "drive0" } }
+# "arguments": { "id": "drive0" } }
# <- { "return": {} }
##
{ 'command': 'block-latency-histogram-set',
From b69864e5a8c7b762e94d1bfd170b8774b28ab993 Mon Sep 17 00:00:00 2001
From: Sam Eiderman <shmuel.eiderman@oracle.com>
Date: Thu, 14 Mar 2019 16:14:37 +0200
Subject: [PATCH 04/12] vmdk: Support version=3 in VMDK descriptor files
Commit 509d39aa22909c0ed1aabf896865f19c81fb38a1 added support for read
only VMDKs of version 3.
This commit fixes the probe function to correctly handle descriptors of
version 3.
This commit has two effects:
1. We no longer need to supply '-f vmdk' when pointing to descriptor
files of version 3 in qemu/qemu-img command line arguments.
2. This fixes the scenario where a VMDK points to a parent version 3
descriptor file which is being probed as "raw" instead of "vmdk".
Reviewed-by: Arbel Moshe <arbel.moshe@oracle.com>
Reviewed-by: Mark Kanda <mark.kanda@oracle.com>
Signed-off-by: Shmuel Eiderman <shmuel.eiderman@oracle.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vmdk.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index d8c0c50390..8dec6ef767 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -195,13 +195,15 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
}
if (end - p >= strlen("version=X\n")) {
if (strncmp("version=1\n", p, strlen("version=1\n")) == 0 ||
- strncmp("version=2\n", p, strlen("version=2\n")) == 0) {
+ strncmp("version=2\n", p, strlen("version=2\n")) == 0 ||
+ strncmp("version=3\n", p, strlen("version=3\n")) == 0) {
return 100;
}
}
if (end - p >= strlen("version=X\r\n")) {
if (strncmp("version=1\r\n", p, strlen("version=1\r\n")) == 0 ||
- strncmp("version=2\r\n", p, strlen("version=2\r\n")) == 0) {
+ strncmp("version=2\r\n", p, strlen("version=2\r\n")) == 0 ||
+ strncmp("version=3\r\n", p, strlen("version=3\r\n")) == 0) {
return 100;
}
}
From 2345bde647a179ac88723e8e0c2fc1643b3a7a7e Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Fri, 15 Mar 2019 12:15:16 +0100
Subject: [PATCH 05/12] block: Silence Coverity in bdrv_drop_intermediate()
Coverity doesn't like that the return value of bdrv_check_update_perm()
stays unused only in this place (CID 1399710).
Even if checking local_err should be equivalent to checking ret < 0,
let's switch to using the return value to be more consistent (and in
case of a bug somewhere down the call chain, forgetting to assign errp
is more likely than returning 0 for an error case).
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
block.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index ed9253c786..0a93ee9ac8 100644
--- a/block.c
+++ b/block.c
@@ -4350,11 +4350,10 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
QLIST_FOREACH_SAFE(c, &top->parents, next_parent, next) {
/* Check whether we are allowed to switch c from top to base */
GSList *ignore_children = g_slist_prepend(NULL, c);
- bdrv_check_update_perm(base, NULL, c->perm, c->shared_perm,
- ignore_children, &local_err);
+ ret = bdrv_check_update_perm(base, NULL, c->perm, c->shared_perm,
+ ignore_children, &local_err);
g_slist_free(ignore_children);
- if (local_err) {
- ret = -EPERM;
+ if (ret < 0) {
error_report_err(local_err);
goto exit;
}
From 9cd97956cfdde85d5887f2ea54ff598f615ee1b1 Mon Sep 17 00:00:00 2001
From: Sergio Lopez <slp@redhat.com>
Date: Fri, 15 Mar 2019 12:46:55 +0100
Subject: [PATCH 06/12] iotests: 153: Wait for an answer to QMP commands
There are various actions in this test that must be executed
sequentially, as the result of it depends on the state triggered by the
previous one.
If the last argument of _send_qemu_cmd() is an empty string, it just
sends the QMP commands without waiting for an answer. While unlikely, it
may happen that the next action in the test gets invoked before QEMU
processes the QMP request.
This issue seems to be easier to reproduce on servers with limited
resources or highly loaded.
With this change, we wait for an answer on all _send_qemu_cmd() calls.
Signed-off-by: Sergio Lopez <slp@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/153 | 12 ++++++------
tests/qemu-iotests/153.out | 6 ++++++
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153
index c989c2495f..08ad8a6730 100755
--- a/tests/qemu-iotests/153
+++ b/tests/qemu-iotests/153
@@ -155,7 +155,7 @@ for opts1 in "" "read-only=on" "read-only=on,force-share=on"; do
_img_info -U | grep 'file format'
fi
done
- _send_qemu_cmd $h "{ 'execute': 'quit', }" ""
+ _send_qemu_cmd $h "{ 'execute': 'quit' }" ''
echo
echo "Round done"
_cleanup_qemu
@@ -219,7 +219,7 @@ echo "Adding drive"
_send_qemu_cmd $QEMU_HANDLE \
"{ 'execute': 'human-monitor-command',
'arguments': { 'command-line': 'drive_add 0 if=none,id=d0,file=${TEST_IMG}' } }" \
- ""
+ 'return'
_run_cmd $QEMU_IO "${TEST_IMG}" -c 'write 0 512'
@@ -230,7 +230,7 @@ echo "== Closing an image should unlock it =="
_send_qemu_cmd $QEMU_HANDLE \
"{ 'execute': 'human-monitor-command',
'arguments': { 'command-line': 'drive_del d0' } }" \
- ""
+ 'return'
_run_cmd $QEMU_IO "${TEST_IMG}" -c 'write 0 512'
@@ -239,7 +239,7 @@ for d in d0 d1; do
_send_qemu_cmd $QEMU_HANDLE \
"{ 'execute': 'human-monitor-command',
'arguments': { 'command-line': 'drive_add 0 if=none,id=$d,file=${TEST_IMG},readonly=on' } }" \
- ""
+ 'return'
done
_run_cmd $QEMU_IMG info "${TEST_IMG}"
@@ -247,7 +247,7 @@ _run_cmd $QEMU_IMG info "${TEST_IMG}"
_send_qemu_cmd $QEMU_HANDLE \
"{ 'execute': 'human-monitor-command',
'arguments': { 'command-line': 'drive_del d0' } }" \
- ""
+ 'return'
_run_cmd $QEMU_IO "${TEST_IMG}" -c 'write 0 512'
@@ -255,7 +255,7 @@ echo "Closing the other"
_send_qemu_cmd $QEMU_HANDLE \
"{ 'execute': 'human-monitor-command',
'arguments': { 'command-line': 'drive_del d1' } }" \
- ""
+ 'return'
_run_cmd $QEMU_IO "${TEST_IMG}" -c 'write 0 512'
diff --git a/tests/qemu-iotests/153.out b/tests/qemu-iotests/153.out
index 884254868c..9747ce3c41 100644
--- a/tests/qemu-iotests/153.out
+++ b/tests/qemu-iotests/153.out
@@ -417,6 +417,7 @@ Is another process using the image [TEST_DIR/t.qcow2]?
_qemu_img_wrapper commit -b TEST_DIR/t.qcow2.b TEST_DIR/t.qcow2.c
{"return": {}}
Adding drive
+{"return": "OKrn"}
_qemu_io_wrapper TEST_DIR/t.qcow2 -c write 0 512
can't open device TEST_DIR/t.qcow2: Failed to get "write" lock
@@ -425,16 +426,21 @@ Creating overlay with qemu-img when the guest is running should be allowed
_qemu_img_wrapper create -f qcow2 -b TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.overlay
== Closing an image should unlock it ==
+{"return": ""}
_qemu_io_wrapper TEST_DIR/t.qcow2 -c write 0 512
Adding two and closing one
+{"return": "OKrn"}
+{"return": "OKrn"}
_qemu_img_wrapper info TEST_DIR/t.qcow2
+{"return": ""}
_qemu_io_wrapper TEST_DIR/t.qcow2 -c write 0 512
can't open device TEST_DIR/t.qcow2: Failed to get "write" lock
Is another process using the image [TEST_DIR/t.qcow2]?
Closing the other
+{"return": ""}
_qemu_io_wrapper TEST_DIR/t.qcow2 -c write 0 512
From e60483f2f8498ae08ae79ca4c6fb03a3317f5e1e Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Wed, 13 Mar 2019 09:43:30 +0100
Subject: [PATCH 07/12] vl: Fix to create migration object before block
backends again
Recent commit cda4aa9a5a0 moved block backend creation before machine
property evaluation. This broke qemu-iotests 055. Turns out we need
to create the migration object before block backends, so block
backends can add migration blockers. Fix by calling
migration_object_init() earlier, right before configure_blockdev().
Fixes: cda4aa9a5a08777cf13e164c0543bd4888b8adce
Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
vl.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/vl.c b/vl.c
index c1d5484e12..d61d5604e5 100644
--- a/vl.c
+++ b/vl.c
@@ -4276,10 +4276,17 @@ int main(int argc, char **argv, char **envp)
exit(0);
}
+ /*
+ * Migration object can only be created after global properties
+ * are applied correctly.
+ */
+ migration_object_init();
+
/*
* Note: we need to create block backends before
* machine_set_property(), so machine properties can refer to
- * them.
+ * them, and after migration_object_init(), so we can create
+ * migration blockers.
*/
configure_blockdev(&bdo_queue, machine_class, snapshot);
@@ -4297,12 +4304,6 @@ int main(int argc, char **argv, char **envp)
machine_class->name, machine_class->deprecation_reason);
}
- /*
- * Migration object can only be created after global properties
- * are applied correctly.
- */
- migration_object_init();
-
if (qtest_chrdev) {
qtest_init(qtest_chrdev, qtest_log, &error_fatal);
}
From 27e42789b7769ee9556767627542cf8fd9bfbc4d Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Mon, 18 Mar 2019 17:42:37 +0100
Subject: [PATCH 08/12] qemu-iotests: Fix 232 for non-qcow2
232 is marked as generic, but commit 12efe428c9e added code that assumes
qcow2. What the new test really needs is backing files and support for
updating the backing file link (.bdrv_change_backing_file).
Split the non-generic code into a new test case 247 and make it work
with qed, too.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/232 | 30 ---------------
tests/qemu-iotests/232.out | 20 ----------
tests/qemu-iotests/247 | 79 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/247.out | 22 +++++++++++
tests/qemu-iotests/group | 1 +
5 files changed, 102 insertions(+), 50 deletions(-)
create mode 100755 tests/qemu-iotests/247
create mode 100644 tests/qemu-iotests/247.out
diff --git a/tests/qemu-iotests/232 b/tests/qemu-iotests/232
index 0de097fc88..2063f78876 100755
--- a/tests/qemu-iotests/232
+++ b/tests/qemu-iotests/232
@@ -144,36 +144,6 @@ run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,a
run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,auto-read-only=on
run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0
-echo
-echo "=== Try commit to backing file with auto-read-only ==="
-echo
-
-TEST_IMG="$TEST_IMG.0" _make_test_img $size
-TEST_IMG="$TEST_IMG.1" _make_test_img $size
-TEST_IMG="$TEST_IMG.2" _make_test_img $size
-TEST_IMG="$TEST_IMG.3" _make_test_img $size
-TEST_IMG="$TEST_IMG.4" _make_test_img $size
-
-(cat <<EOF
-{"execute":"qmp_capabilities"}
-{"execute":"block-commit",
- "arguments":{"device":"format-4", "top-node": "format-2", "base-node":"format-0", "job-id":"job0"}}
-EOF
-sleep 1
-echo '{"execute":"quit"}'
-) | $QEMU -qmp stdio -nographic -nodefaults \
- -blockdev file,node-name=file-0,filename=$TEST_IMG.0,auto-read-only=on \
- -blockdev qcow2,node-name=format-0,file=file-0,read-only=on \
- -blockdev file,node-name=file-1,filename=$TEST_IMG.1,auto-read-only=on \
- -blockdev qcow2,node-name=format-1,file=file-1,read-only=on,backing=format-0 \
- -blockdev file,node-name=file-2,filename=$TEST_IMG.2,auto-read-only=on \
- -blockdev qcow2,node-name=format-2,file=file-2,read-only=on,backing=format-1 \
- -blockdev file,node-name=file-3,filename=$TEST_IMG.3,auto-read-only=on \
- -blockdev qcow2,node-name=format-3,file=file-3,read-only=on,backing=format-2 \
- -blockdev file,node-name=file-4,filename=$TEST_IMG.4,auto-read-only=on \
- -blockdev qcow2,node-name=format-4,file=file-4,read-only=on,backing=format-3 |
- _filter_qmp
-
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/232.out b/tests/qemu-iotests/232.out
index 5bcc44bb62..3bd1a920af 100644
--- a/tests/qemu-iotests/232.out
+++ b/tests/qemu-iotests/232.out
@@ -56,24 +56,4 @@ QEMU_PROG: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0,read
QEMU_PROG: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0,auto-read-only=off: Could not open 'TEST_DIR/t.IMGFMT': Permission denied
node0: TEST_DIR/t.IMGFMT (file)
QEMU_PROG: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0: Could not open 'TEST_DIR/t.IMGFMT': Permission denied
-
-=== Try commit to backing file with auto-read-only ===
-
-Formatting 'TEST_DIR/t.IMGFMT.0', fmt=IMGFMT size=134217728
-Formatting 'TEST_DIR/t.IMGFMT.1', fmt=IMGFMT size=134217728
-Formatting 'TEST_DIR/t.IMGFMT.2', fmt=IMGFMT size=134217728
-Formatting 'TEST_DIR/t.IMGFMT.3', fmt=IMGFMT size=134217728
-Formatting 'TEST_DIR/t.IMGFMT.4', fmt=IMGFMT size=134217728
-QMP_VERSION
-{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "job0"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "job0"}}
-{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "job0"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "job0"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 134217728, "offset": 134217728, "speed": 0, "type": "commit"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "job0"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "job0"}}
-{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
*** done
diff --git a/tests/qemu-iotests/247 b/tests/qemu-iotests/247
new file mode 100755
index 0000000000..fc50eb5dc1
--- /dev/null
+++ b/tests/qemu-iotests/247
@@ -0,0 +1,79 @@
+#!/usr/bin/env bash
+#
+# Test for auto-read-only with commit block job
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=kwolf@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+ rm -f $TEST_IMG.[01234]
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# Requires backing files and .bdrv_change_backing_file support
+_supported_fmt qcow2 qed
+_supported_proto file
+_supported_os Linux
+
+size=128M
+
+echo
+echo "=== Try commit to backing file with auto-read-only ==="
+echo
+TEST_IMG="$TEST_IMG.0" _make_test_img $size
+TEST_IMG="$TEST_IMG.1" _make_test_img $size
+TEST_IMG="$TEST_IMG.2" _make_test_img $size
+TEST_IMG="$TEST_IMG.3" _make_test_img $size
+TEST_IMG="$TEST_IMG.4" _make_test_img $size
+
+(cat <<EOF
+{"execute":"qmp_capabilities"}
+{"execute":"block-commit",
+ "arguments":{"device":"format-4", "top-node": "format-2", "base-node":"format-0", "job-id":"job0"}}
+EOF
+sleep 1
+echo '{"execute":"quit"}'
+) | $QEMU -qmp stdio -nographic -nodefaults \
+ -blockdev file,node-name=file-0,filename=$TEST_IMG.0,auto-read-only=on \
+ -blockdev $IMGFMT,node-name=format-0,file=file-0,read-only=on \
+ -blockdev file,node-name=file-1,filename=$TEST_IMG.1,auto-read-only=on \
+ -blockdev $IMGFMT,node-name=format-1,file=file-1,read-only=on,backing=format-0 \
+ -blockdev file,node-name=file-2,filename=$TEST_IMG.2,auto-read-only=on \
+ -blockdev $IMGFMT,node-name=format-2,file=file-2,read-only=on,backing=format-1 \
+ -blockdev file,node-name=file-3,filename=$TEST_IMG.3,auto-read-only=on \
+ -blockdev $IMGFMT,node-name=format-3,file=file-3,read-only=on,backing=format-2 \
+ -blockdev file,node-name=file-4,filename=$TEST_IMG.4,auto-read-only=on \
+ -blockdev $IMGFMT,node-name=format-4,file=file-4,read-only=on,backing=format-3 |
+ _filter_qmp
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/247.out b/tests/qemu-iotests/247.out
new file mode 100644
index 0000000000..e909e83994
--- /dev/null
+++ b/tests/qemu-iotests/247.out
@@ -0,0 +1,22 @@
+QA output created by 247
+
+=== Try commit to backing file with auto-read-only ===
+
+Formatting 'TEST_DIR/t.IMGFMT.0', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT.1', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT.2', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT.3', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT.4', fmt=IMGFMT size=134217728
+QMP_VERSION
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "job0"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "job0"}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "job0"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "job0"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 134217728, "offset": 134217728, "speed": 0, "type": "commit"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "job0"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "job0"}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 7289309604..d192abaecf 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -245,3 +245,4 @@
244 rw auto quick
245 rw auto
246 rw auto quick
+247 rw auto quick
From 8d9648cbf3ea60aad05fb10485ae6d2ff6ae587c Mon Sep 17 00:00:00 2001
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Date: Tue, 19 Mar 2019 12:24:42 +0300
Subject: [PATCH 09/12] blockjob: fix user pause in block_job_error_action
Job (especially mirror) may call block_job_error_action several
times before actual pause if it has several in-flight requests.
block_job_error_action will call job_pause more than once in this case,
which lead to following block-job-resume qmp command can't actually
resume the job.
Fix it by do not increase pause level in block_job_error_action if
user_paused already set.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockjob.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/blockjob.c b/blockjob.c
index 58de8cb024..730101d282 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -501,9 +501,11 @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err,
action);
}
if (action == BLOCK_ERROR_ACTION_STOP) {
- job_pause(&job->job);
- /* make the pause user visible, which will be resumed from QMP. */
- job->job.user_paused = true;
+ if (!job->job.user_paused) {
+ job_pause(&job->job);
+ /* make the pause user visible, which will be resumed from QMP. */
+ job->job.user_paused = true;
+ }
block_job_iostatus_set_err(job, error);
}
return action;
From 782b9d06bf9853678fe3225426659539d0bdf668 Mon Sep 17 00:00:00 2001
From: Alberto Garcia <berto@igalia.com>
Date: Mon, 18 Mar 2019 17:48:01 +0200
Subject: [PATCH 10/12] block: Make bdrv_{copy_on_read,crypto_luks,replication}
static
Signed-off-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/copy-on-read.c | 2 +-
block/crypto.c | 2 +-
block/replication.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 64dcc424b5..d670fec42b 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -134,7 +134,7 @@ static bool cor_recurse_is_first_non_filter(BlockDriverState *bs,
}
-BlockDriver bdrv_copy_on_read = {
+static BlockDriver bdrv_copy_on_read = {
.format_name = "copy-on-read",
.bdrv_open = cor_open,
diff --git a/block/crypto.c b/block/crypto.c
index fd8c7cfac6..3af46b805f 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -625,7 +625,7 @@ static const char *const block_crypto_strong_runtime_opts[] = {
NULL
};
-BlockDriver bdrv_crypto_luks = {
+static BlockDriver bdrv_crypto_luks = {
.format_name = "luks",
.instance_size = sizeof(BlockCrypto),
.bdrv_probe = block_crypto_probe_luks,
diff --git a/block/replication.c b/block/replication.c
index b95bd28802..3d4dedddfc 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -682,7 +682,7 @@ static const char *const replication_strong_runtime_opts[] = {
NULL
};
-BlockDriver bdrv_replication = {
+static BlockDriver bdrv_replication = {
.format_name = "replication",
.instance_size = sizeof(BDRVReplicationState),
From 74ce9e466ae5632889d3187382764fc3b55fcc91 Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Wed, 13 Feb 2019 23:53:01 +0100
Subject: [PATCH 11/12] blockdev: Check @replaces in blockdev_mirror_common
There is no reason why the constraints we put on @replaces should be
limited to drive-mirror. Therefore, move the sanity checks from
qmp_drive_mirror() to blockdev_mirror_common() so they apply to
blockdev-mirror as well.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 55 ++++++++++++++++++++++++++++++++----------------------
1 file changed, 33 insertions(+), 22 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 53df2eb875..4775a07d93 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3756,6 +3756,39 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs,
sync = MIRROR_SYNC_MODE_FULL;
}
+ if (has_replaces) {
+ BlockDriverState *to_replace_bs;
+ AioContext *replace_aio_context;
+ int64_t bs_size, replace_size;
+
+ bs_size = bdrv_getlength(bs);
+ if (bs_size < 0) {
+ error_setg_errno(errp, -bs_size, "Failed to query device's size");
+ return;
+ }
+
+ to_replace_bs = check_to_replace_node(bs, replaces, errp);
+ if (!to_replace_bs) {
+ return;
+ }
+
+ replace_aio_context = bdrv_get_aio_context(to_replace_bs);
+ aio_context_acquire(replace_aio_context);
+ replace_size = bdrv_getlength(to_replace_bs);
+ aio_context_release(replace_aio_context);
+
+ if (replace_size < 0) {
+ error_setg_errno(errp, -replace_size,
+ "Failed to query the replacement node's size");
+ return;
+ }
+ if (bs_size != replace_size) {
+ error_setg(errp, "cannot replace image with a mirror image of "
+ "different size");
+ return;
+ }
+ }
+
/* pass the node name to replace to mirror start since it's loose coupling
* and will allow to check whether the node still exist at mirror completion
*/
@@ -3816,33 +3849,11 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
}
if (arg->has_replaces) {
- BlockDriverState *to_replace_bs;
- AioContext *replace_aio_context;
- int64_t replace_size;
-
if (!arg->has_node_name) {
error_setg(errp, "a node-name must be provided when replacing a"
" named node of the graph");
goto out;
}
-
- to_replace_bs = check_to_replace_node(bs, arg->replaces, &local_err);
-
- if (!to_replace_bs) {
- error_propagate(errp, local_err);
- goto out;
- }
-
- replace_aio_context = bdrv_get_aio_context(to_replace_bs);
- aio_context_acquire(replace_aio_context);
- replace_size = bdrv_getlength(to_replace_bs);
- aio_context_release(replace_aio_context);
-
- if (size != replace_size) {
- error_setg(errp, "cannot replace image with a mirror image of "
- "different size");
- goto out;
- }
}
if (arg->mode == NEW_IMAGE_MODE_ABSOLUTE_PATHS) {
From 59fba0aaee7438002d9803a86c888f21a1070cc8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Doktor?= <ldoktor@redhat.com>
Date: Tue, 19 Mar 2019 15:20:49 +0100
Subject: [PATCH 12/12] qemu-iotests: Treat custom TEST_DIR in 051
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When custom TEST_DIR is specified the output includes it without leading
'/':
$ TEST_DIR=/var/tmp ./check -file -qcow2 051
....
-drive0 (NODE_NAME): json:{"backing": {"driver": "qcow2", "file":
{"driver": "file", "filename": "TEST_DIR/t.qcow2"}}, "driver": "qcow2",
"file": {"driver": "file", "filename": SNAPSHOT_PATH}} (qcow2,
read-only)
+drive0 (NODE_NAME): json:{"backing": {"driver": "qcow2", "file":
{"driver": "file", "filename": "TEST_DIR/t.qcow2"}}, "driver": "qcow2",
"file": {"driver": "file", "filename": "TEST_DIR/vl.ziHfeP"}} (qcow2,
read-only)
Let's remove it from the sed regexp.
Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/051 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 6a3b7c2b89..02ac960da4 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -360,7 +360,7 @@ TMPDIR=/nonexistent run_qemu -drive driver=null-co,snapshot=on
echo "info block" |
run_qemu -drive file="$TEST_IMG",snapshot=on,read-only=on,if=none,id=$device_id |
_filter_qemu_io |
- sed -e 's#"/[^"]*/vl\.[A-Za-z0-9]\{6\}"#SNAPSHOT_PATH#g'
+ sed -e 's#"[^"]*/vl\.[A-Za-z0-9]\{6\}"#SNAPSHOT_PATH#g'
# success, all done