From 6deb20f6683662a3ff5b5248dea2fd4d1055a628 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 23 Oct 2020 12:12:17 +0200 Subject: [PATCH 1/7] char-stdio: Fix QMP default for 'signal' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 02c4bdf1 tried to make signal=on the default for stdio chardevs except for '-serial mon:stdio', but it forgot about QMP and accidentally switched the QMP default from true (except for -nographic) to false (always). The documentation was kept unchanged and still describes the opposite of the old behaviour (which is an even older documentation bug). Fix all of this by making signal=true the default in ChardevStdio and documenting it as such. Fixes: 02c4bdf1d2ca8c02a9bae16398f260b5c08d08bf Signed-off-by: Kevin Wolf Message-Id: <20201023101222.250147-2-kwolf@redhat.com> Reviewed-by: Marc-André Lureau Reviewed-by: Markus Armbruster Signed-off-by: Kevin Wolf --- chardev/char-stdio.c | 4 +--- qapi/char.json | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/chardev/char-stdio.c b/chardev/char-stdio.c index 82eaebc1db..403da308c9 100644 --- a/chardev/char-stdio.c +++ b/chardev/char-stdio.c @@ -112,9 +112,7 @@ static void qemu_chr_open_stdio(Chardev *chr, qemu_chr_open_fd(chr, 0, 1); - if (opts->has_signal) { - stdio_allow_signal = opts->signal; - } + stdio_allow_signal = !opts->has_signal || opts->signal; qemu_chr_set_echo_stdio(chr, false); } #endif diff --git a/qapi/char.json b/qapi/char.json index b4d66ec90b..43486d1daa 100644 --- a/qapi/char.json +++ b/qapi/char.json @@ -321,8 +321,7 @@ # Configuration info for stdio chardevs. # # @signal: Allow signals (such as SIGINT triggered by ^C) -# be delivered to qemu. Default: true in -nographic mode, -# false otherwise. +# be delivered to qemu. Default: true. # # Since: 1.5 ## From 5aaabf9161c501142558f1837f6f90c94ee15081 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 13 Nov 2020 11:06:02 +0100 Subject: [PATCH 2/7] iotests: Replace deprecated ConfigParser.readfp() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit iotest 277 fails on Fedora 33 (Python 3.9) because a deprecation warning changes the output: nbd-fault-injector.py:230: DeprecationWarning: This method will be removed in future versions. Use 'parser.read_file()' instead. In fact, readfp() has already been deprecated in Python 3.2 and the replacement has existed since the same version, so we can now unconditionally switch to read_file(). Signed-off-by: Kevin Wolf Message-Id: <20201113100602.15936-1-kwolf@redhat.com> Reviewed-by: Eric Blake Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Kevin Wolf --- tests/qemu-iotests/nbd-fault-injector.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py index 78f42c4214..6e11ef89b8 100755 --- a/tests/qemu-iotests/nbd-fault-injector.py +++ b/tests/qemu-iotests/nbd-fault-injector.py @@ -227,7 +227,7 @@ def parse_config(config): def load_rules(filename): config = configparser.RawConfigParser() with open(filename, 'rt') as f: - config.readfp(f, filename) + config.read_file(f, filename) return parse_config(config) def open_socket(path): From ece4fa9152ea37c7ebd158af330e3b20e33cf385 Mon Sep 17 00:00:00 2001 From: Maxim Levitsky Date: Wed, 11 Nov 2020 17:39:12 +0200 Subject: [PATCH 3/7] file-posix: allow -EBUSY errors during write zeros on raw block devices On Linux, fallocate(fd, FALLOC_FL_PUNCH_HOLE) when it is used on a block device, without O_DIRECT can return -EBUSY if it races with another write to the same page. Since this is rare and discard is not a critical operation, ignore this error Signed-off-by: Maxim Levitsky Message-Id: <20201111153913.41840-2-mlevitsk@redhat.com> Signed-off-by: Kevin Wolf --- block/file-posix.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/file-posix.c b/block/file-posix.c index c63926d592..d5fd1dbcd2 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -1698,6 +1698,7 @@ static int handle_aiocb_write_zeroes_unmap(void *opaque) switch (ret) { case -ENOTSUP: case -EINVAL: + case -EBUSY: break; default: return ret; From bd89f93603f4e588e80f2e622118bf4131f30865 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 13 Nov 2020 10:41:02 -0500 Subject: [PATCH 4/7] io_uring: do not use pointer after free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even though only the pointer value is only printed, it is untidy and Coverity complains. Cc: Stefan Hajnoczi Signed-off-by: Paolo Bonzini Message-Id: <20201113154102.1460459-1-pbonzini@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Kevin Wolf --- block/io_uring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/io_uring.c b/block/io_uring.c index 037af09471..00a3ee9fb8 100644 --- a/block/io_uring.c +++ b/block/io_uring.c @@ -425,6 +425,6 @@ LuringState *luring_init(Error **errp) void luring_cleanup(LuringState *s) { io_uring_queue_exit(&s->ring); - g_free(s); trace_luring_cleanup_state(s); + g_free(s); } From 9ca5b0e8427789438f5f8e8b6cc76bc8eb76c941 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 13 Nov 2020 22:17:16 +0100 Subject: [PATCH 5/7] quorum: Require WRITE perm with rewrite-corrupted Using rewrite-corrupted means quorum may issue writes to its children just from receiving read requests from its parents. Thus, it must take the WRITE permission when rewrite-corrupted is used. Signed-off-by: Max Reitz Message-Id: <20201113211718.261671-2-mreitz@redhat.com> Reviewed-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block/quorum.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/block/quorum.c b/block/quorum.c index e846a7e892..b10fc2089e 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -1163,7 +1163,12 @@ static void quorum_child_perm(BlockDriverState *bs, BdrvChild *c, uint64_t perm, uint64_t shared, uint64_t *nperm, uint64_t *nshared) { + BDRVQuorumState *s = bs->opaque; + *nperm = perm & DEFAULT_PERM_PASSTHROUGH; + if (s->rewrite_corrupted) { + *nperm |= BLK_PERM_WRITE; + } /* * We cannot share RESIZE or WRITE, as this would make the From 55f2c014d7a4f79bc22dc7f018ef785b5d6baf49 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 13 Nov 2020 22:17:17 +0100 Subject: [PATCH 6/7] iotests/081: Filter image format after testdir Otherwise, this breaks whenever the test directory contains the image format (e.g. "/tmp/test-raw-file" is filtered to "/tmp/test-IMGFMT-file" instead of "TEST_DIR"). Signed-off-by: Max Reitz Message-Id: <20201113211718.261671-3-mreitz@redhat.com> Signed-off-by: Kevin Wolf --- tests/qemu-iotests/081 | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081 index 537d40dfd5..253b7576be 100755 --- a/tests/qemu-iotests/081 +++ b/tests/qemu-iotests/081 @@ -45,15 +45,16 @@ _require_drivers quorum do_run_qemu() { - echo Testing: "$@" | _filter_imgfmt + echo Testing: "$@" $QEMU -nographic -qmp stdio -serial none "$@" echo } run_qemu() { - do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_qmp\ - | _filter_qemu_io | _filter_generated_node_ids + do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_imgfmt | _filter_qemu \ + | _filter_qmp | _filter_qemu_io \ + | _filter_generated_node_ids } quorum="driver=raw,file.driver=quorum,file.vote-threshold=2" From c61c644f59c43017f9577d7f867c48bb9d7a28ad Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 13 Nov 2020 22:17:18 +0100 Subject: [PATCH 7/7] iotests/081: Test rewrite-corrupted without WRITE Test what happens when a rewrite-corrupted quorum node performs such a rewrite, while there is no parent that has taken the WRITE permission. Signed-off-by: Max Reitz Message-Id: <20201113211718.261671-4-mreitz@redhat.com> Signed-off-by: Kevin Wolf --- tests/qemu-iotests/081 | 54 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/081.out | 27 +++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081 index 253b7576be..4e19972931 100755 --- a/tests/qemu-iotests/081 +++ b/tests/qemu-iotests/081 @@ -42,6 +42,7 @@ _supported_fmt raw _supported_proto file _supported_os Linux _require_drivers quorum +_require_devices virtio-scsi do_run_qemu() { @@ -155,6 +156,59 @@ echo "== checking that quorum has corrected the corrupted file ==" $QEMU_IO -c "read -P 0x32 0 $size" "$TEST_DIR/2.raw" | _filter_qemu_io +echo +echo "== using quorum rewrite corrupted mode without WRITE permission ==" + +# The same as above, but this time, do it on a quorum node whose only +# parent will not take the WRITE permission + +echo '-- corrupting --' +# Only corrupt a portion: The guest device (scsi-hd on virtio-scsi) +# will read some data (looking for a partition table to guess the +# disk's geometry), which would trigger a quorum mismatch if the +# beginning of the image was corrupted. The subsequent +# QUORUM_REPORT_BAD event would be suppressed (because at that point, +# there cannot have been a qmp_capabilities on the monitor). Because +# that event is rate-limited, the next QUORUM_REPORT_BAD that happens +# thanks to our qemu-io read (which should trigger a mismatch) would +# then be delayed past the VM quit and not appear in the output. +# So we keep the first 1M intact to see a QUORUM_REPORT_BAD resulting +# from the qemu-io invocation. +$QEMU_IO -c "write -P 0x42 1M 1M" "$TEST_DIR/2.raw" | _filter_qemu_io + +# Fix the corruption (on a read-only quorum node, i.e. without taking +# the WRITE permission on it -- its child nodes need to be R/W OTOH, +# so that rewrite-corrupted works) +echo +echo '-- running quorum --' +run_qemu \ + -blockdev file,node-name=file1,filename="$TEST_DIR/1.raw" \ + -blockdev file,node-name=file2,filename="$TEST_DIR/2.raw" \ + -blockdev file,node-name=file3,filename="$TEST_DIR/3.raw" \ + -blockdev '{ + "driver": "quorum", + "node-name": "quorum", + "read-only": true, + "vote-threshold": 2, + "rewrite-corrupted": true, + "children": [ "file1", "file2", "file3" ] + }' \ + -device virtio-scsi,id=scsi \ + -device scsi-hd,id=quorum-drive,bus=scsi.0,drive=quorum \ + <