1
The following changes since commit 508ba0f7e2092d3ca56e3f75e894d52d8b94818e:
1
The following changes since commit 6c769690ac845fa62642a5f93b4e4bd906adab95:
2
2
3
Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20171109' into staging (2017-11-13 11:41:47 +0000)
3
Merge remote-tracking branch 'remotes/vsementsov/tags/pull-simplebench-2021-05-04' into staging (2021-05-21 12:02:34 +0100)
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://gitlab.com/stefanha/qemu.git tags/block-pull-request
8
8
9
for you to fetch changes up to 0761562687e0d8135310a94b1d3e08376387c027:
9
for you to fetch changes up to 0a6f0c76a030710780ce10d6347a70f098024d21:
10
10
11
qemu-iotests: Test I/O limits with removable media (2017-11-13 15:46:26 +0000)
11
coroutine-sleep: introduce qemu_co_sleep (2021-05-21 18:22:33 +0100)
12
12
13
----------------------------------------------------------------
13
----------------------------------------------------------------
14
Pull request
14
Pull request
15
15
16
The following disk I/O throttling fixes solve recent bugs.
16
(Resent due to an email preparation mistake.)
17
17
18
----------------------------------------------------------------
18
----------------------------------------------------------------
19
19
20
Alberto Garcia (3):
20
Paolo Bonzini (6):
21
block: Check for inserted BlockDriverState in blk_io_limits_disable()
21
coroutine-sleep: use a stack-allocated timer
22
block: Leave valid throttle timers when removing a BDS from a backend
22
coroutine-sleep: disallow NULL QemuCoSleepState** argument
23
qemu-iotests: Test I/O limits with removable media
23
coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing
24
coroutine-sleep: move timer out of QemuCoSleepState
25
coroutine-sleep: replace QemuCoSleepState pointer with struct in the
26
API
27
coroutine-sleep: introduce qemu_co_sleep
24
28
25
Stefan Hajnoczi (1):
29
Philippe Mathieu-Daudé (1):
26
throttle-groups: drain before detaching ThrottleState
30
bitops.h: Improve find_xxx_bit() documentation
27
31
28
Zhengui (1):
32
Zenghui Yu (1):
29
block: all I/O should be completed before removing throttle timers.
33
multi-process: Initialize variables declared with g_auto*
30
34
31
block/block-backend.c | 36 ++++++++++++++++++---------
35
include/qemu/bitops.h | 15 ++++++--
32
block/throttle-groups.c | 6 +++++
36
include/qemu/coroutine.h | 27 ++++++++-----
33
tests/qemu-iotests/093 | 62 ++++++++++++++++++++++++++++++++++++++++++++++
37
block/block-copy.c | 10 ++---
34
tests/qemu-iotests/093.out | 4 +--
38
block/nbd.c | 14 +++----
35
4 files changed, 94 insertions(+), 14 deletions(-)
39
hw/remote/memory.c | 5 +--
40
hw/remote/proxy.c | 3 +-
41
util/qemu-coroutine-sleep.c | 75 +++++++++++++++++++------------------
42
7 files changed, 79 insertions(+), 70 deletions(-)
36
43
37
--
44
--
38
2.13.6
45
2.31.1
39
46
40
diff view generated by jsdifflib
New patch
1
From: Zenghui Yu <yuzenghui@huawei.com>
1
2
3
Quote docs/devel/style.rst (section "Automatic memory deallocation"):
4
5
* Variables declared with g_auto* MUST always be initialized,
6
otherwise the cleanup function will use uninitialized stack memory
7
8
Initialize @name properly to get rid of the compilation error (using
9
gcc-7.3.0 on CentOS):
10
11
../hw/remote/proxy.c: In function 'pci_proxy_dev_realize':
12
/usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be used uninitialized in this function [-Werror=maybe-uninitialized]
13
g_free (*pp);
14
^~~~~~~~~~~~
15
../hw/remote/proxy.c:350:30: note: 'name' was declared here
16
g_autofree char *name;
17
^~~~
18
19
Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
20
Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
21
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
22
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
23
Message-id: 20210312112143.1369-1-yuzenghui@huawei.com
24
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
25
---
26
hw/remote/memory.c | 5 ++---
27
hw/remote/proxy.c | 3 +--
28
2 files changed, 3 insertions(+), 5 deletions(-)
29
30
diff --git a/hw/remote/memory.c b/hw/remote/memory.c
31
index XXXXXXX..XXXXXXX 100644
32
--- a/hw/remote/memory.c
33
+++ b/hw/remote/memory.c
34
@@ -XXX,XX +XXX,XX @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
35
36
remote_sysmem_reset();
37
38
- for (region = 0; region < msg->num_fds; region++) {
39
- g_autofree char *name;
40
+ for (region = 0; region < msg->num_fds; region++, suffix++) {
41
+ g_autofree char *name = g_strdup_printf("remote-mem-%u", suffix);
42
subregion = g_new(MemoryRegion, 1);
43
- name = g_strdup_printf("remote-mem-%u", suffix++);
44
memory_region_init_ram_from_fd(subregion, NULL,
45
name, sysmem_info->sizes[region],
46
true, msg->fds[region],
47
diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
48
index XXXXXXX..XXXXXXX 100644
49
--- a/hw/remote/proxy.c
50
+++ b/hw/remote/proxy.c
51
@@ -XXX,XX +XXX,XX @@ static void probe_pci_info(PCIDevice *dev, Error **errp)
52
PCI_BASE_ADDRESS_SPACE_IO : PCI_BASE_ADDRESS_SPACE_MEMORY;
53
54
if (size) {
55
- g_autofree char *name;
56
+ g_autofree char *name = g_strdup_printf("bar-region-%d", i);
57
pdev->region[i].dev = pdev;
58
pdev->region[i].present = true;
59
if (type == PCI_BASE_ADDRESS_SPACE_MEMORY) {
60
pdev->region[i].memory = true;
61
}
62
- name = g_strdup_printf("bar-region-%d", i);
63
memory_region_init_io(&pdev->region[i].mr, OBJECT(pdev),
64
&proxy_mr_ops, &pdev->region[i],
65
name, size);
66
--
67
2.31.1
68
diff view generated by jsdifflib
New patch
1
From: Philippe Mathieu-Daudé <philmd@redhat.com>
1
2
3
Document the following functions return the bitmap size
4
if no matching bit is found:
5
6
- find_first_bit
7
- find_next_bit
8
- find_last_bit
9
- find_first_zero_bit
10
- find_next_zero_bit
11
12
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
13
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
14
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
15
Message-id: 20210510200758.2623154-2-philmd@redhat.com
16
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
17
---
18
include/qemu/bitops.h | 15 ++++++++++++---
19
1 file changed, 12 insertions(+), 3 deletions(-)
20
21
diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
22
index XXXXXXX..XXXXXXX 100644
23
--- a/include/qemu/bitops.h
24
+++ b/include/qemu/bitops.h
25
@@ -XXX,XX +XXX,XX @@ static inline int test_bit(long nr, const unsigned long *addr)
26
* @addr: The address to start the search at
27
* @size: The maximum size to search
28
*
29
- * Returns the bit number of the first set bit, or size.
30
+ * Returns the bit number of the last set bit,
31
+ * or @size if there is no set bit in the bitmap.
32
*/
33
unsigned long find_last_bit(const unsigned long *addr,
34
unsigned long size);
35
@@ -XXX,XX +XXX,XX @@ unsigned long find_last_bit(const unsigned long *addr,
36
* @addr: The address to base the search on
37
* @offset: The bitnumber to start searching at
38
* @size: The bitmap size in bits
39
+ *
40
+ * Returns the bit number of the next set bit,
41
+ * or @size if there are no further set bits in the bitmap.
42
*/
43
unsigned long find_next_bit(const unsigned long *addr,
44
unsigned long size,
45
@@ -XXX,XX +XXX,XX @@ unsigned long find_next_bit(const unsigned long *addr,
46
* @addr: The address to base the search on
47
* @offset: The bitnumber to start searching at
48
* @size: The bitmap size in bits
49
+ *
50
+ * Returns the bit number of the next cleared bit,
51
+ * or @size if there are no further clear bits in the bitmap.
52
*/
53
54
unsigned long find_next_zero_bit(const unsigned long *addr,
55
@@ -XXX,XX +XXX,XX @@ unsigned long find_next_zero_bit(const unsigned long *addr,
56
* @addr: The address to start the search at
57
* @size: The maximum size to search
58
*
59
- * Returns the bit number of the first set bit.
60
+ * Returns the bit number of the first set bit,
61
+ * or @size if there is no set bit in the bitmap.
62
*/
63
static inline unsigned long find_first_bit(const unsigned long *addr,
64
unsigned long size)
65
@@ -XXX,XX +XXX,XX @@ static inline unsigned long find_first_bit(const unsigned long *addr,
66
* @addr: The address to start the search at
67
* @size: The maximum size to search
68
*
69
- * Returns the bit number of the first cleared bit.
70
+ * Returns the bit number of the first cleared bit,
71
+ * or @size if there is no clear bit in the bitmap.
72
*/
73
static inline unsigned long find_first_zero_bit(const unsigned long *addr,
74
unsigned long size)
75
--
76
2.31.1
77
diff view generated by jsdifflib
New patch
1
From: Paolo Bonzini <pbonzini@redhat.com>
1
2
3
The lifetime of the timer is well-known (it cannot outlive
4
qemu_co_sleep_ns_wakeable, because it's deleted by the time the
5
coroutine resumes), so it is not necessary to place it on the heap.
6
7
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
8
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
9
Message-id: 20210517100548.28806-2-pbonzini@redhat.com
10
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
11
---
12
util/qemu-coroutine-sleep.c | 9 ++++-----
13
1 file changed, 4 insertions(+), 5 deletions(-)
14
15
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
16
index XXXXXXX..XXXXXXX 100644
17
--- a/util/qemu-coroutine-sleep.c
18
+++ b/util/qemu-coroutine-sleep.c
19
@@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
20
21
struct QemuCoSleepState {
22
Coroutine *co;
23
- QEMUTimer *ts;
24
+ QEMUTimer ts;
25
QemuCoSleepState **user_state_pointer;
26
};
27
28
@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
29
if (sleep_state->user_state_pointer) {
30
*sleep_state->user_state_pointer = NULL;
31
}
32
- timer_del(sleep_state->ts);
33
+ timer_del(&sleep_state->ts);
34
aio_co_wake(sleep_state->co);
35
}
36
37
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
38
AioContext *ctx = qemu_get_current_aio_context();
39
QemuCoSleepState state = {
40
.co = qemu_coroutine_self(),
41
- .ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, &state),
42
.user_state_pointer = sleep_state,
43
};
44
45
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
46
abort();
47
}
48
49
+ aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
50
if (sleep_state) {
51
*sleep_state = &state;
52
}
53
- timer_mod(state.ts, qemu_clock_get_ns(type) + ns);
54
+ timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
55
qemu_coroutine_yield();
56
if (sleep_state) {
57
/*
58
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
59
*/
60
assert(*sleep_state == NULL);
61
}
62
- timer_free(state.ts);
63
}
64
--
65
2.31.1
66
diff view generated by jsdifflib
1
From: Alberto Garcia <berto@igalia.com>
1
From: Paolo Bonzini <pbonzini@redhat.com>
2
2
3
This test hotplugs a CD drive to a VM and checks that I/O limits can
3
Simplify the code by removing conditionals. qemu_co_sleep_ns
4
be set only when the drive has media inserted and that they are kept
4
can simply point the argument to an on-stack temporary.
5
when the media is replaced.
6
5
7
This also tests the removal of a device with valid I/O limits set but
6
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
8
no media inserted. This involves deleting and disabling the limits
7
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
9
of a BlockBackend without BlockDriverState, a scenario that has been
8
Message-id: 20210517100548.28806-3-pbonzini@redhat.com
10
crashing until the fixes from the last couple of patches.
11
12
[Python PEP8 fixup: "Don't use spaces are the = sign when used to
13
indicate a keyword argument or a default parameter value"
14
--Stefan]
15
16
Signed-off-by: Alberto Garcia <berto@igalia.com>
17
Reviewed-by: Max Reitz <mreitz@redhat.com>
18
Message-id: 071eb397118ed207c5a7f01d58766e415ee18d6a.1510339534.git.berto@igalia.com
19
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
9
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
20
---
10
---
21
tests/qemu-iotests/093 | 62 ++++++++++++++++++++++++++++++++++++++++++++++
11
include/qemu/coroutine.h | 5 +++--
22
tests/qemu-iotests/093.out | 4 +--
12
util/qemu-coroutine-sleep.c | 18 +++++-------------
23
2 files changed, 64 insertions(+), 2 deletions(-)
13
2 files changed, 8 insertions(+), 15 deletions(-)
24
14
25
diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
15
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
26
index XXXXXXX..XXXXXXX 100755
16
index XXXXXXX..XXXXXXX 100644
27
--- a/tests/qemu-iotests/093
17
--- a/include/qemu/coroutine.h
28
+++ b/tests/qemu-iotests/093
18
+++ b/include/qemu/coroutine.h
29
@@ -XXX,XX +XXX,XX @@ class ThrottleTestGroupNames(iotests.QMPTestCase):
19
@@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleepState QemuCoSleepState;
30
groupname = "group%d" % i
20
31
self.verify_name(devname, groupname)
21
/**
32
22
* Yield the coroutine for a given duration. During this yield, @sleep_state
33
+class ThrottleTestRemovableMedia(iotests.QMPTestCase):
23
- * (if not NULL) is set to an opaque pointer, which may be used for
34
+ def setUp(self):
24
+ * is set to an opaque pointer, which may be used for
35
+ self.vm = iotests.VM()
25
* qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
36
+ if iotests.qemu_default_machine == 's390-ccw-virtio':
26
* timer fires. Don't save the obtained value to other variables and don't call
37
+ self.vm.add_device("virtio-scsi-ccw,id=virtio-scsi")
27
* qemu_co_sleep_wake from another aio context.
38
+ else:
28
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
39
+ self.vm.add_device("virtio-scsi-pci,id=virtio-scsi")
29
QemuCoSleepState **sleep_state);
40
+ self.vm.launch()
30
static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
31
{
32
- qemu_co_sleep_ns_wakeable(type, ns, NULL);
33
+ QemuCoSleepState *unused = NULL;
34
+ qemu_co_sleep_ns_wakeable(type, ns, &unused);
35
}
36
37
/**
38
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
39
index XXXXXXX..XXXXXXX 100644
40
--- a/util/qemu-coroutine-sleep.c
41
+++ b/util/qemu-coroutine-sleep.c
42
@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
43
qemu_co_sleep_ns__scheduled, NULL);
44
45
assert(scheduled == qemu_co_sleep_ns__scheduled);
46
- if (sleep_state->user_state_pointer) {
47
- *sleep_state->user_state_pointer = NULL;
48
- }
49
+ *sleep_state->user_state_pointer = NULL;
50
timer_del(&sleep_state->ts);
51
aio_co_wake(sleep_state->co);
52
}
53
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
54
}
55
56
aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
57
- if (sleep_state) {
58
- *sleep_state = &state;
59
- }
60
+ *sleep_state = &state;
61
timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
62
qemu_coroutine_yield();
63
- if (sleep_state) {
64
- /*
65
- * Note that *sleep_state is cleared during qemu_co_sleep_wake
66
- * before resuming this coroutine.
67
- */
68
- assert(*sleep_state == NULL);
69
- }
41
+
70
+
42
+ def tearDown(self):
71
+ /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */
43
+ self.vm.shutdown()
72
+ assert(*sleep_state == NULL);
44
+
73
}
45
+ def test_removable_media(self):
46
+ # Add a couple of dummy nodes named cd0 and cd1
47
+ result = self.vm.qmp("blockdev-add", driver="null-aio",
48
+ node_name="cd0")
49
+ self.assert_qmp(result, 'return', {})
50
+ result = self.vm.qmp("blockdev-add", driver="null-aio",
51
+ node_name="cd1")
52
+ self.assert_qmp(result, 'return', {})
53
+
54
+ # Attach a CD drive with cd0 inserted
55
+ result = self.vm.qmp("device_add", driver="scsi-cd",
56
+ id="dev0", drive="cd0")
57
+ self.assert_qmp(result, 'return', {})
58
+
59
+ # Set I/O limits
60
+ args = { "id": "dev0", "iops": 100, "iops_rd": 0, "iops_wr": 0,
61
+ "bps": 50, "bps_rd": 0, "bps_wr": 0 }
62
+ result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **args)
63
+ self.assert_qmp(result, 'return', {})
64
+
65
+ # Check that the I/O limits have been set
66
+ result = self.vm.qmp("query-block")
67
+ self.assert_qmp(result, 'return[0]/inserted/iops', 100)
68
+ self.assert_qmp(result, 'return[0]/inserted/bps', 50)
69
+
70
+ # Now eject cd0 and insert cd1
71
+ result = self.vm.qmp("blockdev-open-tray", id='dev0')
72
+ self.assert_qmp(result, 'return', {})
73
+ result = self.vm.qmp("x-blockdev-remove-medium", id='dev0')
74
+ self.assert_qmp(result, 'return', {})
75
+ result = self.vm.qmp("x-blockdev-insert-medium", id='dev0', node_name='cd1')
76
+ self.assert_qmp(result, 'return', {})
77
+
78
+ # Check that the I/O limits are still the same
79
+ result = self.vm.qmp("query-block")
80
+ self.assert_qmp(result, 'return[0]/inserted/iops', 100)
81
+ self.assert_qmp(result, 'return[0]/inserted/bps', 50)
82
+
83
+ # Eject cd1
84
+ result = self.vm.qmp("x-blockdev-remove-medium", id='dev0')
85
+ self.assert_qmp(result, 'return', {})
86
+
87
+ # Check that we can't set limits if the device has no medium
88
+ result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **args)
89
+ self.assert_qmp(result, 'error/class', 'GenericError')
90
+
91
+ # Remove the CD drive
92
+ result = self.vm.qmp("device_del", id='dev0')
93
+ self.assert_qmp(result, 'return', {})
94
+
95
96
if __name__ == '__main__':
97
iotests.main(supported_fmts=["raw"])
98
diff --git a/tests/qemu-iotests/093.out b/tests/qemu-iotests/093.out
99
index XXXXXXX..XXXXXXX 100644
100
--- a/tests/qemu-iotests/093.out
101
+++ b/tests/qemu-iotests/093.out
102
@@ -XXX,XX +XXX,XX @@
103
-.......
104
+........
105
----------------------------------------------------------------------
106
-Ran 7 tests
107
+Ran 8 tests
108
109
OK
110
--
74
--
111
2.13.6
75
2.31.1
112
76
113
diff view generated by jsdifflib
1
From: Alberto Garcia <berto@igalia.com>
1
From: Paolo Bonzini <pbonzini@redhat.com>
2
2
3
When you set I/O limits using block_set_io_throttle or the command
3
All callers of qemu_co_sleep_wake are checking whether they are passing
4
line throttling.* options they are kept in the BlockBackend regardless
4
a NULL argument inside the pointer-to-pointer: do the check in
5
of whether a BlockDriverState is attached to the backend or not.
5
qemu_co_sleep_wake itself.
6
6
7
Therefore when removing the limits using blk_io_limits_disable() we
7
As a side effect, qemu_co_sleep_wake can be called more than once and
8
need to check if there's a BDS before attempting to drain it, else it
8
it will only wake the coroutine once; after the first time, the argument
9
will crash QEMU. This can be reproduced very easily using HMP:
9
will be set to NULL via *sleep_state->user_state_pointer. However, this
10
would not be safe unless co_sleep_cb keeps using the QemuCoSleepState*
11
directly, so make it go through the pointer-to-pointer instead.
10
12
11
(qemu) drive_add 0 if=none,throttling.iops-total=5000
13
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
12
(qemu) drive_del none0
14
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
13
15
Message-id: 20210517100548.28806-4-pbonzini@redhat.com
14
Reported-by: sochin jiang <sochin.jiang@huawei.com>
15
Signed-off-by: Alberto Garcia <berto@igalia.com>
16
Reviewed-by: Max Reitz <mreitz@redhat.com>
17
Message-id: 0d3a67ce8d948bb33e08672564714dcfb76a3d8c.1510339534.git.berto@igalia.com
18
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
16
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
19
---
17
---
20
block/block-backend.c | 14 ++++++++++----
18
block/block-copy.c | 4 +---
21
1 file changed, 10 insertions(+), 4 deletions(-)
19
block/nbd.c | 8 ++------
20
util/qemu-coroutine-sleep.c | 21 ++++++++++++---------
21
3 files changed, 15 insertions(+), 18 deletions(-)
22
22
23
diff --git a/block/block-backend.c b/block/block-backend.c
23
diff --git a/block/block-copy.c b/block/block-copy.c
24
index XXXXXXX..XXXXXXX 100644
24
index XXXXXXX..XXXXXXX 100644
25
--- a/block/block-backend.c
25
--- a/block/block-copy.c
26
+++ b/block/block-backend.c
26
+++ b/block/block-copy.c
27
@@ -XXX,XX +XXX,XX @@ void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg)
27
@@ -XXX,XX +XXX,XX @@ out:
28
28
29
void blk_io_limits_disable(BlockBackend *blk)
29
void block_copy_kick(BlockCopyCallState *call_state)
30
{
30
{
31
- assert(blk->public.throttle_group_member.throttle_state);
31
- if (call_state->sleep_state) {
32
- bdrv_drained_begin(blk_bs(blk));
32
- qemu_co_sleep_wake(call_state->sleep_state);
33
- throttle_group_unregister_tgm(&blk->public.throttle_group_member);
33
- }
34
- bdrv_drained_end(blk_bs(blk));
34
+ qemu_co_sleep_wake(call_state->sleep_state);
35
+ BlockDriverState *bs = blk_bs(blk);
35
}
36
+ ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
36
37
+ assert(tgm->throttle_state);
37
/*
38
+ if (bs) {
38
diff --git a/block/nbd.c b/block/nbd.c
39
+ bdrv_drained_begin(bs);
39
index XXXXXXX..XXXXXXX 100644
40
+ }
40
--- a/block/nbd.c
41
+ throttle_group_unregister_tgm(tgm);
41
+++ b/block/nbd.c
42
+ if (bs) {
42
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
43
+ bdrv_drained_end(bs);
43
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
44
45
s->drained = true;
46
- if (s->connection_co_sleep_ns_state) {
47
- qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
48
- }
49
+ qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
50
51
nbd_co_establish_connection_cancel(bs, false);
52
53
@@ -XXX,XX +XXX,XX @@ static void nbd_teardown_connection(BlockDriverState *bs)
54
55
s->state = NBD_CLIENT_QUIT;
56
if (s->connection_co) {
57
- if (s->connection_co_sleep_ns_state) {
58
- qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
59
- }
60
+ qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
61
nbd_co_establish_connection_cancel(bs, true);
62
}
63
if (qemu_in_coroutine()) {
64
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
65
index XXXXXXX..XXXXXXX 100644
66
--- a/util/qemu-coroutine-sleep.c
67
+++ b/util/qemu-coroutine-sleep.c
68
@@ -XXX,XX +XXX,XX @@ struct QemuCoSleepState {
69
70
void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
71
{
72
- /* Write of schedule protected by barrier write in aio_co_schedule */
73
- const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
74
- qemu_co_sleep_ns__scheduled, NULL);
75
+ if (sleep_state) {
76
+ /* Write of schedule protected by barrier write in aio_co_schedule */
77
+ const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
78
+ qemu_co_sleep_ns__scheduled, NULL);
79
80
- assert(scheduled == qemu_co_sleep_ns__scheduled);
81
- *sleep_state->user_state_pointer = NULL;
82
- timer_del(&sleep_state->ts);
83
- aio_co_wake(sleep_state->co);
84
+ assert(scheduled == qemu_co_sleep_ns__scheduled);
85
+ *sleep_state->user_state_pointer = NULL;
86
+ timer_del(&sleep_state->ts);
87
+ aio_co_wake(sleep_state->co);
44
+ }
88
+ }
45
}
89
}
46
90
47
/* should be called before blk_set_io_limits if a limit is set */
91
static void co_sleep_cb(void *opaque)
92
{
93
- qemu_co_sleep_wake(opaque);
94
+ QemuCoSleepState **sleep_state = opaque;
95
+ qemu_co_sleep_wake(*sleep_state);
96
}
97
98
void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
99
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
100
abort();
101
}
102
103
- aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
104
+ aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
105
*sleep_state = &state;
106
timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
107
qemu_coroutine_yield();
48
--
108
--
49
2.13.6
109
2.31.1
50
110
51
diff view generated by jsdifflib
1
From: Alberto Garcia <berto@igalia.com>
1
From: Paolo Bonzini <pbonzini@redhat.com>
2
2
3
If a BlockBackend has I/O limits set then its ThrottleGroupMember
3
This simplification is enabled by the previous patch. Now aio_co_wake
4
structure uses the AioContext from its attached BlockDriverState.
4
will only be called once, therefore we do not care about a spurious
5
Those two contexts must be kept in sync manually. This is not
5
firing of the timer after a qemu_co_sleep_wake.
6
ideal and will be fixed in the future by removing the throttling
7
configuration from the BlockBackend and storing it in an implicit
8
filter node instead, but for now we have to live with this.
9
6
10
When you remove the BlockDriverState from the backend then the
7
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
11
throttle timers are destroyed. If a new BlockDriverState is later
8
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
12
inserted then they are created again using the new AioContext.
9
Message-id: 20210517100548.28806-5-pbonzini@redhat.com
13
14
There are a couple of problems with this:
15
16
a) The code manipulates the timers directly, leaving the
17
ThrottleGroupMember.aio_context field in an inconsisent state.
18
19
b) If you remove the I/O limits (e.g by destroying the backend)
20
when the timers are gone then throttle_group_unregister_tgm()
21
will attempt to destroy them again, crashing QEMU.
22
23
While b) could be fixed easily by allowing the timers to be freed
24
twice, this would result in a situation in which we can no longer
25
guarantee that a valid ThrottleState has a valid AioContext and
26
timers.
27
28
This patch ensures that the timers and AioContext are always valid
29
when I/O limits are set, regardless of whether the BlockBackend has a
30
BlockDriverState inserted or not.
31
32
[Fixed "There'a" typo as suggested by Max Reitz <mreitz@redhat.com>
33
--Stefan]
34
35
Reported-by: sochin jiang <sochin.jiang@huawei.com>
36
Signed-off-by: Alberto Garcia <berto@igalia.com>
37
Reviewed-by: Max Reitz <mreitz@redhat.com>
38
Message-id: e089c66e7c20289b046d782cea4373b765c5bc1d.1510339534.git.berto@igalia.com
39
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
10
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
40
---
11
---
41
block/block-backend.c | 16 ++++++++--------
12
util/qemu-coroutine-sleep.c | 8 ++++----
42
1 file changed, 8 insertions(+), 8 deletions(-)
13
1 file changed, 4 insertions(+), 4 deletions(-)
43
14
44
diff --git a/block/block-backend.c b/block/block-backend.c
15
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
45
index XXXXXXX..XXXXXXX 100644
16
index XXXXXXX..XXXXXXX 100644
46
--- a/block/block-backend.c
17
--- a/util/qemu-coroutine-sleep.c
47
+++ b/block/block-backend.c
18
+++ b/util/qemu-coroutine-sleep.c
48
@@ -XXX,XX +XXX,XX @@ BlockBackend *blk_by_public(BlockBackendPublic *public)
19
@@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
49
*/
20
50
void blk_remove_bs(BlockBackend *blk)
21
struct QemuCoSleepState {
22
Coroutine *co;
23
- QEMUTimer ts;
24
QemuCoSleepState **user_state_pointer;
25
};
26
27
@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
28
29
assert(scheduled == qemu_co_sleep_ns__scheduled);
30
*sleep_state->user_state_pointer = NULL;
31
- timer_del(&sleep_state->ts);
32
aio_co_wake(sleep_state->co);
33
}
34
}
35
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
36
QemuCoSleepState **sleep_state)
51
{
37
{
52
+ ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
38
AioContext *ctx = qemu_get_current_aio_context();
53
BlockDriverState *bs;
39
+ QEMUTimer ts;
54
- ThrottleTimers *tt;
40
QemuCoSleepState state = {
55
41
.co = qemu_coroutine_self(),
56
notifier_list_notify(&blk->remove_bs_notifiers, blk);
42
.user_state_pointer = sleep_state,
57
- if (blk->public.throttle_group_member.throttle_state) {
43
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
58
- tt = &blk->public.throttle_group_member.throttle_timers;
44
abort();
59
+ if (tgm->throttle_state) {
60
bs = blk_bs(blk);
61
bdrv_drained_begin(bs);
62
- throttle_timers_detach_aio_context(tt);
63
+ throttle_group_detach_aio_context(tgm);
64
+ throttle_group_attach_aio_context(tgm, qemu_get_aio_context());
65
bdrv_drained_end(bs);
66
}
45
}
67
46
68
@@ -XXX,XX +XXX,XX @@ void blk_remove_bs(BlockBackend *blk)
47
- aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
69
*/
48
+ aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
70
int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
49
*sleep_state = &state;
71
{
50
- timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
72
+ ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
51
+ timer_mod(&ts, qemu_clock_get_ns(type) + ns);
73
blk->root = bdrv_root_attach_child(bs, "root", &child_root,
52
qemu_coroutine_yield();
74
blk->perm, blk->shared_perm, blk, errp);
53
+ timer_del(&ts);
75
if (blk->root == NULL) {
54
76
@@ -XXX,XX +XXX,XX @@ int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
55
/* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */
77
bdrv_ref(bs);
56
assert(*sleep_state == NULL);
78
79
notifier_list_notify(&blk->insert_bs_notifiers, blk);
80
- if (blk->public.throttle_group_member.throttle_state) {
81
- throttle_timers_attach_aio_context(
82
- &blk->public.throttle_group_member.throttle_timers,
83
- bdrv_get_aio_context(bs));
84
+ if (tgm->throttle_state) {
85
+ throttle_group_detach_aio_context(tgm);
86
+ throttle_group_attach_aio_context(tgm, bdrv_get_aio_context(bs));
87
}
88
89
return 0;
90
--
57
--
91
2.13.6
58
2.31.1
92
59
93
diff view generated by jsdifflib
1
I/O requests hang after stop/cont commands at least since QEMU 2.10.0
1
From: Paolo Bonzini <pbonzini@redhat.com>
2
with -drive iops=100:
2
3
3
Right now, users of qemu_co_sleep_ns_wakeable are simply passing
4
(guest)$ dd if=/dev/zero of=/dev/vdb oflag=direct count=1000
4
a pointer to QemuCoSleepState by reference to the function. But
5
(qemu) stop
5
QemuCoSleepState really is just a Coroutine*; making the
6
(qemu) cont
6
content of the struct public is just as efficient and lets us
7
...I/O is stuck...
7
skip the user_state_pointer indirection.
8
8
9
This happens because blk_set_aio_context() detaches the ThrottleState
9
Since the usage is changed, take the occasion to rename the
10
while requests may still be in flight:
10
struct to QemuCoSleep.
11
11
12
if (tgm->throttle_state) {
12
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
13
throttle_group_detach_aio_context(tgm);
13
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
14
throttle_group_attach_aio_context(tgm, new_context);
14
Message-id: 20210517100548.28806-6-pbonzini@redhat.com
15
}
16
17
This patch encloses the detach/attach calls in a drained region so no
18
I/O request is left hanging. Also add assertions so we don't make the
19
same mistake again in the future.
20
21
Reported-by: Yongxue Hong <yhong@redhat.com>
22
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
23
Reviewed-by: Alberto Garcia <berto@igalia.com>
24
Message-id: 20171110151934.16883-1-stefanha@redhat.com
25
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
15
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
26
---
16
---
27
block/block-backend.c | 2 ++
17
include/qemu/coroutine.h | 23 +++++++++++----------
28
block/throttle-groups.c | 6 ++++++
18
block/block-copy.c | 8 ++++----
29
2 files changed, 8 insertions(+)
19
block/nbd.c | 10 ++++-----
30
20
util/qemu-coroutine-sleep.c | 41 ++++++++++++++++---------------------
31
diff --git a/block/block-backend.c b/block/block-backend.c
21
4 files changed, 39 insertions(+), 43 deletions(-)
32
index XXXXXXX..XXXXXXX 100644
22
33
--- a/block/block-backend.c
23
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
34
+++ b/block/block-backend.c
24
index XXXXXXX..XXXXXXX 100644
35
@@ -XXX,XX +XXX,XX @@ void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)
25
--- a/include/qemu/coroutine.h
36
26
+++ b/include/qemu/coroutine.h
37
if (bs) {
27
@@ -XXX,XX +XXX,XX @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
38
if (tgm->throttle_state) {
28
*/
39
+ bdrv_drained_begin(bs);
29
void qemu_co_rwlock_unlock(CoRwlock *lock);
40
throttle_group_detach_aio_context(tgm);
30
41
throttle_group_attach_aio_context(tgm, new_context);
31
-typedef struct QemuCoSleepState QemuCoSleepState;
42
+ bdrv_drained_end(bs);
32
+typedef struct QemuCoSleep {
43
}
33
+ Coroutine *to_wake;
44
bdrv_set_aio_context(bs, new_context);
34
+} QemuCoSleep;
35
36
/**
37
- * Yield the coroutine for a given duration. During this yield, @sleep_state
38
- * is set to an opaque pointer, which may be used for
39
- * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
40
- * timer fires. Don't save the obtained value to other variables and don't call
41
- * qemu_co_sleep_wake from another aio context.
42
+ * Yield the coroutine for a given duration. Initializes @w so that,
43
+ * during this yield, it can be passed to qemu_co_sleep_wake() to
44
+ * terminate the sleep.
45
*/
46
-void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
47
- QemuCoSleepState **sleep_state);
48
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
49
+ QEMUClockType type, int64_t ns);
50
+
51
static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
52
{
53
- QemuCoSleepState *unused = NULL;
54
- qemu_co_sleep_ns_wakeable(type, ns, &unused);
55
+ QemuCoSleep w = { 0 };
56
+ qemu_co_sleep_ns_wakeable(&w, type, ns);
57
}
58
59
/**
60
@@ -XXX,XX +XXX,XX @@ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
61
* qemu_co_sleep_ns() and should be checked to be non-NULL before calling
62
* qemu_co_sleep_wake().
63
*/
64
-void qemu_co_sleep_wake(QemuCoSleepState *sleep_state);
65
+void qemu_co_sleep_wake(QemuCoSleep *w);
66
67
/**
68
* Yield until a file descriptor becomes readable
69
diff --git a/block/block-copy.c b/block/block-copy.c
70
index XXXXXXX..XXXXXXX 100644
71
--- a/block/block-copy.c
72
+++ b/block/block-copy.c
73
@@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyCallState {
74
/* State */
75
int ret;
76
bool finished;
77
- QemuCoSleepState *sleep_state;
78
+ QemuCoSleep sleep;
79
bool cancelled;
80
81
/* OUT parameters */
82
@@ -XXX,XX +XXX,XX @@ block_copy_dirty_clusters(BlockCopyCallState *call_state)
83
if (ns > 0) {
84
block_copy_task_end(task, -EAGAIN);
85
g_free(task);
86
- qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, ns,
87
- &call_state->sleep_state);
88
+ qemu_co_sleep_ns_wakeable(&call_state->sleep,
89
+ QEMU_CLOCK_REALTIME, ns);
90
continue;
91
}
92
}
93
@@ -XXX,XX +XXX,XX @@ out:
94
95
void block_copy_kick(BlockCopyCallState *call_state)
96
{
97
- qemu_co_sleep_wake(call_state->sleep_state);
98
+ qemu_co_sleep_wake(&call_state->sleep);
99
}
100
101
/*
102
diff --git a/block/nbd.c b/block/nbd.c
103
index XXXXXXX..XXXXXXX 100644
104
--- a/block/nbd.c
105
+++ b/block/nbd.c
106
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVNBDState {
107
CoQueue free_sema;
108
Coroutine *connection_co;
109
Coroutine *teardown_co;
110
- QemuCoSleepState *connection_co_sleep_ns_state;
111
+ QemuCoSleep reconnect_sleep;
112
bool drained;
113
bool wait_drained_end;
114
int in_flight;
115
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
116
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
117
118
s->drained = true;
119
- qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
120
+ qemu_co_sleep_wake(&s->reconnect_sleep);
121
122
nbd_co_establish_connection_cancel(bs, false);
123
124
@@ -XXX,XX +XXX,XX @@ static void nbd_teardown_connection(BlockDriverState *bs)
125
126
s->state = NBD_CLIENT_QUIT;
127
if (s->connection_co) {
128
- qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
129
+ qemu_co_sleep_wake(&s->reconnect_sleep);
130
nbd_co_establish_connection_cancel(bs, true);
45
}
131
}
46
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
132
if (qemu_in_coroutine()) {
47
index XXXXXXX..XXXXXXX 100644
133
@@ -XXX,XX +XXX,XX @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
48
--- a/block/throttle-groups.c
134
}
49
+++ b/block/throttle-groups.c
135
bdrv_inc_in_flight(s->bs);
50
@@ -XXX,XX +XXX,XX @@ void throttle_group_attach_aio_context(ThrottleGroupMember *tgm,
136
} else {
51
void throttle_group_detach_aio_context(ThrottleGroupMember *tgm)
137
- qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout,
52
{
138
- &s->connection_co_sleep_ns_state);
53
ThrottleTimers *tt = &tgm->throttle_timers;
139
+ qemu_co_sleep_ns_wakeable(&s->reconnect_sleep,
54
+
140
+ QEMU_CLOCK_REALTIME, timeout);
55
+ /* Requests must have been drained */
141
if (s->drained) {
56
+ assert(tgm->pending_reqs[0] == 0 && tgm->pending_reqs[1] == 0);
142
continue;
57
+ assert(qemu_co_queue_empty(&tgm->throttled_reqs[0]));
143
}
58
+ assert(qemu_co_queue_empty(&tgm->throttled_reqs[1]));
144
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
59
+
145
index XXXXXXX..XXXXXXX 100644
60
throttle_timers_detach_aio_context(tt);
146
--- a/util/qemu-coroutine-sleep.c
61
tgm->aio_context = NULL;
147
+++ b/util/qemu-coroutine-sleep.c
148
@@ -XXX,XX +XXX,XX @@
149
150
static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
151
152
-struct QemuCoSleepState {
153
+void qemu_co_sleep_wake(QemuCoSleep *w)
154
+{
155
Coroutine *co;
156
- QemuCoSleepState **user_state_pointer;
157
-};
158
159
-void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
160
-{
161
- if (sleep_state) {
162
+ co = w->to_wake;
163
+ w->to_wake = NULL;
164
+ if (co) {
165
/* Write of schedule protected by barrier write in aio_co_schedule */
166
- const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
167
+ const char *scheduled = qatomic_cmpxchg(&co->scheduled,
168
qemu_co_sleep_ns__scheduled, NULL);
169
170
assert(scheduled == qemu_co_sleep_ns__scheduled);
171
- *sleep_state->user_state_pointer = NULL;
172
- aio_co_wake(sleep_state->co);
173
+ aio_co_wake(co);
174
}
175
}
176
177
static void co_sleep_cb(void *opaque)
178
{
179
- QemuCoSleepState **sleep_state = opaque;
180
- qemu_co_sleep_wake(*sleep_state);
181
+ QemuCoSleep *w = opaque;
182
+ qemu_co_sleep_wake(w);
183
}
184
185
-void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
186
- QemuCoSleepState **sleep_state)
187
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
188
+ QEMUClockType type, int64_t ns)
189
{
190
+ Coroutine *co = qemu_coroutine_self();
191
AioContext *ctx = qemu_get_current_aio_context();
192
QEMUTimer ts;
193
- QemuCoSleepState state = {
194
- .co = qemu_coroutine_self(),
195
- .user_state_pointer = sleep_state,
196
- };
197
198
- const char *scheduled = qatomic_cmpxchg(&state.co->scheduled, NULL,
199
- qemu_co_sleep_ns__scheduled);
200
+ const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
201
+ qemu_co_sleep_ns__scheduled);
202
if (scheduled) {
203
fprintf(stderr,
204
"%s: Co-routine was already scheduled in '%s'\n",
205
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
206
abort();
207
}
208
209
- aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
210
- *sleep_state = &state;
211
+ w->to_wake = co;
212
+ aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
213
timer_mod(&ts, qemu_clock_get_ns(type) + ns);
214
qemu_coroutine_yield();
215
timer_del(&ts);
216
217
- /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */
218
- assert(*sleep_state == NULL);
219
+ /* w->to_wake is cleared before resuming this coroutine. */
220
+ assert(w->to_wake == NULL);
62
}
221
}
63
--
222
--
64
2.13.6
223
2.31.1
65
224
66
diff view generated by jsdifflib
1
From: Zhengui <lizhengui@huawei.com>
1
From: Paolo Bonzini <pbonzini@redhat.com>
2
2
3
In blk_remove_bs, all I/O should be completed before removing throttle
3
Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake.
4
timers. If there has inflight I/O, removing throttle timers here will
4
This makes the logic of qemu_co_sleep_ns_wakeable easy to understand.
5
cause the inflight I/O never return.
6
This patch add bdrv_drained_begin before throttle_timers_detach_aio_context
7
to let all I/O completed before removing throttle timers.
8
5
9
[Moved declaration of bs as suggested by Alberto Garcia
6
In the future we will introduce an API that can work even if the
10
<berto@igalia.com>.
7
sleep and wake happen from different threads. For now, initializing
11
--Stefan]
8
w->to_wake after timer_mod is fine because the timer can only fire in
9
the same AioContext.
12
10
13
Signed-off-by: Zhengui <lizhengui@huawei.com>
11
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
14
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
12
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
15
Reviewed-by: Alberto Garcia <berto@igalia.com>
13
Message-id: 20210517100548.28806-7-pbonzini@redhat.com
16
Message-id: 1508564040-120700-1-git-send-email-lizhengui@huawei.com
17
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
14
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
18
---
15
---
19
block/block-backend.c | 4 ++++
16
include/qemu/coroutine.h | 5 +++++
20
1 file changed, 4 insertions(+)
17
util/qemu-coroutine-sleep.c | 26 +++++++++++++++++++-------
18
2 files changed, 24 insertions(+), 7 deletions(-)
21
19
22
diff --git a/block/block-backend.c b/block/block-backend.c
20
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
23
index XXXXXXX..XXXXXXX 100644
21
index XXXXXXX..XXXXXXX 100644
24
--- a/block/block-backend.c
22
--- a/include/qemu/coroutine.h
25
+++ b/block/block-backend.c
23
+++ b/include/qemu/coroutine.h
26
@@ -XXX,XX +XXX,XX @@ BlockBackend *blk_by_public(BlockBackendPublic *public)
24
@@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleep {
27
*/
25
void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
28
void blk_remove_bs(BlockBackend *blk)
26
QEMUClockType type, int64_t ns);
27
28
+/**
29
+ * Yield the coroutine until the next call to qemu_co_sleep_wake.
30
+ */
31
+void coroutine_fn qemu_co_sleep(QemuCoSleep *w);
32
+
33
static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
29
{
34
{
30
+ BlockDriverState *bs;
35
QemuCoSleep w = { 0 };
31
ThrottleTimers *tt;
36
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
32
37
index XXXXXXX..XXXXXXX 100644
33
notifier_list_notify(&blk->remove_bs_notifiers, blk);
38
--- a/util/qemu-coroutine-sleep.c
34
if (blk->public.throttle_group_member.throttle_state) {
39
+++ b/util/qemu-coroutine-sleep.c
35
tt = &blk->public.throttle_group_member.throttle_timers;
40
@@ -XXX,XX +XXX,XX @@ static void co_sleep_cb(void *opaque)
36
+ bs = blk_bs(blk);
41
qemu_co_sleep_wake(w);
37
+ bdrv_drained_begin(bs);
42
}
38
throttle_timers_detach_aio_context(tt);
43
39
+ bdrv_drained_end(bs);
44
-void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
45
- QEMUClockType type, int64_t ns)
46
+void coroutine_fn qemu_co_sleep(QemuCoSleep *w)
47
{
48
Coroutine *co = qemu_coroutine_self();
49
- AioContext *ctx = qemu_get_current_aio_context();
50
- QEMUTimer ts;
51
52
const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
53
qemu_co_sleep_ns__scheduled);
54
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
40
}
55
}
41
56
42
blk_update_root_state(blk);
57
w->to_wake = co;
58
- aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
59
- timer_mod(&ts, qemu_clock_get_ns(type) + ns);
60
qemu_coroutine_yield();
61
- timer_del(&ts);
62
63
/* w->to_wake is cleared before resuming this coroutine. */
64
assert(w->to_wake == NULL);
65
}
66
+
67
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
68
+ QEMUClockType type, int64_t ns)
69
+{
70
+ AioContext *ctx = qemu_get_current_aio_context();
71
+ QEMUTimer ts;
72
+
73
+ aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w);
74
+ timer_mod(&ts, qemu_clock_get_ns(type) + ns);
75
+
76
+ /*
77
+ * The timer will fire in the current AiOContext, so the callback
78
+ * must happen after qemu_co_sleep yields and there is no race
79
+ * between timer_mod and qemu_co_sleep.
80
+ */
81
+ qemu_co_sleep(w);
82
+ timer_del(&ts);
83
+}
43
--
84
--
44
2.13.6
85
2.31.1
45
86
46
diff view generated by jsdifflib