:p
atchew
Login
The following changes since commit b98a66201dbc7cf3b962f4bb260f66100cc75578: Merge remote-tracking branch 'remotes/palmer/tags/riscv-for-master-4.0-rc0-2' into staging (2019-03-19 12:55:02 +0000) are available in the Git repository at: git://repo.or.cz/qemu/kevin.git tags/for-upstream for you to fetch changes up to 59fba0aaee7438002d9803a86c888f21a1070cc8: qemu-iotests: Treat custom TEST_DIR in 051 (2019-03-19 15:51:31 +0100) ---------------------------------------------------------------- Block layer patches: - mirror: Fix early return from drain (could cause deadlocks) - vmdk: Fixed probing for version 3 images - vl: Fix to create migration object before block backends again (fixes segfault for block drivers that set migration blockers) - Several minor fixes, documentation and test case improvements ---------------------------------------------------------------- Alberto Garcia (1): block: Make bdrv_{copy_on_read,crypto_luks,replication} static Kevin Wolf (3): qcow2: Fix data file error condition in qcow2_co_create() block: Silence Coverity in bdrv_drop_intermediate() qemu-iotests: Fix 232 for non-qcow2 Lukáš Doktor (1): qemu-iotests: Treat custom TEST_DIR in 051 Markus Armbruster (1): vl: Fix to create migration object before block backends again Max Reitz (1): blockdev: Check @replaces in blockdev_mirror_common Sam Eiderman (1): vmdk: Support version=3 in VMDK descriptor files Sergio Lopez (2): mirror: Confirm we're quiesced only if the job is paused or cancelled iotests: 153: Wait for an answer to QMP commands Vladimir Sementsov-Ogievskiy (2): qapi: fix block-latency-histogram-set description and examples blockjob: fix user pause in block_job_error_action qapi/block-core.json | 10 +++--- block.c | 7 ++-- block/copy-on-read.c | 2 +- block/crypto.c | 2 +- block/mirror.c | 16 ++++++++++ block/qcow2.c | 2 +- block/replication.c | 2 +- block/vmdk.c | 6 ++-- blockdev.c | 55 +++++++++++++++++++------------- blockjob.c | 8 +++-- vl.c | 15 +++++---- tests/qemu-iotests/051 | 2 +- tests/qemu-iotests/153 | 12 +++---- tests/qemu-iotests/153.out | 6 ++++ 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 + 19 files changed, 193 insertions(+), 104 deletions(-) create mode 100755 tests/qemu-iotests/247 create mode 100644 tests/qemu-iotests/247.out
From: Sergio Lopez <slp@redhat.com> 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 XXXXXXX..XXXXXXX 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -XXX,XX +XXX,XX @@ typedef struct MirrorBlockJob { bool initial_zeroing_ongoing; int in_active_write_counter; bool prepared; + bool in_drain; } MirrorBlockJob; typedef struct MirrorBDSOpaque { @@ -XXX,XX +XXX,XX @@ 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); @@ -XXX,XX +XXX,XX @@ 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); @@ -XXX,XX +XXX,XX @@ 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; } @@ -XXX,XX +XXX,XX @@ immediate_exit: bdrv_dirty_iter_free(s->dbi); if (need_drain) { + s->in_drain = true; bdrv_drained_begin(bs); } @@ -XXX,XX +XXX,XX @@ 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; } -- 2.20.1
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 XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ 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; } -- 2.20.1
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> 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 XXXXXXX..XXXXXXX 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -XXX,XX +XXX,XX @@ # # 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. @@ -XXX,XX +XXX,XX @@ # [0, 10), [10, 50), [50, 100), [100, +inf): # # -> { "execute": "block-latency-histogram-set", -# "arguments": { "device": "drive0", +# "arguments": { "id": "drive0", # "boundaries": [10, 50, 100] } } # <- { "return": {} } # @@ -XXX,XX +XXX,XX @@ # not changed (or not created): # # -> { "execute": "block-latency-histogram-set", -# "arguments": { "device": "drive0", +# "arguments": { "id": "drive0", # "boundaries-write": [10, 50, 100] } } # <- { "return": {} } # @@ -XXX,XX +XXX,XX @@ # 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": {} } @@ -XXX,XX +XXX,XX @@ # Example: remove all latency histograms: # # -> { "execute": "block-latency-histogram-set", -# "arguments": { "device": "drive0" } } +# "arguments": { "id": "drive0" } } # <- { "return": {} } ## { 'command': 'block-latency-histogram-set', -- 2.20.1
From: Sam Eiderman <shmuel.eiderman@oracle.com> 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 XXXXXXX..XXXXXXX 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -XXX,XX +XXX,XX @@ 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; } } -- 2.20.1
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 XXXXXXX..XXXXXXX 100644 --- a/block.c +++ b/block.c @@ -XXX,XX +XXX,XX @@ 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; } -- 2.20.1
From: Sergio Lopez <slp@redhat.com> 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 XXXXXXX..XXXXXXX 100755 --- a/tests/qemu-iotests/153 +++ b/tests/qemu-iotests/153 @@ -XXX,XX +XXX,XX @@ 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 @@ -XXX,XX +XXX,XX @@ 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' @@ -XXX,XX +XXX,XX @@ 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' @@ -XXX,XX +XXX,XX @@ 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}" @@ -XXX,XX +XXX,XX @@ _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' @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/153.out +++ b/tests/qemu-iotests/153.out @@ -XXX,XX +XXX,XX @@ 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 @@ -XXX,XX +XXX,XX @@ 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 -- 2.20.1
From: Markus Armbruster <armbru@redhat.com> 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 XXXXXXX..XXXXXXX 100644 --- a/vl.c +++ b/vl.c @@ -XXX,XX +XXX,XX @@ 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); @@ -XXX,XX +XXX,XX @@ 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); } -- 2.20.1
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 XXXXXXX..XXXXXXX 100755 --- a/tests/qemu-iotests/232 +++ b/tests/qemu-iotests/232 @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/232.out +++ b/tests/qemu-iotests/232.out @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/qemu-iotests/247 @@ -XXX,XX +XXX,XX @@ +#!/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 XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/qemu-iotests/247.out @@ -XXX,XX +XXX,XX @@ +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 XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -XXX,XX +XXX,XX @@ 244 rw auto quick 245 rw auto 246 rw auto quick +247 rw auto quick -- 2.20.1
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> 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 XXXXXXX..XXXXXXX 100644 --- a/blockjob.c +++ b/blockjob.c @@ -XXX,XX +XXX,XX @@ 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; -- 2.20.1
From: Alberto Garcia <berto@igalia.com> 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 XXXXXXX..XXXXXXX 100644 --- a/block/copy-on-read.c +++ b/block/copy-on-read.c @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/block/replication.c +++ b/block/replication.c @@ -XXX,XX +XXX,XX @@ static const char *const replication_strong_runtime_opts[] = { NULL }; -BlockDriver bdrv_replication = { +static BlockDriver bdrv_replication = { .format_name = "replication", .instance_size = sizeof(BDRVReplicationState), -- 2.20.1
From: Max Reitz <mreitz@redhat.com> 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 XXXXXXX..XXXXXXX 100644 --- a/blockdev.c +++ b/blockdev.c @@ -XXX,XX +XXX,XX @@ 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 */ @@ -XXX,XX +XXX,XX @@ 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) { -- 2.20.1
From: Lukáš Doktor <ldoktor@redhat.com> 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 XXXXXXX..XXXXXXX 100755 --- a/tests/qemu-iotests/051 +++ b/tests/qemu-iotests/051 @@ -XXX,XX +XXX,XX @@ 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 -- 2.20.1
The following changes since commit 13356edb87506c148b163b8c7eb0695647d00c2a: Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2023-01-24 09:45:33 +0000) are available in the Git repository at: https://repo.or.cz/qemu/kevin.git tags/for-upstream for you to fetch changes up to d570177b50c389f379f93183155a27d44856ab46: qemu-img: Change info key names for protocol nodes (2023-02-01 16:52:33 +0100) v4: - Fixed the 'qemu-img-close-errors' test case to run only on Linux and only with the file protocol, use qemu-io instead of truncate v3: - Make the compiler happier on BSD and CentOS Stream 8 v2: - Rebased to resolve merge conflicts in coroutine.h ---------------------------------------------------------------- Block layer patches - qemu-img info: Show protocol-level information - Move more functions to coroutines - Make coroutine annotations ready for static analysis - qemu-img: Fix exit code for errors closing the image - qcow2 bitmaps: Fix theoretical corruption in error path - pflash: Only load non-zero parts of backend image to save memory - Code cleanup and test case improvements ---------------------------------------------------------------- Alberto Faria (2): coroutine: annotate coroutine_fn for libclang block: Add no_coroutine_fn and coroutine_mixed_fn marker Emanuele Giuseppe Esposito (14): block-coroutine-wrapper: support void functions block: Convert bdrv_io_plug() to co_wrapper block: Convert bdrv_io_unplug() to co_wrapper block: Convert bdrv_is_inserted() to co_wrapper block: Rename refresh_total_sectors to bdrv_refresh_total_sectors block: Convert bdrv_refresh_total_sectors() to co_wrapper_mixed block-backend: use bdrv_getlength instead of blk_getlength block: use bdrv_co_refresh_total_sectors when possible block: Convert bdrv_get_allocated_file_size() to co_wrapper block: Convert bdrv_get_info() to co_wrapper_mixed block: Convert bdrv_eject() to co_wrapper block: Convert bdrv_lock_medium() to co_wrapper block: Convert bdrv_debug_event() to co_wrapper_mixed block: Rename bdrv_load/save_vmstate() to bdrv_co_load/save_vmstate() Hanna Reitz (12): block: Improve empty format-specific info dump block/file: Add file-specific image info block/vmdk: Change extent info type block: Split BlockNodeInfo off of ImageInfo qemu-img: Use BlockNodeInfo block/qapi: Let bdrv_query_image_info() recurse block/qapi: Introduce BlockGraphInfo block/qapi: Add indentation to bdrv_node_info_dump() iotests: Filter child node information iotests/106, 214, 308: Read only one size line qemu-img: Let info print block graph qemu-img: Change info key names for protocol nodes Kevin Wolf (4): qcow2: Fix theoretical corruption in store_bitmap() error path qemu-img commit: Report errors while closing the image qemu-img bitmap: Report errors while closing the image qemu-iotests: Test qemu-img bitmap/commit exit code on error Paolo Bonzini (2): qemu-io: do not reinvent the blk_pwrite_zeroes wheel block: remove bdrv_coroutine_enter Philippe Mathieu-Daudé (1): block/nbd: Add missing <qemu/bswap.h> include Thomas Huth (2): tests/qemu-iotests/312: Mark "quorum" as required driver tests/qemu-iotests/262: Check for availability of "blkverify" first Xiang Zheng (1): pflash: Only read non-zero parts of backend image qapi/block-core.json | 123 +++++++- include/block/block-common.h | 11 +- include/block/block-io.h | 41 ++- include/block/block_int-common.h | 26 +- include/block/block_int-io.h | 5 +- include/block/nbd.h | 1 + include/block/qapi.h | 14 +- include/qemu/osdep.h | 44 +++ include/sysemu/block-backend-io.h | 31 +- block.c | 88 +++--- block/blkdebug.c | 11 +- block/blkio.c | 15 +- block/blklogwrites.c | 6 +- block/blkreplay.c | 6 +- block/blkverify.c | 6 +- block/block-backend.c | 38 +-- block/commit.c | 4 +- block/copy-on-read.c | 18 +- block/crypto.c | 14 +- block/curl.c | 10 +- block/file-posix.c | 137 +++++---- block/file-win32.c | 18 +- block/filter-compress.c | 20 +- block/gluster.c | 23 +- block/io.c | 76 ++--- block/iscsi.c | 17 +- block/mirror.c | 6 +- block/monitor/block-hmp-cmds.c | 2 +- block/nbd.c | 8 +- block/nfs.c | 4 +- block/null.c | 13 +- block/nvme.c | 14 +- block/preallocate.c | 16 +- block/qapi.c | 317 ++++++++++++++++----- block/qcow.c | 5 +- block/qcow2-bitmap.c | 5 +- block/qcow2-refcount.c | 2 +- block/qcow2.c | 17 +- block/qed.c | 11 +- block/quorum.c | 8 +- block/raw-format.c | 25 +- block/rbd.c | 9 +- block/replication.c | 6 +- block/ssh.c | 4 +- block/throttle.c | 6 +- block/vdi.c | 7 +- block/vhdx.c | 5 +- block/vmdk.c | 22 +- block/vpc.c | 5 +- blockdev.c | 8 +- hw/block/block.c | 36 ++- hw/scsi/scsi-disk.c | 5 + qemu-img.c | 100 +++++-- qemu-io-cmds.c | 62 +--- tests/unit/test-block-iothread.c | 3 + scripts/block-coroutine-wrapper.py | 20 +- tests/qemu-iotests/iotests.py | 18 +- block/meson.build | 1 + tests/qemu-iotests/065 | 2 +- tests/qemu-iotests/106 | 4 +- tests/qemu-iotests/214 | 6 +- tests/qemu-iotests/262 | 3 +- tests/qemu-iotests/302.out | 5 + tests/qemu-iotests/308 | 4 +- tests/qemu-iotests/312 | 1 + tests/qemu-iotests/common.filter | 22 +- tests/qemu-iotests/common.rc | 22 +- tests/qemu-iotests/tests/qemu-img-close-errors | 96 +++++++ tests/qemu-iotests/tests/qemu-img-close-errors.out | 23 ++ 69 files changed, 1209 insertions(+), 552 deletions(-) create mode 100755 tests/qemu-iotests/tests/qemu-img-close-errors create mode 100644 tests/qemu-iotests/tests/qemu-img-close-errors.out