1
The following changes since commit f6b06fcceef465de0cf2514c9f76fe0192896781:
1
The following changes since commit 29e0855c5af62bbb0b0b6fed792e004dad92ba95:
2
2
3
Merge remote-tracking branch 'remotes/kraxel/tags/ui-20190121-pull-request' into staging (2019-01-23 17:57:47 +0000)
3
Merge remote-tracking branch 'remotes/elmarco/tags/slirp-pull-request' into staging (2020-03-22 21:00:38 +0000)
4
4
5
are available in the Git repository at:
5
are available in the Git repository at:
6
6
7
git://github.com/stefanha/qemu.git tags/block-pull-request
7
https://github.com/stefanha/qemu.git tags/block-pull-request
8
8
9
for you to fetch changes up to 8595685986152334b1ec28c78cb0e5e855d56b54:
9
for you to fetch changes up to ff807d559205a434fd37d3343f01a0ab128bd190:
10
10
11
qemu-coroutine-sleep: drop CoSleepCB (2019-01-24 10:05:16 +0000)
11
aio-posix: fix io_uring with external events (2020-03-23 11:05:44 +0000)
12
12
13
----------------------------------------------------------------
13
----------------------------------------------------------------
14
Pull request
14
Pull request
15
15
16
Changelog: No user-visible changes.
17
18
----------------------------------------------------------------
16
----------------------------------------------------------------
19
17
20
Stefan Hajnoczi (2):
18
Stefan Hajnoczi (1):
21
throttle-groups: fix restart coroutine iothread race
19
aio-posix: fix io_uring with external events
22
iotests: add 238 for throttling tgm unregister iothread segfault
23
20
24
Vladimir Sementsov-Ogievskiy (1):
21
util/fdmon-io_uring.c | 13 ++++++++++++-
25
qemu-coroutine-sleep: drop CoSleepCB
22
1 file changed, 12 insertions(+), 1 deletion(-)
26
27
include/block/throttle-groups.h | 5 ++++
28
block/throttle-groups.c | 9 +++++++
29
util/qemu-coroutine-sleep.c | 27 +++++++------------
30
tests/qemu-iotests/238 | 47 +++++++++++++++++++++++++++++++++
31
tests/qemu-iotests/238.out | 6 +++++
32
tests/qemu-iotests/group | 1 +
33
6 files changed, 78 insertions(+), 17 deletions(-)
34
create mode 100755 tests/qemu-iotests/238
35
create mode 100644 tests/qemu-iotests/238.out
36
23
37
--
24
--
38
2.20.1
25
2.24.1
39
26
40
diff view generated by jsdifflib
Deleted patch
1
The following QMP command leads to a crash when iothreads are used:
2
1
3
{ 'execute': 'device_del', 'arguments': {'id': 'data'} }
4
5
The backtrace involves the queue restart coroutine where
6
tgm->throttle_state is a NULL pointer because
7
throttle_group_unregister_tgm() has already been called:
8
9
(gdb) bt full
10
#0 0x00005585a7a3b378 in qemu_mutex_lock_impl (mutex=0xffffffffffffffd0, file=0x5585a7bb3d54 "block/throttle-groups.c", line=412) at util/qemu-thread-posix.c:64
11
err = <optimized out>
12
__PRETTY_FUNCTION__ = "qemu_mutex_lock_impl"
13
__func__ = "qemu_mutex_lock_impl"
14
#1 0x00005585a79be074 in throttle_group_restart_queue_entry (opaque=0x5585a9de4eb0) at block/throttle-groups.c:412
15
_f = <optimized out>
16
data = 0x5585a9de4eb0
17
tgm = 0x5585a9079440
18
ts = 0x0
19
tg = 0xffffffffffffff98
20
is_write = false
21
empty_queue = 255
22
23
This coroutine should not execute in the iothread after the throttle
24
group member has been unregistered!
25
26
The root cause is that the device_del code path schedules the restart
27
coroutine in the iothread while holding the AioContext lock. Therefore
28
the iothread cannot execute the coroutine until after device_del
29
releases the lock - by this time it's too late.
30
31
This patch adds a reference count to ThrottleGroupMember so we can
32
synchronously wait for restart coroutines to complete. Once they are
33
done it is safe to unregister the ThrottleGroupMember.
34
35
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
36
Reviewed-by: Alberto Garcia <berto@igalia.com>
37
Message-id: 20190114133257.30299-2-stefanha@redhat.com
38
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
39
---
40
include/block/throttle-groups.h | 5 +++++
41
block/throttle-groups.c | 9 +++++++++
42
2 files changed, 14 insertions(+)
43
44
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
45
index XXXXXXX..XXXXXXX 100644
46
--- a/include/block/throttle-groups.h
47
+++ b/include/block/throttle-groups.h
48
@@ -XXX,XX +XXX,XX @@ typedef struct ThrottleGroupMember {
49
*/
50
unsigned int io_limits_disabled;
51
52
+ /* Number of pending throttle_group_restart_queue_entry() coroutines.
53
+ * Accessed with atomic operations.
54
+ */
55
+ unsigned int restart_pending;
56
+
57
/* The following fields are protected by the ThrottleGroup lock.
58
* See the ThrottleGroup documentation for details.
59
* throttle_state tells us if I/O limits are configured. */
60
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
61
index XXXXXXX..XXXXXXX 100644
62
--- a/block/throttle-groups.c
63
+++ b/block/throttle-groups.c
64
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn throttle_group_restart_queue_entry(void *opaque)
65
}
66
67
g_free(data);
68
+
69
+ atomic_dec(&tgm->restart_pending);
70
+ aio_wait_kick();
71
}
72
73
static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write)
74
@@ -XXX,XX +XXX,XX @@ static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write
75
* be no timer pending on this tgm at this point */
76
assert(!timer_pending(tgm->throttle_timers.timers[is_write]));
77
78
+ atomic_inc(&tgm->restart_pending);
79
+
80
co = qemu_coroutine_create(throttle_group_restart_queue_entry, rd);
81
aio_co_enter(tgm->aio_context, co);
82
}
83
@@ -XXX,XX +XXX,XX @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm,
84
85
tgm->throttle_state = ts;
86
tgm->aio_context = ctx;
87
+ atomic_set(&tgm->restart_pending, 0);
88
89
qemu_mutex_lock(&tg->lock);
90
/* If the ThrottleGroup is new set this ThrottleGroupMember as the token */
91
@@ -XXX,XX +XXX,XX @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm)
92
return;
93
}
94
95
+ /* Wait for throttle_group_restart_queue_entry() coroutines to finish */
96
+ AIO_WAIT_WHILE(tgm->aio_context, atomic_read(&tgm->restart_pending) > 0);
97
+
98
qemu_mutex_lock(&tg->lock);
99
for (i = 0; i < 2; i++) {
100
assert(tgm->pending_reqs[i] == 0);
101
--
102
2.20.1
103
104
diff view generated by jsdifflib
Deleted patch
1
Hot-unplug a scsi-hd using an iothread. The previous patch fixes a
2
segfault in this scenario.
3
1
4
This patch adds a regression test.
5
6
Suggested-by: Alberto Garcia <berto@igalia.com>
7
Suggested-by: Kevin Wolf <kwolf@redhat.com>
8
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
9
Reviewed-by: Alberto Garcia <berto@igalia.com>
10
Message-id: 20190114133257.30299-3-stefanha@redhat.com
11
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
12
---
13
tests/qemu-iotests/238 | 47 ++++++++++++++++++++++++++++++++++++++
14
tests/qemu-iotests/238.out | 6 +++++
15
tests/qemu-iotests/group | 1 +
16
3 files changed, 54 insertions(+)
17
create mode 100755 tests/qemu-iotests/238
18
create mode 100644 tests/qemu-iotests/238.out
19
20
diff --git a/tests/qemu-iotests/238 b/tests/qemu-iotests/238
21
new file mode 100755
22
index XXXXXXX..XXXXXXX
23
--- /dev/null
24
+++ b/tests/qemu-iotests/238
25
@@ -XXX,XX +XXX,XX @@
26
+#!/usr/bin/env python
27
+#
28
+# Regression test for throttle group member unregister segfault with iothread
29
+#
30
+# Copyright (c) 2019 Red Hat, Inc.
31
+#
32
+# This program is free software; you can redistribute it and/or modify
33
+# it under the terms of the GNU General Public License as published by
34
+# the Free Software Foundation; either version 2 of the License, or
35
+# (at your option) any later version.
36
+#
37
+# This program is distributed in the hope that it will be useful,
38
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
39
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
40
+# GNU General Public License for more details.
41
+#
42
+# You should have received a copy of the GNU General Public License
43
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
44
+#
45
+
46
+import sys
47
+import os
48
+import iotests
49
+from iotests import log
50
+
51
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'scripts'))
52
+
53
+from qemu import QEMUMachine
54
+
55
+if iotests.qemu_default_machine == 's390-ccw-virtio':
56
+ virtio_scsi_device = 'virtio-scsi-ccw'
57
+else:
58
+ virtio_scsi_device = 'virtio-scsi-pci'
59
+
60
+vm = QEMUMachine(iotests.qemu_prog)
61
+vm.add_args('-machine', 'accel=kvm')
62
+vm.launch()
63
+
64
+log(vm.qmp('blockdev-add', node_name='hd0', driver='null-co'))
65
+log(vm.qmp('object-add', qom_type='iothread', id='iothread0'))
66
+log(vm.qmp('device_add', id='scsi0', driver=virtio_scsi_device, iothread='iothread0'))
67
+log(vm.qmp('device_add', id='scsi-hd0', driver='scsi-hd', drive='hd0'))
68
+log(vm.qmp('block_set_io_throttle', id='scsi-hd0', bps=0, bps_rd=0, bps_wr=0,
69
+ iops=1000, iops_rd=0, iops_wr=0, conv_keys=False))
70
+log(vm.qmp('device_del', id='scsi-hd0'))
71
+
72
+vm.shutdown()
73
diff --git a/tests/qemu-iotests/238.out b/tests/qemu-iotests/238.out
74
new file mode 100644
75
index XXXXXXX..XXXXXXX
76
--- /dev/null
77
+++ b/tests/qemu-iotests/238.out
78
@@ -XXX,XX +XXX,XX @@
79
+{"return": {}}
80
+{"return": {}}
81
+{"return": {}}
82
+{"return": {}}
83
+{"return": {}}
84
+{"return": {}}
85
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
86
index XXXXXXX..XXXXXXX 100644
87
--- a/tests/qemu-iotests/group
88
+++ b/tests/qemu-iotests/group
89
@@ -XXX,XX +XXX,XX @@
90
234 auto quick migration
91
235 auto quick
92
236 auto quick
93
+238 auto quick
94
--
95
2.20.1
96
97
diff view generated by jsdifflib
1
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
1
When external event sources are disabled fdmon-io_uring falls back to
2
fdmon-poll. The ->need_wait() callback needs to watch for this so it
3
can return true when external event sources are disabled.
2
4
3
Drop CoSleepCB structure. It's actually unused.
5
It is also necessary to call ->wait() when AioHandlers have changed
6
because io_uring is asynchronous and we must submit new sqes.
4
7
5
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
8
Both of these changes to ->need_wait() together fix tests/test-aio -p
6
Message-id: 20190122143113.20331-1-vsementsov@virtuozzo.com
9
/aio/external-client, which failed with:
10
11
test-aio: tests/test-aio.c:404: test_aio_external_client: Assertion `aio_poll(ctx, false)' failed.
12
13
Reported-by: Julia Suvorova <jusual@redhat.com>
14
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
15
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
16
Message-id: 20200319163559.117903-1-stefanha@redhat.com
7
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
17
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
8
---
18
---
9
util/qemu-coroutine-sleep.c | 27 ++++++++++-----------------
19
util/fdmon-io_uring.c | 13 ++++++++++++-
10
1 file changed, 10 insertions(+), 17 deletions(-)
20
1 file changed, 12 insertions(+), 1 deletion(-)
11
21
12
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
22
diff --git a/util/fdmon-io_uring.c b/util/fdmon-io_uring.c
13
index XXXXXXX..XXXXXXX 100644
23
index XXXXXXX..XXXXXXX 100644
14
--- a/util/qemu-coroutine-sleep.c
24
--- a/util/fdmon-io_uring.c
15
+++ b/util/qemu-coroutine-sleep.c
25
+++ b/util/fdmon-io_uring.c
16
@@ -XXX,XX +XXX,XX @@
26
@@ -XXX,XX +XXX,XX @@ static int fdmon_io_uring_wait(AioContext *ctx, AioHandlerList *ready_list,
17
#include "qemu/timer.h"
27
18
#include "block/aio.h"
28
static bool fdmon_io_uring_need_wait(AioContext *ctx)
19
20
-typedef struct CoSleepCB {
21
- QEMUTimer *ts;
22
- Coroutine *co;
23
-} CoSleepCB;
24
-
25
static void co_sleep_cb(void *opaque)
26
{
29
{
27
- CoSleepCB *sleep_cb = opaque;
30
- return io_uring_cq_ready(&ctx->fdmon_io_uring);
28
+ Coroutine *co = opaque;
31
+ /* Have io_uring events completed? */
29
32
+ if (io_uring_cq_ready(&ctx->fdmon_io_uring)) {
30
/* Write of schedule protected by barrier write in aio_co_schedule */
33
+ return true;
31
- atomic_set(&sleep_cb->co->scheduled, NULL);
34
+ }
32
- aio_co_wake(sleep_cb->co);
35
+
33
+ atomic_set(&co->scheduled, NULL);
36
+ /* Do we need to submit new io_uring sqes? */
34
+ aio_co_wake(co);
37
+ if (!QSLIST_EMPTY_RCU(&ctx->submit_list)) {
38
+ return true;
39
+ }
40
+
41
+ /* Are we falling back to fdmon-poll? */
42
+ return atomic_read(&ctx->external_disable_cnt);
35
}
43
}
36
44
37
void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
45
static const FDMonOps fdmon_io_uring_ops = {
38
{
39
AioContext *ctx = qemu_get_current_aio_context();
40
- CoSleepCB sleep_cb = {
41
- .co = qemu_coroutine_self(),
42
- };
43
+ QEMUTimer *ts;
44
+ Coroutine *co = qemu_coroutine_self();
45
46
- const char *scheduled = atomic_cmpxchg(&sleep_cb.co->scheduled, NULL,
47
- __func__);
48
+ const char *scheduled = atomic_cmpxchg(&co->scheduled, NULL, __func__);
49
if (scheduled) {
50
fprintf(stderr,
51
"%s: Co-routine was already scheduled in '%s'\n",
52
__func__, scheduled);
53
abort();
54
}
55
- sleep_cb.ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, &sleep_cb);
56
- timer_mod(sleep_cb.ts, qemu_clock_get_ns(type) + ns);
57
+ ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, co);
58
+ timer_mod(ts, qemu_clock_get_ns(type) + ns);
59
qemu_coroutine_yield();
60
- timer_del(sleep_cb.ts);
61
- timer_free(sleep_cb.ts);
62
+ timer_del(ts);
63
+ timer_free(ts);
64
}
65
--
46
--
66
2.20.1
47
2.24.1
67
48
68
diff view generated by jsdifflib