1
The following changes since commit e4ae62b802cec437f877f2cadc4ef059cc0eca76:
1
The following changes since commit 0ab4537f08e09b13788db67efd760592fb7db769:
2
2
3
Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2018-03-09 17:28:16 +0000)
3
Merge remote-tracking branch 'remotes/stefanberger/tags/pull-tpm-2018-03-07-1' into staging (2018-03-08 12:56:39 +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
git://github.com/stefanha/qemu.git tags/block-pull-request
8
8
9
for you to fetch changes up to 7376eda7c2e0451e819e81bd05fabc56a9deb946:
9
for you to fetch changes up to 4486e89c219c0d1b9bd8dfa0b1dd5b0d51ff2268:
10
10
11
block: make BDRV_POLL_WHILE() re-entrancy safe (2018-03-12 11:07:37 +0000)
11
vl: introduce vm_shutdown() (2018-03-08 17:38:51 +0000)
12
12
13
----------------------------------------------------------------
13
----------------------------------------------------------------
14
14
15
----------------------------------------------------------------
15
----------------------------------------------------------------
16
16
17
Stefan Hajnoczi (1):
17
Deepa Srinivasan (1):
18
block: make BDRV_POLL_WHILE() re-entrancy safe
18
block: Fix qemu crash when using scsi-block
19
19
20
include/block/aio-wait.h | 61 ++++++++++++++++++++++++------------------------
20
Fam Zheng (1):
21
util/aio-wait.c | 2 +-
21
README: Fix typo 'git-publish'
22
2 files changed, 31 insertions(+), 32 deletions(-)
22
23
Sergio Lopez (1):
24
virtio-blk: dataplane: Don't batch notifications if EVENT_IDX is
25
present
26
27
Stefan Hajnoczi (4):
28
block: add aio_wait_bh_oneshot()
29
virtio-blk: fix race between .ioeventfd_stop() and vq handler
30
virtio-scsi: fix race between .ioeventfd_stop() and vq handler
31
vl: introduce vm_shutdown()
32
33
include/block/aio-wait.h | 13 +++++++++++
34
include/sysemu/iothread.h | 1 -
35
include/sysemu/sysemu.h | 1 +
36
block/block-backend.c | 51 ++++++++++++++++++++---------------------
37
cpus.c | 16 ++++++++++---
38
hw/block/dataplane/virtio-blk.c | 39 +++++++++++++++++++++++--------
39
hw/scsi/virtio-scsi-dataplane.c | 9 ++++----
40
iothread.c | 31 -------------------------
41
util/aio-wait.c | 31 +++++++++++++++++++++++++
42
vl.c | 13 +++--------
43
README | 2 +-
44
11 files changed, 122 insertions(+), 85 deletions(-)
23
45
24
--
46
--
25
2.14.3
47
2.14.3
26
48
27
49
diff view generated by jsdifflib
New patch
1
From: Deepa Srinivasan <deepa.srinivasan@oracle.com>
1
2
3
Starting qemu with the following arguments causes qemu to segfault:
4
... -device lsi,id=lsi0 -drive file=iscsi:<...>,format=raw,if=none,node-name=
5
iscsi1 -device scsi-block,bus=lsi0.0,id=<...>,drive=iscsi1
6
7
This patch fixes blk_aio_ioctl() so it does not pass stack addresses to
8
blk_aio_ioctl_entry() which may be invoked after blk_aio_ioctl() returns. More
9
details about the bug follow.
10
11
blk_aio_ioctl() invokes blk_aio_prwv() with blk_aio_ioctl_entry as the
12
coroutine parameter. blk_aio_prwv() ultimately calls aio_co_enter().
13
14
When blk_aio_ioctl() is executed from within a coroutine context (e.g.
15
iscsi_bh_cb()), aio_co_enter() adds the coroutine (blk_aio_ioctl_entry) to
16
the current coroutine's wakeup queue. blk_aio_ioctl() then returns.
17
18
When blk_aio_ioctl_entry() executes later, it accesses an invalid pointer:
19
....
20
BlkRwCo *rwco = &acb->rwco;
21
22
rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,
23
rwco->qiov->iov[0].iov_base); <--- qiov is
24
invalid here
25
...
26
27
In the case when blk_aio_ioctl() is called from a non-coroutine context,
28
blk_aio_ioctl_entry() executes immediately. But if bdrv_co_ioctl() calls
29
qemu_coroutine_yield(), blk_aio_ioctl() will return. When the coroutine
30
execution is complete, control returns to blk_aio_ioctl_entry() after the call
31
to blk_co_ioctl(). There is no invalid reference after this point, but the
32
function is still holding on to invalid pointers.
33
34
The fix is to change blk_aio_prwv() to accept a void pointer for the IO buffer
35
rather than a QEMUIOVector. blk_aio_prwv() passes this through in BlkRwCo and the
36
coroutine function casts it to QEMUIOVector or uses the void pointer directly.
37
38
Signed-off-by: Deepa Srinivasan <deepa.srinivasan@oracle.com>
39
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
40
Reviewed-by: Mark Kanda <mark.kanda@oracle.com>
41
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
42
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
43
---
44
block/block-backend.c | 51 +++++++++++++++++++++++++--------------------------
45
1 file changed, 25 insertions(+), 26 deletions(-)
46
47
diff --git a/block/block-backend.c b/block/block-backend.c
48
index XXXXXXX..XXXXXXX 100644
49
--- a/block/block-backend.c
50
+++ b/block/block-backend.c
51
@@ -XXX,XX +XXX,XX @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
52
typedef struct BlkRwCo {
53
BlockBackend *blk;
54
int64_t offset;
55
- QEMUIOVector *qiov;
56
+ void *iobuf;
57
int ret;
58
BdrvRequestFlags flags;
59
} BlkRwCo;
60
@@ -XXX,XX +XXX,XX @@ typedef struct BlkRwCo {
61
static void blk_read_entry(void *opaque)
62
{
63
BlkRwCo *rwco = opaque;
64
+ QEMUIOVector *qiov = rwco->iobuf;
65
66
- rwco->ret = blk_co_preadv(rwco->blk, rwco->offset, rwco->qiov->size,
67
- rwco->qiov, rwco->flags);
68
+ rwco->ret = blk_co_preadv(rwco->blk, rwco->offset, qiov->size,
69
+ qiov, rwco->flags);
70
}
71
72
static void blk_write_entry(void *opaque)
73
{
74
BlkRwCo *rwco = opaque;
75
+ QEMUIOVector *qiov = rwco->iobuf;
76
77
- rwco->ret = blk_co_pwritev(rwco->blk, rwco->offset, rwco->qiov->size,
78
- rwco->qiov, rwco->flags);
79
+ rwco->ret = blk_co_pwritev(rwco->blk, rwco->offset, qiov->size,
80
+ qiov, rwco->flags);
81
}
82
83
static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,
84
@@ -XXX,XX +XXX,XX @@ static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,
85
rwco = (BlkRwCo) {
86
.blk = blk,
87
.offset = offset,
88
- .qiov = &qiov,
89
+ .iobuf = &qiov,
90
.flags = flags,
91
.ret = NOT_DONE,
92
};
93
@@ -XXX,XX +XXX,XX @@ static void blk_aio_complete_bh(void *opaque)
94
}
95
96
static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes,
97
- QEMUIOVector *qiov, CoroutineEntry co_entry,
98
+ void *iobuf, CoroutineEntry co_entry,
99
BdrvRequestFlags flags,
100
BlockCompletionFunc *cb, void *opaque)
101
{
102
@@ -XXX,XX +XXX,XX @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes,
103
acb->rwco = (BlkRwCo) {
104
.blk = blk,
105
.offset = offset,
106
- .qiov = qiov,
107
+ .iobuf = iobuf,
108
.flags = flags,
109
.ret = NOT_DONE,
110
};
111
@@ -XXX,XX +XXX,XX @@ static void blk_aio_read_entry(void *opaque)
112
{
113
BlkAioEmAIOCB *acb = opaque;
114
BlkRwCo *rwco = &acb->rwco;
115
+ QEMUIOVector *qiov = rwco->iobuf;
116
117
- assert(rwco->qiov->size == acb->bytes);
118
+ assert(qiov->size == acb->bytes);
119
rwco->ret = blk_co_preadv(rwco->blk, rwco->offset, acb->bytes,
120
- rwco->qiov, rwco->flags);
121
+ qiov, rwco->flags);
122
blk_aio_complete(acb);
123
}
124
125
@@ -XXX,XX +XXX,XX @@ static void blk_aio_write_entry(void *opaque)
126
{
127
BlkAioEmAIOCB *acb = opaque;
128
BlkRwCo *rwco = &acb->rwco;
129
+ QEMUIOVector *qiov = rwco->iobuf;
130
131
- assert(!rwco->qiov || rwco->qiov->size == acb->bytes);
132
+ assert(!qiov || qiov->size == acb->bytes);
133
rwco->ret = blk_co_pwritev(rwco->blk, rwco->offset, acb->bytes,
134
- rwco->qiov, rwco->flags);
135
+ qiov, rwco->flags);
136
blk_aio_complete(acb);
137
}
138
139
@@ -XXX,XX +XXX,XX @@ int blk_co_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
140
static void blk_ioctl_entry(void *opaque)
141
{
142
BlkRwCo *rwco = opaque;
143
+ QEMUIOVector *qiov = rwco->iobuf;
144
+
145
rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,
146
- rwco->qiov->iov[0].iov_base);
147
+ qiov->iov[0].iov_base);
148
}
149
150
int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
151
@@ -XXX,XX +XXX,XX @@ static void blk_aio_ioctl_entry(void *opaque)
152
BlkAioEmAIOCB *acb = opaque;
153
BlkRwCo *rwco = &acb->rwco;
154
155
- rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,
156
- rwco->qiov->iov[0].iov_base);
157
+ rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset, rwco->iobuf);
158
+
159
blk_aio_complete(acb);
160
}
161
162
BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
163
BlockCompletionFunc *cb, void *opaque)
164
{
165
- QEMUIOVector qiov;
166
- struct iovec iov;
167
-
168
- iov = (struct iovec) {
169
- .iov_base = buf,
170
- .iov_len = 0,
171
- };
172
- qemu_iovec_init_external(&qiov, &iov, 1);
173
-
174
- return blk_aio_prwv(blk, req, 0, &qiov, blk_aio_ioctl_entry, 0, cb, opaque);
175
+ return blk_aio_prwv(blk, req, 0, buf, blk_aio_ioctl_entry, 0, cb, opaque);
176
}
177
178
int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
179
@@ -XXX,XX +XXX,XX @@ int blk_truncate(BlockBackend *blk, int64_t offset, PreallocMode prealloc,
180
static void blk_pdiscard_entry(void *opaque)
181
{
182
BlkRwCo *rwco = opaque;
183
- rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, rwco->qiov->size);
184
+ QEMUIOVector *qiov = rwco->iobuf;
185
+
186
+ rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, qiov->size);
187
}
188
189
int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
190
--
191
2.14.3
192
193
diff view generated by jsdifflib
New patch
1
From: Fam Zheng <famz@redhat.com>
1
2
3
Reported-by: Alberto Garcia <berto@igalia.com>
4
Signed-off-by: Fam Zheng <famz@redhat.com>
5
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
6
Message-id: 20180306024328.19195-1-famz@redhat.com
7
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
8
---
9
README | 2 +-
10
1 file changed, 1 insertion(+), 1 deletion(-)
11
12
diff --git a/README b/README
13
index XXXXXXX..XXXXXXX 100644
14
--- a/README
15
+++ b/README
16
@@ -XXX,XX +XXX,XX @@ The QEMU website is also maintained under source control.
17
git clone git://git.qemu.org/qemu-web.git
18
https://www.qemu.org/2017/02/04/the-new-qemu-website-is-up/
19
20
-A 'git-profile' utility was created to make above process less
21
+A 'git-publish' utility was created to make above process less
22
cumbersome, and is highly recommended for making regular contributions,
23
or even just for sending consecutive patch series revisions. It also
24
requires a working 'git send-email' setup, and by default doesn't
25
--
26
2.14.3
27
28
diff view generated by jsdifflib
New patch
1
From: Sergio Lopez <slp@redhat.com>
1
2
3
Commit 5b2ffbe4d99843fd8305c573a100047a8c962327 ("virtio-blk: dataplane:
4
notify guest as a batch") deferred guest notification to a BH in order
5
batch notifications, with purpose of avoiding flooding the guest with
6
interruptions.
7
8
This optimization came with a cost. The average latency perceived in the
9
guest is increased by a few microseconds, but also when multiple IO
10
operations finish at the same time, the guest won't be notified until
11
all completions from each operation has been run. On the contrary,
12
virtio-scsi issues the notification at the end of each completion.
13
14
On the other hand, nowadays we have the EVENT_IDX feature that allows a
15
better coordination between QEMU and the Guest OS to avoid sending
16
unnecessary interruptions.
17
18
With this change, virtio-blk/dataplane only batches notifications if the
19
EVENT_IDX feature is not present.
20
21
Some numbers obtained with fio (ioengine=sync, iodepth=1, direct=1):
22
- Test specs:
23
* fio-3.4 (ioengine=sync, iodepth=1, direct=1)
24
* qemu master
25
* virtio-blk with a dedicated iothread (default poll-max-ns)
26
* backend: null_blk nr_devices=1 irqmode=2 completion_nsec=280000
27
* 8 vCPUs pinned to isolated physical cores
28
* Emulator and iothread also pinned to separate isolated cores
29
* variance between runs < 1%
30
31
- Not patched
32
* numjobs=1: lat_avg=327.32 irqs=29998
33
* numjobs=4: lat_avg=337.89 irqs=29073
34
* numjobs=8: lat_avg=342.98 irqs=28643
35
36
- Patched:
37
* numjobs=1: lat_avg=323.92 irqs=30262
38
* numjobs=4: lat_avg=332.65 irqs=29520
39
* numjobs=8: lat_avg=335.54 irqs=29323
40
41
Signed-off-by: Sergio Lopez <slp@redhat.com>
42
Message-id: 20180307114459.26636-1-slp@redhat.com
43
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
44
---
45
hw/block/dataplane/virtio-blk.c | 15 +++++++++++++--
46
1 file changed, 13 insertions(+), 2 deletions(-)
47
48
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
49
index XXXXXXX..XXXXXXX 100644
50
--- a/hw/block/dataplane/virtio-blk.c
51
+++ b/hw/block/dataplane/virtio-blk.c
52
@@ -XXX,XX +XXX,XX @@ struct VirtIOBlockDataPlane {
53
VirtIODevice *vdev;
54
QEMUBH *bh; /* bh for guest notification */
55
unsigned long *batch_notify_vqs;
56
+ bool batch_notifications;
57
58
/* Note that these EventNotifiers are assigned by value. This is
59
* fine as long as you do not call event_notifier_cleanup on them
60
@@ -XXX,XX +XXX,XX @@ struct VirtIOBlockDataPlane {
61
/* Raise an interrupt to signal guest, if necessary */
62
void virtio_blk_data_plane_notify(VirtIOBlockDataPlane *s, VirtQueue *vq)
63
{
64
- set_bit(virtio_get_queue_index(vq), s->batch_notify_vqs);
65
- qemu_bh_schedule(s->bh);
66
+ if (s->batch_notifications) {
67
+ set_bit(virtio_get_queue_index(vq), s->batch_notify_vqs);
68
+ qemu_bh_schedule(s->bh);
69
+ } else {
70
+ virtio_notify_irqfd(s->vdev, vq);
71
+ }
72
}
73
74
static void notify_guest_bh(void *opaque)
75
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
76
77
s->starting = true;
78
79
+ if (!virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
80
+ s->batch_notifications = true;
81
+ } else {
82
+ s->batch_notifications = false;
83
+ }
84
+
85
/* Set up guest notifier (irq) */
86
r = k->set_guest_notifiers(qbus->parent, nvqs, true);
87
if (r != 0) {
88
--
89
2.14.3
90
91
diff view generated by jsdifflib
1
Nested BDRV_POLL_WHILE() calls can occur. Currently
1
Sometimes it's necessary for the main loop thread to run a BH in an
2
assert(!wait_->wakeup) fails in AIO_WAIT_WHILE() when this happens.
2
IOThread and wait for its completion. This primitive is useful during
3
startup/shutdown to synchronize and avoid race conditions.
3
4
4
This patch converts the bool wait_->need_kick flag to an unsigned
5
wait_->num_waiters counter.
6
7
Nesting works correctly because outer AIO_WAIT_WHILE() callers evaluate
8
the condition again after the inner caller completes (invoking the inner
9
caller counts as aio_poll() progress).
10
11
Reported-by: "fuweiwei (C)" <fuweiwei2@huawei.com>
12
Reviewed-by: Eric Blake <eblake@redhat.com>
13
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
5
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
14
Message-id: 20180307124619.6218-1-stefanha@redhat.com
6
Reviewed-by: Fam Zheng <famz@redhat.com>
15
Cc: Paolo Bonzini <pbonzini@redhat.com>
7
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
8
Message-id: 20180307144205.20619-2-stefanha@redhat.com
16
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
9
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
17
---
10
---
18
include/block/aio-wait.h | 61 ++++++++++++++++++++++++------------------------
11
include/block/aio-wait.h | 13 +++++++++++++
19
util/aio-wait.c | 2 +-
12
util/aio-wait.c | 31 +++++++++++++++++++++++++++++++
20
2 files changed, 31 insertions(+), 32 deletions(-)
13
2 files changed, 44 insertions(+)
21
14
22
diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h
15
diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h
23
index XXXXXXX..XXXXXXX 100644
16
index XXXXXXX..XXXXXXX 100644
24
--- a/include/block/aio-wait.h
17
--- a/include/block/aio-wait.h
25
+++ b/include/block/aio-wait.h
18
+++ b/include/block/aio-wait.h
26
@@ -XXX,XX +XXX,XX @@
19
@@ -XXX,XX +XXX,XX @@ typedef struct {
27
* }
28
*/
20
*/
29
typedef struct {
21
void aio_wait_kick(AioWait *wait);
30
- /* Is the main loop waiting for a kick? Accessed with atomic ops. */
22
31
- bool need_kick;
23
+/**
32
+ /* Number of waiting AIO_WAIT_WHILE() callers. Accessed with atomic ops. */
24
+ * aio_wait_bh_oneshot:
33
+ unsigned num_waiters;
25
+ * @ctx: the aio context
34
} AioWait;
26
+ * @cb: the BH callback function
35
27
+ * @opaque: user data for the BH callback function
36
/**
28
+ *
37
@@ -XXX,XX +XXX,XX @@ typedef struct {
29
+ * Run a BH in @ctx and wait for it to complete.
38
* wait on conditions between two IOThreads since that could lead to deadlock,
30
+ *
39
* go via the main loop instead.
31
+ * Must be called from the main loop thread with @ctx acquired exactly once.
40
*/
32
+ * Note that main loop event processing may occur.
41
-#define AIO_WAIT_WHILE(wait, ctx, cond) ({ \
33
+ */
42
- bool waited_ = false; \
34
+void aio_wait_bh_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque);
43
- bool busy_ = true; \
35
+
44
- AioWait *wait_ = (wait); \
36
#endif /* QEMU_AIO_WAIT */
45
- AioContext *ctx_ = (ctx); \
46
- if (in_aio_context_home_thread(ctx_)) { \
47
- while ((cond) || busy_) { \
48
- busy_ = aio_poll(ctx_, (cond)); \
49
- waited_ |= !!(cond) | busy_; \
50
- } \
51
- } else { \
52
- assert(qemu_get_current_aio_context() == \
53
- qemu_get_aio_context()); \
54
- assert(!wait_->need_kick); \
55
- /* Set wait_->need_kick before evaluating cond. */ \
56
- atomic_mb_set(&wait_->need_kick, true); \
57
- while (busy_) { \
58
- if ((cond)) { \
59
- waited_ = busy_ = true; \
60
- aio_context_release(ctx_); \
61
- aio_poll(qemu_get_aio_context(), true); \
62
- aio_context_acquire(ctx_); \
63
- } else { \
64
- busy_ = aio_poll(ctx_, false); \
65
- waited_ |= busy_; \
66
- } \
67
- } \
68
- atomic_set(&wait_->need_kick, false); \
69
- } \
70
+#define AIO_WAIT_WHILE(wait, ctx, cond) ({ \
71
+ bool waited_ = false; \
72
+ bool busy_ = true; \
73
+ AioWait *wait_ = (wait); \
74
+ AioContext *ctx_ = (ctx); \
75
+ if (in_aio_context_home_thread(ctx_)) { \
76
+ while ((cond) || busy_) { \
77
+ busy_ = aio_poll(ctx_, (cond)); \
78
+ waited_ |= !!(cond) | busy_; \
79
+ } \
80
+ } else { \
81
+ assert(qemu_get_current_aio_context() == \
82
+ qemu_get_aio_context()); \
83
+ /* Increment wait_->num_waiters before evaluating cond. */ \
84
+ atomic_inc(&wait_->num_waiters); \
85
+ while (busy_) { \
86
+ if ((cond)) { \
87
+ waited_ = busy_ = true; \
88
+ aio_context_release(ctx_); \
89
+ aio_poll(qemu_get_aio_context(), true); \
90
+ aio_context_acquire(ctx_); \
91
+ } else { \
92
+ busy_ = aio_poll(ctx_, false); \
93
+ waited_ |= busy_; \
94
+ } \
95
+ } \
96
+ atomic_dec(&wait_->num_waiters); \
97
+ } \
98
waited_; })
99
100
/**
101
diff --git a/util/aio-wait.c b/util/aio-wait.c
37
diff --git a/util/aio-wait.c b/util/aio-wait.c
102
index XXXXXXX..XXXXXXX 100644
38
index XXXXXXX..XXXXXXX 100644
103
--- a/util/aio-wait.c
39
--- a/util/aio-wait.c
104
+++ b/util/aio-wait.c
40
+++ b/util/aio-wait.c
105
@@ -XXX,XX +XXX,XX @@ static void dummy_bh_cb(void *opaque)
41
@@ -XXX,XX +XXX,XX @@ void aio_wait_kick(AioWait *wait)
106
void aio_wait_kick(AioWait *wait)
107
{
108
/* The barrier (or an atomic op) is in the caller. */
109
- if (atomic_read(&wait->need_kick)) {
110
+ if (atomic_read(&wait->num_waiters)) {
111
aio_bh_schedule_oneshot(qemu_get_aio_context(), dummy_bh_cb, NULL);
42
aio_bh_schedule_oneshot(qemu_get_aio_context(), dummy_bh_cb, NULL);
112
}
43
}
113
}
44
}
45
+
46
+typedef struct {
47
+ AioWait wait;
48
+ bool done;
49
+ QEMUBHFunc *cb;
50
+ void *opaque;
51
+} AioWaitBHData;
52
+
53
+/* Context: BH in IOThread */
54
+static void aio_wait_bh(void *opaque)
55
+{
56
+ AioWaitBHData *data = opaque;
57
+
58
+ data->cb(data->opaque);
59
+
60
+ data->done = true;
61
+ aio_wait_kick(&data->wait);
62
+}
63
+
64
+void aio_wait_bh_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
65
+{
66
+ AioWaitBHData data = {
67
+ .cb = cb,
68
+ .opaque = opaque,
69
+ };
70
+
71
+ assert(qemu_get_current_aio_context() == qemu_get_aio_context());
72
+
73
+ aio_bh_schedule_oneshot(ctx, aio_wait_bh, &data);
74
+ AIO_WAIT_WHILE(&data.wait, ctx, !data.done);
75
+}
114
--
76
--
115
2.14.3
77
2.14.3
116
78
117
79
diff view generated by jsdifflib
New patch
1
If the main loop thread invokes .ioeventfd_stop() just as the vq handler
2
function begins in the IOThread then the handler may lose the race for
3
the AioContext lock. By the time the vq handler is able to acquire the
4
AioContext lock the ioeventfd has already been removed and the handler
5
isn't supposed to run anymore!
1
6
7
Use the new aio_wait_bh_oneshot() function to perform ioeventfd removal
8
from within the IOThread. This way no races with the vq handler are
9
possible.
10
11
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
12
Reviewed-by: Fam Zheng <famz@redhat.com>
13
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
14
Message-id: 20180307144205.20619-3-stefanha@redhat.com
15
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
16
---
17
hw/block/dataplane/virtio-blk.c | 24 +++++++++++++++++-------
18
1 file changed, 17 insertions(+), 7 deletions(-)
19
20
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
21
index XXXXXXX..XXXXXXX 100644
22
--- a/hw/block/dataplane/virtio-blk.c
23
+++ b/hw/block/dataplane/virtio-blk.c
24
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
25
return -ENOSYS;
26
}
27
28
+/* Stop notifications for new requests from guest.
29
+ *
30
+ * Context: BH in IOThread
31
+ */
32
+static void virtio_blk_data_plane_stop_bh(void *opaque)
33
+{
34
+ VirtIOBlockDataPlane *s = opaque;
35
+ unsigned i;
36
+
37
+ for (i = 0; i < s->conf->num_queues; i++) {
38
+ VirtQueue *vq = virtio_get_queue(s->vdev, i);
39
+
40
+ virtio_queue_aio_set_host_notifier_handler(vq, s->ctx, NULL);
41
+ }
42
+}
43
+
44
/* Context: QEMU global mutex held */
45
void virtio_blk_data_plane_stop(VirtIODevice *vdev)
46
{
47
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
48
trace_virtio_blk_data_plane_stop(s);
49
50
aio_context_acquire(s->ctx);
51
-
52
- /* Stop notifications for new requests from guest */
53
- for (i = 0; i < nvqs; i++) {
54
- VirtQueue *vq = virtio_get_queue(s->vdev, i);
55
-
56
- virtio_queue_aio_set_host_notifier_handler(vq, s->ctx, NULL);
57
- }
58
+ aio_wait_bh_oneshot(s->ctx, virtio_blk_data_plane_stop_bh, s);
59
60
/* Drain and switch bs back to the QEMU main loop */
61
blk_set_aio_context(s->conf->conf.blk, qemu_get_aio_context());
62
--
63
2.14.3
64
65
diff view generated by jsdifflib
New patch
1
If the main loop thread invokes .ioeventfd_stop() just as the vq handler
2
function begins in the IOThread then the handler may lose the race for
3
the AioContext lock. By the time the vq handler is able to acquire the
4
AioContext lock the ioeventfd has already been removed and the handler
5
isn't supposed to run anymore!
1
6
7
Use the new aio_wait_bh_oneshot() function to perform ioeventfd removal
8
from within the IOThread. This way no races with the vq handler are
9
possible.
10
11
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
12
Reviewed-by: Fam Zheng <famz@redhat.com>
13
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
14
Message-id: 20180307144205.20619-4-stefanha@redhat.com
15
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
16
---
17
hw/scsi/virtio-scsi-dataplane.c | 9 +++++----
18
1 file changed, 5 insertions(+), 4 deletions(-)
19
20
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
21
index XXXXXXX..XXXXXXX 100644
22
--- a/hw/scsi/virtio-scsi-dataplane.c
23
+++ b/hw/scsi/virtio-scsi-dataplane.c
24
@@ -XXX,XX +XXX,XX @@ static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue *vq, int n,
25
return 0;
26
}
27
28
-/* assumes s->ctx held */
29
-static void virtio_scsi_clear_aio(VirtIOSCSI *s)
30
+/* Context: BH in IOThread */
31
+static void virtio_scsi_dataplane_stop_bh(void *opaque)
32
{
33
+ VirtIOSCSI *s = opaque;
34
VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
35
int i;
36
37
@@ -XXX,XX +XXX,XX @@ int virtio_scsi_dataplane_start(VirtIODevice *vdev)
38
return 0;
39
40
fail_vrings:
41
- virtio_scsi_clear_aio(s);
42
+ aio_wait_bh_oneshot(s->ctx, virtio_scsi_dataplane_stop_bh, s);
43
aio_context_release(s->ctx);
44
for (i = 0; i < vs->conf.num_queues + 2; i++) {
45
virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
46
@@ -XXX,XX +XXX,XX @@ void virtio_scsi_dataplane_stop(VirtIODevice *vdev)
47
s->dataplane_stopping = true;
48
49
aio_context_acquire(s->ctx);
50
- virtio_scsi_clear_aio(s);
51
+ aio_wait_bh_oneshot(s->ctx, virtio_scsi_dataplane_stop_bh, s);
52
aio_context_release(s->ctx);
53
54
blk_drain_all(); /* ensure there are no in-flight requests */
55
--
56
2.14.3
57
58
diff view generated by jsdifflib
New patch
1
Commit 00d09fdbbae5f7864ce754913efc84c12fdf9f1a ("vl: pause vcpus before
2
stopping iothreads") and commit dce8921b2baaf95974af8176406881872067adfa
3
("iothread: Stop threads before main() quits") tried to work around the
4
fact that emulation was still active during termination by stopping
5
iothreads. They suffer from race conditions:
6
1. virtio_scsi_handle_cmd_vq() racing with iothread_stop_all() hits the
7
virtio_scsi_ctx_check() assertion failure because the BDS AioContext
8
has been modified by iothread_stop_all().
9
2. Guest vq kick racing with main loop termination leaves a readable
10
ioeventfd that is handled by the next aio_poll() when external
11
clients are enabled again, resulting in unwanted emulation activity.
1
12
13
This patch obsoletes those commits by fully disabling emulation activity
14
when vcpus are stopped.
15
16
Use the new vm_shutdown() function instead of pause_all_vcpus() so that
17
vm change state handlers are invoked too. Virtio devices will now stop
18
their ioeventfds, preventing further emulation activity after vm_stop().
19
20
Note that vm_stop(RUN_STATE_SHUTDOWN) cannot be used because it emits a
21
QMP STOP event that may affect existing clients.
22
23
It is no longer necessary to call replay_disable_events() directly since
24
vm_shutdown() does so already.
25
26
Drop iothread_stop_all() since it is no longer used.
27
28
Cc: Fam Zheng <famz@redhat.com>
29
Cc: Kevin Wolf <kwolf@redhat.com>
30
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
31
Reviewed-by: Fam Zheng <famz@redhat.com>
32
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
33
Message-id: 20180307144205.20619-5-stefanha@redhat.com
34
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
35
---
36
include/sysemu/iothread.h | 1 -
37
include/sysemu/sysemu.h | 1 +
38
cpus.c | 16 +++++++++++++---
39
iothread.c | 31 -------------------------------
40
vl.c | 13 +++----------
41
5 files changed, 17 insertions(+), 45 deletions(-)
42
43
diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
44
index XXXXXXX..XXXXXXX 100644
45
--- a/include/sysemu/iothread.h
46
+++ b/include/sysemu/iothread.h
47
@@ -XXX,XX +XXX,XX @@ typedef struct {
48
char *iothread_get_id(IOThread *iothread);
49
IOThread *iothread_by_id(const char *id);
50
AioContext *iothread_get_aio_context(IOThread *iothread);
51
-void iothread_stop_all(void);
52
GMainContext *iothread_get_g_main_context(IOThread *iothread);
53
54
/*
55
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
56
index XXXXXXX..XXXXXXX 100644
57
--- a/include/sysemu/sysemu.h
58
+++ b/include/sysemu/sysemu.h
59
@@ -XXX,XX +XXX,XX @@ void vm_start(void);
60
int vm_prepare_start(void);
61
int vm_stop(RunState state);
62
int vm_stop_force_state(RunState state);
63
+int vm_shutdown(void);
64
65
typedef enum WakeupReason {
66
/* Always keep QEMU_WAKEUP_REASON_NONE = 0 */
67
diff --git a/cpus.c b/cpus.c
68
index XXXXXXX..XXXXXXX 100644
69
--- a/cpus.c
70
+++ b/cpus.c
71
@@ -XXX,XX +XXX,XX @@ void cpu_synchronize_all_pre_loadvm(void)
72
}
73
}
74
75
-static int do_vm_stop(RunState state)
76
+static int do_vm_stop(RunState state, bool send_stop)
77
{
78
int ret = 0;
79
80
@@ -XXX,XX +XXX,XX @@ static int do_vm_stop(RunState state)
81
pause_all_vcpus();
82
runstate_set(state);
83
vm_state_notify(0, state);
84
- qapi_event_send_stop(&error_abort);
85
+ if (send_stop) {
86
+ qapi_event_send_stop(&error_abort);
87
+ }
88
}
89
90
bdrv_drain_all();
91
@@ -XXX,XX +XXX,XX @@ static int do_vm_stop(RunState state)
92
return ret;
93
}
94
95
+/* Special vm_stop() variant for terminating the process. Historically clients
96
+ * did not expect a QMP STOP event and so we need to retain compatibility.
97
+ */
98
+int vm_shutdown(void)
99
+{
100
+ return do_vm_stop(RUN_STATE_SHUTDOWN, false);
101
+}
102
+
103
static bool cpu_can_run(CPUState *cpu)
104
{
105
if (cpu->stop) {
106
@@ -XXX,XX +XXX,XX @@ int vm_stop(RunState state)
107
return 0;
108
}
109
110
- return do_vm_stop(state);
111
+ return do_vm_stop(state, true);
112
}
113
114
/**
115
diff --git a/iothread.c b/iothread.c
116
index XXXXXXX..XXXXXXX 100644
117
--- a/iothread.c
118
+++ b/iothread.c
119
@@ -XXX,XX +XXX,XX @@ void iothread_stop(IOThread *iothread)
120
qemu_thread_join(&iothread->thread);
121
}
122
123
-static int iothread_stop_iter(Object *object, void *opaque)
124
-{
125
- IOThread *iothread;
126
-
127
- iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
128
- if (!iothread) {
129
- return 0;
130
- }
131
- iothread_stop(iothread);
132
- return 0;
133
-}
134
-
135
static void iothread_instance_init(Object *obj)
136
{
137
IOThread *iothread = IOTHREAD(obj);
138
@@ -XXX,XX +XXX,XX @@ IOThreadInfoList *qmp_query_iothreads(Error **errp)
139
return head;
140
}
141
142
-void iothread_stop_all(void)
143
-{
144
- Object *container = object_get_objects_root();
145
- BlockDriverState *bs;
146
- BdrvNextIterator it;
147
-
148
- for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
149
- AioContext *ctx = bdrv_get_aio_context(bs);
150
- if (ctx == qemu_get_aio_context()) {
151
- continue;
152
- }
153
- aio_context_acquire(ctx);
154
- bdrv_set_aio_context(bs, qemu_get_aio_context());
155
- aio_context_release(ctx);
156
- }
157
-
158
- object_child_foreach(container, iothread_stop_iter, NULL);
159
-}
160
-
161
static gpointer iothread_g_main_context_init(gpointer opaque)
162
{
163
AioContext *ctx;
164
diff --git a/vl.c b/vl.c
165
index XXXXXXX..XXXXXXX 100644
166
--- a/vl.c
167
+++ b/vl.c
168
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv, char **envp)
169
os_setup_post();
170
171
main_loop();
172
- replay_disable_events();
173
174
- /* The ordering of the following is delicate. Stop vcpus to prevent new
175
- * I/O requests being queued by the guest. Then stop IOThreads (this
176
- * includes a drain operation and completes all request processing). At
177
- * this point emulated devices are still associated with their IOThreads
178
- * (if any) but no longer have any work to do. Only then can we close
179
- * block devices safely because we know there is no more I/O coming.
180
- */
181
- pause_all_vcpus();
182
- iothread_stop_all();
183
+ /* No more vcpu or device emulation activity beyond this point */
184
+ vm_shutdown();
185
+
186
bdrv_close_all();
187
188
res_free();
189
--
190
2.14.3
191
192
diff view generated by jsdifflib