[PULL 0/7] Patches for 5.2.0-rc2

[PULL 0/7] Patches for 5.2.0-rc2

[Kevin Wolf]

The following changes since commit bce36c6c5436c0c2679b42c6de54de5393dfcf59:

  Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2020-11-16 22:29:51 +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 c61c644f59c43017f9577d7f867c48bb9d7a28ad:

  iotests/081: Test rewrite-corrupted without WRITE (2020-11-17 12:38:32 +0100)

----------------------------------------------------------------
Patches for 5.2.0-rc2:

- quorum: Fix crash with rewrite-corrupted and without read-write user
- io_uring: do not use pointer after free
- file-posix: Use fallback path for -EBUSY from FALLOC_FL_PUNCH_HOLE
- iotests: Fix failure on Python 3.9 due to use of a deprecated function
- char-stdio: Fix QMP default for 'signal'

----------------------------------------------------------------
Kevin Wolf (2):
      char-stdio: Fix QMP default for 'signal'
      iotests: Replace deprecated ConfigParser.readfp()

Max Reitz (3):
      quorum: Require WRITE perm with rewrite-corrupted
      iotests/081: Filter image format after testdir
      iotests/081: Test rewrite-corrupted without WRITE

Maxim Levitsky (1):
      file-posix: allow -EBUSY errors during write zeros on raw block devices

Paolo Bonzini (1):
      io_uring: do not use pointer after free

 qapi/char.json                           |  3 +-
 block/file-posix.c                       |  1 +
 block/io_uring.c                         |  2 +-
 block/quorum.c                           |  5 +++
 chardev/char-stdio.c                     |  4 +--
 tests/qemu-iotests/nbd-fault-injector.py |  2 +-
 tests/qemu-iotests/081                   | 61 ++++++++++++++++++++++++++++++--
 tests/qemu-iotests/081.out               | 27 ++++++++++++++
 8 files changed, 95 insertions(+), 10 deletions(-)

[PULL 1/7] char-stdio: Fix QMP default for 'signal'

[Kevin Wolf]

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 <kwolf@redhat.com>
Message-Id: <20201023101222.250147-2-kwolf@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/char.json       | 3 +--
 chardev/char-stdio.c | 4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

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
 ##
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
-- 
2.28.0

[PULL 2/7] iotests: Replace deprecated ConfigParser.readfp()

[Kevin Wolf]

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 <kwolf@redhat.com>
Message-Id: <20201113100602.15936-1-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 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):
-- 
2.28.0

[PULL 3/7] file-posix: allow -EBUSY errors during write zeros on raw block devices

[Kevin Wolf]

From: Maxim Levitsky <mlevitsk@redhat.com>

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 <mlevitsk@redhat.com>
Message-Id: <20201111153913.41840-2-mlevitsk@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 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;
-- 
2.28.0

[PULL 4/7] io_uring: do not use pointer after free

[Kevin Wolf]

From: Paolo Bonzini <pbonzini@redhat.com>

Even though only the pointer value is only printed, it is untidy
and Coverity complains.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201113154102.1460459-1-pbonzini@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 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);
 }
-- 
2.28.0

[PULL 5/7] quorum: Require WRITE perm with rewrite-corrupted

[Kevin Wolf]

From: Max Reitz <mreitz@redhat.com>

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 <mreitz@redhat.com>
Message-Id: <20201113211718.261671-2-mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 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
-- 
2.28.0

[PULL 6/7] iotests/081: Filter image format after testdir

[Kevin Wolf]

From: Max Reitz <mreitz@redhat.com>

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 <mreitz@redhat.com>
Message-Id: <20201113211718.261671-3-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 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"
-- 
2.28.0

[PULL 7/7] iotests/081: Test rewrite-corrupted without WRITE

[Kevin Wolf]

From: Max Reitz <mreitz@redhat.com>

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 <mreitz@redhat.com>
Message-Id: <20201113211718.261671-4-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 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 \
+    <<EOF
+{ "execute": "qmp_capabilities" }
+{
+    "execute": "human-monitor-command",
+    "arguments": {
+        "command-line": 'qemu-io -d quorum-drive "read -P 0x32 0 $size"'
+    }
+}
+{ "execute": "quit" }
+EOF
+
+echo '-- checking that the image has been corrected --'
+$QEMU_IO -c "read -P 0x32 0 $size" "$TEST_DIR/2.raw" | _filter_qemu_io
+
 echo
 echo "== breaking quorum =="
 
diff --git a/tests/qemu-iotests/081.out b/tests/qemu-iotests/081.out
index 04091b64e5..1974262fac 100644
--- a/tests/qemu-iotests/081.out
+++ b/tests/qemu-iotests/081.out
@@ -47,6 +47,33 @@ read 10485760/10485760 bytes at offset 0
 read 10485760/10485760 bytes at offset 0
 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
+== using quorum rewrite corrupted mode without WRITE permission ==
+-- corrupting --
+wrote 1048576/1048576 bytes at offset 1048576
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+-- running quorum --
+Testing: -blockdev file,node-name=file1,filename=TEST_DIR/1.IMGFMT -blockdev file,node-name=file2,filename=TEST_DIR/2.IMGFMT -blockdev file,node-name=file3,filename=TEST_DIR/3.IMGFMT -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
+QMP_VERSION
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "QUORUM_REPORT_BAD", "data": {"node-name": "file2", "sectors-count": 20480, "sector-num": 0, "type": "read"}}
+read 10485760/10485760 bytes at offset 0
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": ""}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
+
+-- checking that the image has been corrected --
+read 10485760/10485760 bytes at offset 0
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
 == breaking quorum ==
 wrote 10485760/10485760 bytes at offset 0
 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-- 
2.28.0

Re: [PULL 0/7] Patches for 5.2.0-rc2

[Peter Maydell]

On Tue, 17 Nov 2020 at 12:16, Kevin Wolf <kwolf@redhat.com> wrote:
>
> The following changes since commit bce36c6c5436c0c2679b42c6de54de5393dfcf59:
>
>   Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2020-11-16 22:29:51 +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 c61c644f59c43017f9577d7f867c48bb9d7a28ad:
>
>   iotests/081: Test rewrite-corrupted without WRITE (2020-11-17 12:38:32 +0100)
>
> ----------------------------------------------------------------
> Patches for 5.2.0-rc2:
>
> - quorum: Fix crash with rewrite-corrupted and without read-write user
> - io_uring: do not use pointer after free
> - file-posix: Use fallback path for -EBUSY from FALLOC_FL_PUNCH_HOLE
> - iotests: Fix failure on Python 3.9 due to use of a deprecated function
> - char-stdio: Fix QMP default for 'signal'
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM