[PULL 0/1] Block patches
[PULL for-8.1 0/2] Block patches
1
The following changes since commit 887cba855bb6ff4775256f7968409281350b568c:
1
The following changes since commit 9ba37026fcf6b7f3f096c0cca3e1e7307802486b:
2
2
3
configure: Fix cross-building for RISCV host (v5) (2023-07-11 17:56:09 +0100)
3
Update version for v8.1.0-rc2 release (2023-08-02 08:22:45 -0700)
4
4
5
are available in the Git repository at:
5
are available in the Git repository at:
6
6
7
https://gitlab.com/stefanha/qemu.git tags/block-pull-request
7
https://gitlab.com/stefanha/qemu.git tags/block-pull-request
8
8
9
for you to fetch changes up to 75dcb4d790bbe5327169fd72b185960ca58e2fa6:
9
for you to fetch changes up to 9b06d0d076271d76e5384d767ef94a676f0a9efd:
10
10
11
virtio-blk: fix host notifier issues during dataplane start/stop (2023-07-12 15:20:32 -0400)
11
block/blkio: add more comments on the fd passing handling (2023-08-03 11:28:43 -0400)
12
12
13
----------------------------------------------------------------
13
----------------------------------------------------------------
14
Pull request
14
Pull request
15
15
16
Fix for an fd leak in the blkio block driver.
17
16
----------------------------------------------------------------
18
----------------------------------------------------------------
17
19
18
Stefan Hajnoczi (1):
20
Stefano Garzarella (2):
19
virtio-blk: fix host notifier issues during dataplane start/stop
21
block/blkio: close the fd when blkio_connect() fails
22
block/blkio: add more comments on the fd passing handling
20
23
21
hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++--------------
24
block/blkio.c | 28 +++++++++++++++++++++-------
22
1 file changed, 38 insertions(+), 29 deletions(-)
25
1 file changed, 21 insertions(+), 7 deletions(-)
23
26
24
--
27
--
25
2.40.1
28
2.41.0
diff view generated by jsdifflib
[PULL 1/1] virtio-blk: fix host notifier issues during dataplane start/stop
[PULL for-8.1 1/2] block/blkio: close the fd when blkio_connect() fails
1
The main loop thread can consume 100% CPU when using --device
1
From: Stefano Garzarella <sgarzare@redhat.com>
2
virtio-blk-pci,iothread=<iothread>. ppoll() constantly returns but
3
reading virtqueue host notifiers fails with EAGAIN. The file descriptors
4
are stale and remain registered with the AioContext because of bugs in
5
the virtio-blk dataplane start/stop code.
6
2
7
The problem is that the dataplane start/stop code involves drain
3
libblkio drivers take ownership of `fd` only after a successful
8
operations, which call virtio_blk_drained_begin() and
4
blkio_connect(), so if it fails, we are still the owners.
9
virtio_blk_drained_end() at points where the host notifier is not
10
operational:
11
- In virtio_blk_data_plane_start(), blk_set_aio_context() drains after
12
vblk->dataplane_started has been set to true but the host notifier has
13
not been attached yet.
14
- In virtio_blk_data_plane_stop(), blk_drain() and blk_set_aio_context()
15
drain after the host notifier has already been detached but with
16
vblk->dataplane_started still set to true.
17
5
18
I would like to simplify ->ioeventfd_start/stop() to avoid interactions
6
Fixes: cad2ccc395 ("block/blkio: use qemu_open() to support fd passing for virtio-blk")
19
with drain entirely, but couldn't find a way to do that. Instead, this
7
Suggested-by: Hanna Czenczek <hreitz@redhat.com>
20
patch accepts the fragile nature of the code and reorders it so that
8
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
21
vblk->dataplane_started is false during drain operations. This way the
9
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
22
virtio_blk_drained_begin() and virtio_blk_drained_end() calls don't
10
Message-id: 20230803082825.25293-2-sgarzare@redhat.com
23
touch the host notifier. The result is that
24
virtio_blk_data_plane_start() and virtio_blk_data_plane_stop() have
25
complete control over the host notifier and stale file descriptors are
26
no longer left in the AioContext.
27
28
This patch fixes the 100% CPU consumption in the main loop thread and
29
correctly moves host notifier processing to the IOThread.
30
31
Fixes: 1665d9326fd2 ("virtio-blk: implement BlockDevOps->drained_begin()")
32
Reported-by: Lukáš Doktor <ldoktor@redhat.com>
33
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
34
Tested-by: Lukas Doktor <ldoktor@redhat.com>
35
Message-id: 20230704151527.193586-1-stefanha@redhat.com
36
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
11
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
37
---
12
---
38
hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++--------------
13
block/blkio.c | 11 ++++++++---
39
1 file changed, 38 insertions(+), 29 deletions(-)
14
1 file changed, 8 insertions(+), 3 deletions(-)
40
15
41
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
16
diff --git a/block/blkio.c b/block/blkio.c
42
index XXXXXXX..XXXXXXX 100644
17
index XXXXXXX..XXXXXXX 100644
43
--- a/hw/block/dataplane/virtio-blk.c
18
--- a/block/blkio.c
44
+++ b/hw/block/dataplane/virtio-blk.c
19
+++ b/block/blkio.c
45
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
20
@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
46
21
const char *path = qdict_get_try_str(options, "path");
47
memory_region_transaction_commit();
22
BDRVBlkioState *s = bs->opaque;
48
23
bool fd_supported = false;
49
- /*
24
- int fd, ret;
50
- * These fields are visible to the IOThread so we rely on implicit barriers
25
+ int fd = -1, ret;
51
- * in aio_context_acquire() on the write side and aio_notify_accept() on
26
52
- * the read side.
27
if (!path) {
53
- */
28
error_setg(errp, "missing 'path' option");
54
- s->starting = false;
29
@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
55
- vblk->dataplane_started = true;
30
if (ret < 0) {
56
trace_virtio_blk_data_plane_start(s);
31
fd_supported = false;
57
32
qemu_close(fd);
58
old_context = blk_get_aio_context(s->conf->conf.blk);
33
+ fd = -1;
59
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
34
}
60
event_notifier_set(virtio_queue_get_host_notifier(vq));
35
}
61
}
36
}
62
37
@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
63
+ /*
64
+ * These fields must be visible to the IOThread when it processes the
65
+ * virtqueue, otherwise it will think dataplane has not started yet.
66
+ *
67
+ * Make sure ->dataplane_started is false when blk_set_aio_context() is
68
+ * called above so that draining does not cause the host notifier to be
69
+ * detached/attached prematurely.
70
+ */
71
+ s->starting = false;
72
+ vblk->dataplane_started = true;
73
+ smp_wmb(); /* paired with aio_notify_accept() on the read side */
74
+
75
/* Get this show started by hooking up our callbacks */
76
if (!blk_in_drain(s->conf->conf.blk)) {
77
aio_context_acquire(s->ctx);
78
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
79
fail_guest_notifiers:
80
vblk->dataplane_disabled = true;
81
s->starting = false;
82
- vblk->dataplane_started = true;
83
return -ENOSYS;
84
}
85
86
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
87
aio_wait_bh_oneshot(s->ctx, virtio_blk_data_plane_stop_bh, s);
88
}
38
}
89
39
90
+ /*
40
ret = blkio_connect(s->blkio);
91
+ * Batch all the host notifiers in a single transaction to avoid
41
+ if (ret < 0 && fd >= 0) {
92
+ * quadratic time complexity in address_space_update_ioeventfds().
42
+ /* Failed to give the FD to libblkio, close it */
93
+ */
43
+ qemu_close(fd);
94
+ memory_region_transaction_begin();
44
+ fd = -1;
95
+
96
+ for (i = 0; i < nvqs; i++) {
97
+ virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
98
+ }
45
+ }
99
+
46
+
100
+ /*
47
/*
101
+ * The transaction expects the ioeventfds to be open when it
48
* If the libblkio driver doesn't support the `fd` property, blkio_connect()
102
+ * commits. Do it now, before the cleanup loop.
49
* will fail with -EINVAL. So let's try calling blkio_connect() again by
103
+ */
50
* directly setting `path`.
104
+ memory_region_transaction_commit();
51
*/
105
+
52
if (fd_supported && ret == -EINVAL) {
106
+ for (i = 0; i < nvqs; i++) {
53
- qemu_close(fd);
107
+ virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i);
108
+ }
109
+
110
+ /*
111
+ * Set ->dataplane_started to false before draining so that host notifiers
112
+ * are not detached/attached anymore.
113
+ */
114
+ vblk->dataplane_started = false;
115
+
116
aio_context_acquire(s->ctx);
117
118
/* Wait for virtio_blk_dma_restart_bh() and in flight I/O to complete */
119
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
120
121
aio_context_release(s->ctx);
122
123
- /*
124
- * Batch all the host notifiers in a single transaction to avoid
125
- * quadratic time complexity in address_space_update_ioeventfds().
126
- */
127
- memory_region_transaction_begin();
128
-
54
-
129
- for (i = 0; i < nvqs; i++) {
55
/*
130
- virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
56
* We need to clear the `fd` property we set previously by setting
131
- }
57
* it to -1.
132
-
133
- /*
134
- * The transaction expects the ioeventfds to be open when it
135
- * commits. Do it now, before the cleanup loop.
136
- */
137
- memory_region_transaction_commit();
138
-
139
- for (i = 0; i < nvqs; i++) {
140
- virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i);
141
- }
142
-
143
qemu_bh_cancel(s->bh);
144
notify_guest_bh(s); /* final chance to notify guest */
145
146
/* Clean up guest notifier (irq) */
147
k->set_guest_notifiers(qbus->parent, nvqs, false);
148
149
- vblk->dataplane_started = false;
150
s->stopping = false;
151
}
152
--
58
--
153
2.40.1
59
2.41.0
154
155
diff view generated by jsdifflib
New patch
[PULL for-8.1 2/2] block/blkio: add more comments on the fd passing handling
1
From: Stefano Garzarella <sgarzare@redhat.com>
1
2
3
As Hanna pointed out, it is not clear in the code why qemu_open()
4
can fail, and why blkio_set_int("fd") is not enough to discover
5
the `fd` property support.
6
7
Let's fix them by adding more details in the code comments.
8
9
Suggested-by: Hanna Czenczek <hreitz@redhat.com>
10
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
11
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
12
Message-id: 20230803082825.25293-3-sgarzare@redhat.com
13
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
14
---
15
block/blkio.c | 15 ++++++++++++---
16
1 file changed, 12 insertions(+), 3 deletions(-)
17
18
diff --git a/block/blkio.c b/block/blkio.c
19
index XXXXXXX..XXXXXXX 100644
20
--- a/block/blkio.c
21
+++ b/block/blkio.c
22
@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
23
*/
24
fd = qemu_open(path, O_RDWR, NULL);
25
if (fd < 0) {
26
+ /*
27
+ * qemu_open() can fail if the user specifies a path that is not
28
+ * a file or device, for example in the case of Unix Domain Socket
29
+ * for the virtio-blk-vhost-user driver. In such cases let's have
30
+ * libblkio open the path directly.
31
+ */
32
fd_supported = false;
33
} else {
34
ret = blkio_set_int(s->blkio, "fd", fd);
35
@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
36
}
37
38
/*
39
- * If the libblkio driver doesn't support the `fd` property, blkio_connect()
40
- * will fail with -EINVAL. So let's try calling blkio_connect() again by
41
- * directly setting `path`.
42
+ * Before https://gitlab.com/libblkio/libblkio/-/merge_requests/208
43
+ * (libblkio <= v1.3.0), setting the `fd` property is not enough to check
44
+ * whether the driver supports the `fd` property or not. In that case,
45
+ * blkio_connect() will fail with -EINVAL.
46
+ * So let's try calling blkio_connect() again by directly setting `path`
47
+ * to cover this scenario.
48
*/
49
if (fd_supported && ret == -EINVAL) {
50
/*
51
--
52
2.41.0
diff view generated by jsdifflib

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.