1 | The following changes since commit 560009f2c8b57b7cdd31a5693ea86ab369382f49: | 1 | The following changes since commit 6c769690ac845fa62642a5f93b4e4bd906adab95: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2019-10-07 15:40:53 +0100) | 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 | https://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 4d804b5305ffb4d5fa414c38d4f1bdfb987c8d0b: | 9 | for you to fetch changes up to 0a6f0c76a030710780ce10d6347a70f098024d21: |
10 | 10 | ||
11 | iotests/262: Switch source/dest VM launch order (2019-10-08 14:28:25 +0100) | 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 | This pull request also contains the two commits from the previous pull request | 16 | (Resent due to an email preparation mistake.) |
17 | that was dropped due to a mingw compilation error. The compilation should now | ||
18 | be fixed. | ||
19 | 17 | ||
20 | ---------------------------------------------------------------- | 18 | ---------------------------------------------------------------- |
21 | 19 | ||
22 | Max Reitz (2): | 20 | Paolo Bonzini (6): |
23 | block: Skip COR for inactive nodes | 21 | coroutine-sleep: use a stack-allocated timer |
24 | iotests/262: Switch source/dest VM launch order | 22 | coroutine-sleep: disallow NULL QemuCoSleepState** argument |
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 | ||
25 | 28 | ||
26 | Sergio Lopez (1): | 29 | Philippe Mathieu-Daudé (1): |
27 | virtio-blk: schedule virtio_notify_config to run on main context | 30 | bitops.h: Improve find_xxx_bit() documentation |
28 | 31 | ||
29 | Vladimir Sementsov-Ogievskiy (1): | 32 | Zenghui Yu (1): |
30 | util/ioc.c: try to reassure Coverity about qemu_iovec_init_extended | 33 | multi-process: Initialize variables declared with g_auto* |
31 | 34 | ||
32 | block/io.c | 41 +++++++++++++++++++++++++------------- | 35 | include/qemu/bitops.h | 15 ++++++-- |
33 | hw/block/virtio-blk.c | 16 ++++++++++++++- | 36 | include/qemu/coroutine.h | 27 ++++++++----- |
34 | util/iov.c | 5 +++-- | 37 | block/block-copy.c | 10 ++--- |
35 | tests/qemu-iotests/262 | 12 +++++------ | 38 | block/nbd.c | 14 +++---- |
36 | tests/qemu-iotests/262.out | 6 +++--- | 39 | hw/remote/memory.c | 5 +-- |
37 | 5 files changed, 54 insertions(+), 26 deletions(-) | 40 | hw/remote/proxy.c | 3 +- |
41 | util/qemu-coroutine-sleep.c | 75 +++++++++++++++++++------------------ | ||
42 | 7 files changed, 79 insertions(+), 70 deletions(-) | ||
38 | 43 | ||
39 | -- | 44 | -- |
40 | 2.21.0 | 45 | 2.31.1 |
41 | 46 | ||
42 | 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 |
New patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
1 | 2 | ||
3 | Simplify the code by removing conditionals. qemu_co_sleep_ns | ||
4 | can simply point the argument to an on-stack temporary. | ||
5 | |||
6 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
7 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
8 | Message-id: 20210517100548.28806-3-pbonzini@redhat.com | ||
9 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
10 | --- | ||
11 | include/qemu/coroutine.h | 5 +++-- | ||
12 | util/qemu-coroutine-sleep.c | 18 +++++------------- | ||
13 | 2 files changed, 8 insertions(+), 15 deletions(-) | ||
14 | |||
15 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h | ||
16 | index XXXXXXX..XXXXXXX 100644 | ||
17 | --- a/include/qemu/coroutine.h | ||
18 | +++ b/include/qemu/coroutine.h | ||
19 | @@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleepState QemuCoSleepState; | ||
20 | |||
21 | /** | ||
22 | * Yield the coroutine for a given duration. During this yield, @sleep_state | ||
23 | - * (if not NULL) is set to an opaque pointer, which may be used for | ||
24 | + * is set to an opaque pointer, which may be used for | ||
25 | * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the | ||
26 | * timer fires. Don't save the obtained value to other variables and don't call | ||
27 | * qemu_co_sleep_wake from another aio context. | ||
28 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
29 | QemuCoSleepState **sleep_state); | ||
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 | - } | ||
70 | + | ||
71 | + /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */ | ||
72 | + assert(*sleep_state == NULL); | ||
73 | } | ||
74 | -- | ||
75 | 2.31.1 | ||
76 | diff view generated by jsdifflib |
1 | From: Sergio Lopez <slp@redhat.com> | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | virtio_notify_config() needs to acquire the global mutex, which isn't | 3 | All callers of qemu_co_sleep_wake are checking whether they are passing |
4 | allowed from an iothread, and may lead to a deadlock like this: | 4 | a NULL argument inside the pointer-to-pointer: do the check in |
5 | qemu_co_sleep_wake itself. | ||
5 | 6 | ||
6 | - main thead | 7 | As a side effect, qemu_co_sleep_wake can be called more than once and |
7 | * Has acquired: qemu_global_mutex. | 8 | it will only wake the coroutine once; after the first time, the argument |
8 | * Is trying the acquire: iothread AioContext lock via | 9 | will be set to NULL via *sleep_state->user_state_pointer. However, this |
9 | AIO_WAIT_WHILE (after aio_poll). | 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 | - iothread | 13 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
12 | * Has acquired: AioContext lock. | 14 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
13 | * Is trying to acquire: qemu_global_mutex (via | 15 | Message-id: 20210517100548.28806-4-pbonzini@redhat.com |
14 | virtio_notify_config->prepare_mmio_access). | ||
15 | |||
16 | If virtio_blk_resize() is called from an iothread, schedule | ||
17 | virtio_notify_config() to be run in the main context BH. | ||
18 | |||
19 | [Removed unnecessary newline as suggested by Kevin Wolf | ||
20 | <kwolf@redhat.com>. | ||
21 | --Stefan] | ||
22 | |||
23 | Signed-off-by: Sergio Lopez <slp@redhat.com> | ||
24 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
25 | Message-id: 20190916112411.21636-1-slp@redhat.com | ||
26 | Message-Id: <20190916112411.21636-1-slp@redhat.com> | ||
27 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
28 | --- | 17 | --- |
29 | hw/block/virtio-blk.c | 16 +++++++++++++++- | 18 | block/block-copy.c | 4 +--- |
30 | 1 file changed, 15 insertions(+), 1 deletion(-) | 19 | block/nbd.c | 8 ++------ |
20 | util/qemu-coroutine-sleep.c | 21 ++++++++++++--------- | ||
21 | 3 files changed, 15 insertions(+), 18 deletions(-) | ||
31 | 22 | ||
32 | diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c | 23 | diff --git a/block/block-copy.c b/block/block-copy.c |
33 | index XXXXXXX..XXXXXXX 100644 | 24 | index XXXXXXX..XXXXXXX 100644 |
34 | --- a/hw/block/virtio-blk.c | 25 | --- a/block/block-copy.c |
35 | +++ b/hw/block/virtio-blk.c | 26 | +++ b/block/block-copy.c |
36 | @@ -XXX,XX +XXX,XX @@ | 27 | @@ -XXX,XX +XXX,XX @@ out: |
37 | #include "qemu/iov.h" | 28 | |
38 | #include "qemu/module.h" | 29 | void block_copy_kick(BlockCopyCallState *call_state) |
39 | #include "qemu/error-report.h" | 30 | { |
40 | +#include "qemu/main-loop.h" | 31 | - if (call_state->sleep_state) { |
41 | #include "trace.h" | 32 | - qemu_co_sleep_wake(call_state->sleep_state); |
42 | #include "hw/block/block.h" | 33 | - } |
43 | #include "hw/qdev-properties.h" | 34 | + qemu_co_sleep_wake(call_state->sleep_state); |
44 | @@ -XXX,XX +XXX,XX @@ static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f, | ||
45 | return 0; | ||
46 | } | 35 | } |
47 | 36 | ||
48 | +static void virtio_resize_cb(void *opaque) | 37 | /* |
49 | +{ | 38 | diff --git a/block/nbd.c b/block/nbd.c |
50 | + VirtIODevice *vdev = opaque; | 39 | index XXXXXXX..XXXXXXX 100644 |
51 | + | 40 | --- a/block/nbd.c |
52 | + assert(qemu_get_current_aio_context() == qemu_get_aio_context()); | 41 | +++ b/block/nbd.c |
53 | + virtio_notify_config(vdev); | 42 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs) |
54 | +} | 43 | BDRVNBDState *s = (BDRVNBDState *)bs->opaque; |
55 | + | 44 | |
56 | static void virtio_blk_resize(void *opaque) | 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) | ||
57 | { | 71 | { |
58 | VirtIODevice *vdev = VIRTIO_DEVICE(opaque); | 72 | - /* Write of schedule protected by barrier write in aio_co_schedule */ |
59 | 73 | - const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled, | |
60 | - virtio_notify_config(vdev); | 74 | - qemu_co_sleep_ns__scheduled, NULL); |
61 | + /* | 75 | + if (sleep_state) { |
62 | + * virtio_notify_config() needs to acquire the global mutex, | 76 | + /* Write of schedule protected by barrier write in aio_co_schedule */ |
63 | + * so it can't be called from an iothread. Instead, schedule | 77 | + const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled, |
64 | + * it to be run in the main context BH. | 78 | + qemu_co_sleep_ns__scheduled, NULL); |
65 | + */ | 79 | |
66 | + aio_bh_schedule_oneshot(qemu_get_aio_context(), virtio_resize_cb, vdev); | 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); | ||
88 | + } | ||
67 | } | 89 | } |
68 | 90 | ||
69 | static const BlockDevOps virtio_block_ops = { | 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(); | ||
70 | -- | 108 | -- |
71 | 2.21.0 | 109 | 2.31.1 |
72 | 110 | ||
73 | diff view generated by jsdifflib |
1 | From: Max Reitz <mreitz@redhat.com> | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Launching the destination VM before the source VM gives us a regression | 3 | This simplification is enabled by the previous patch. Now aio_co_wake |
4 | test for HEAD^: | 4 | will only be called once, therefore we do not care about a spurious |
5 | firing of the timer after a qemu_co_sleep_wake. | ||
5 | 6 | ||
6 | The guest device causes a read from the disk image through | 7 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
7 | guess_disk_lchs(). This will not work if the first sector (containing | 8 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
8 | the partition table) is yet unallocated, we use COR, and the node is | 9 | Message-id: 20210517100548.28806-5-pbonzini@redhat.com |
9 | inactive. | ||
10 | |||
11 | By launching the source VM before the destination, however, the COR | ||
12 | filter on the source will allocate that area in the image shared between | ||
13 | both VMs, thus the problem will not become apparent. | ||
14 | |||
15 | Switching the launch order causes the sector to still be unallocated | ||
16 | when guess_disk_lchs() runs on the inactive node in the destination VM, | ||
17 | and thus we get our test case. | ||
18 | |||
19 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
20 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
21 | Message-id: 20191001174827.11081-3-mreitz@redhat.com | ||
22 | Message-Id: <20191001174827.11081-3-mreitz@redhat.com> | ||
23 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
24 | --- | 11 | --- |
25 | tests/qemu-iotests/262 | 12 ++++++------ | 12 | util/qemu-coroutine-sleep.c | 8 ++++---- |
26 | tests/qemu-iotests/262.out | 6 +++--- | 13 | 1 file changed, 4 insertions(+), 4 deletions(-) |
27 | 2 files changed, 9 insertions(+), 9 deletions(-) | ||
28 | 14 | ||
29 | diff --git a/tests/qemu-iotests/262 b/tests/qemu-iotests/262 | 15 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c |
30 | index XXXXXXX..XXXXXXX 100755 | ||
31 | --- a/tests/qemu-iotests/262 | ||
32 | +++ b/tests/qemu-iotests/262 | ||
33 | @@ -XXX,XX +XXX,XX @@ with iotests.FilePath('img') as img_path, \ | ||
34 | |||
35 | os.mkfifo(fifo) | ||
36 | |||
37 | - iotests.log('Launching source VM...') | ||
38 | - add_opts(vm_a) | ||
39 | - vm_a.launch() | ||
40 | - | ||
41 | - vm_a.enable_migration_events('A') | ||
42 | - | ||
43 | iotests.log('Launching destination VM...') | ||
44 | add_opts(vm_b) | ||
45 | vm_b.add_incoming("exec: cat '%s'" % (fifo)) | ||
46 | @@ -XXX,XX +XXX,XX @@ with iotests.FilePath('img') as img_path, \ | ||
47 | |||
48 | vm_b.enable_migration_events('B') | ||
49 | |||
50 | + iotests.log('Launching source VM...') | ||
51 | + add_opts(vm_a) | ||
52 | + vm_a.launch() | ||
53 | + | ||
54 | + vm_a.enable_migration_events('A') | ||
55 | + | ||
56 | iotests.log('Starting migration to B...') | ||
57 | iotests.log(vm_a.qmp('migrate', uri='exec:cat >%s' % (fifo))) | ||
58 | with iotests.Timeout(3, 'Migration does not complete'): | ||
59 | diff --git a/tests/qemu-iotests/262.out b/tests/qemu-iotests/262.out | ||
60 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100644 |
61 | --- a/tests/qemu-iotests/262.out | 17 | --- a/util/qemu-coroutine-sleep.c |
62 | +++ b/tests/qemu-iotests/262.out | 18 | +++ b/util/qemu-coroutine-sleep.c |
63 | @@ -XXX,XX +XXX,XX @@ | 19 | @@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns"; |
64 | -Launching source VM... | 20 | |
65 | -Enabling migration QMP events on A... | 21 | struct QemuCoSleepState { |
66 | -{"return": {}} | 22 | Coroutine *co; |
67 | Launching destination VM... | 23 | - QEMUTimer ts; |
68 | Enabling migration QMP events on B... | 24 | QemuCoSleepState **user_state_pointer; |
69 | {"return": {}} | 25 | }; |
70 | +Launching source VM... | 26 | |
71 | +Enabling migration QMP events on A... | 27 | @@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) |
72 | +{"return": {}} | 28 | |
73 | Starting migration to B... | 29 | assert(scheduled == qemu_co_sleep_ns__scheduled); |
74 | {"return": {}} | 30 | *sleep_state->user_state_pointer = NULL; |
75 | {"data": {"status": "setup"}, "event": "MIGRATION", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} | 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) | ||
37 | { | ||
38 | AioContext *ctx = qemu_get_current_aio_context(); | ||
39 | + QEMUTimer ts; | ||
40 | QemuCoSleepState state = { | ||
41 | .co = qemu_coroutine_self(), | ||
42 | .user_state_pointer = sleep_state, | ||
43 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
44 | abort(); | ||
45 | } | ||
46 | |||
47 | - aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state); | ||
48 | + aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state); | ||
49 | *sleep_state = &state; | ||
50 | - timer_mod(&state.ts, qemu_clock_get_ns(type) + ns); | ||
51 | + timer_mod(&ts, qemu_clock_get_ns(type) + ns); | ||
52 | qemu_coroutine_yield(); | ||
53 | + timer_del(&ts); | ||
54 | |||
55 | /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */ | ||
56 | assert(*sleep_state == NULL); | ||
76 | -- | 57 | -- |
77 | 2.21.0 | 58 | 2.31.1 |
78 | 59 | ||
79 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Make it more obvious, that filling qiov corresponds to qiov allocation, | 3 | Right now, users of qemu_co_sleep_ns_wakeable are simply passing |
4 | which in turn corresponds to total_niov calculation, based on mid_niov | 4 | a pointer to QemuCoSleepState by reference to the function. But |
5 | (not mid_len). Still add an assertion to show that there should be no | 5 | QemuCoSleepState really is just a Coroutine*; making the |
6 | difference. | 6 | content of the struct public is just as efficient and lets us |
7 | 7 | skip the user_state_pointer indirection. | |
8 | [Added mingw "error: 'mid_iov' may be used uninitialized in this | 8 | |
9 | function" compiler error fix suggested by Vladimir. | 9 | Since the usage is changed, take the occasion to rename the |
10 | --Stefan] | 10 | struct to QemuCoSleep. |
11 | 11 | ||
12 | Reported-by: Coverity (CID 1405302) | 12 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
13 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 13 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
14 | Message-id: 20190910090310.14032-1-vsementsov@virtuozzo.com | 14 | Message-id: 20210517100548.28806-6-pbonzini@redhat.com |
15 | Suggested-by: Peter Maydell <peter.maydell@linaro.org> | ||
16 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
17 | Message-Id: <20190910090310.14032-1-vsementsov@virtuozzo.com> | ||
18 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 15 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
19 | |||
20 | fixup! util/ioc.c: try to reassure Coverity about qemu_iovec_init_extended | ||
21 | --- | 16 | --- |
22 | util/iov.c | 5 +++-- | 17 | include/qemu/coroutine.h | 23 +++++++++++---------- |
23 | 1 file changed, 3 insertions(+), 2 deletions(-) | 18 | block/block-copy.c | 8 ++++---- |
24 | 19 | block/nbd.c | 10 ++++----- | |
25 | diff --git a/util/iov.c b/util/iov.c | 20 | util/qemu-coroutine-sleep.c | 41 ++++++++++++++++--------------------- |
26 | index XXXXXXX..XXXXXXX 100644 | 21 | 4 files changed, 39 insertions(+), 43 deletions(-) |
27 | --- a/util/iov.c | 22 | |
28 | +++ b/util/iov.c | 23 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h |
29 | @@ -XXX,XX +XXX,XX @@ void qemu_iovec_init_extended( | 24 | index XXXXXXX..XXXXXXX 100644 |
30 | { | 25 | --- a/include/qemu/coroutine.h |
31 | size_t mid_head, mid_tail; | 26 | +++ b/include/qemu/coroutine.h |
32 | int total_niov, mid_niov = 0; | 27 | @@ -XXX,XX +XXX,XX @@ void qemu_co_rwlock_wrlock(CoRwlock *lock); |
33 | - struct iovec *p, *mid_iov; | 28 | */ |
34 | + struct iovec *p, *mid_iov = NULL; | 29 | void qemu_co_rwlock_unlock(CoRwlock *lock); |
35 | 30 | ||
36 | if (mid_len) { | 31 | -typedef struct QemuCoSleepState QemuCoSleepState; |
37 | mid_iov = qiov_slice(mid_qiov, mid_offset, mid_len, | 32 | +typedef struct QemuCoSleep { |
38 | @@ -XXX,XX +XXX,XX @@ void qemu_iovec_init_extended( | 33 | + Coroutine *to_wake; |
39 | p++; | 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); | ||
40 | } | 131 | } |
41 | 132 | if (qemu_in_coroutine()) { | |
42 | - if (mid_len) { | 133 | @@ -XXX,XX +XXX,XX @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s) |
43 | + assert(!mid_niov == !mid_len); | 134 | } |
44 | + if (mid_niov) { | 135 | bdrv_inc_in_flight(s->bs); |
45 | memcpy(p, mid_iov, mid_niov * sizeof(*p)); | 136 | } else { |
46 | p[0].iov_base = (uint8_t *)p[0].iov_base + mid_head; | 137 | - qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout, |
47 | p[0].iov_len -= mid_head; | 138 | - &s->connection_co_sleep_ns_state); |
139 | + qemu_co_sleep_ns_wakeable(&s->reconnect_sleep, | ||
140 | + QEMU_CLOCK_REALTIME, timeout); | ||
141 | if (s->drained) { | ||
142 | continue; | ||
143 | } | ||
144 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c | ||
145 | index XXXXXXX..XXXXXXX 100644 | ||
146 | --- a/util/qemu-coroutine-sleep.c | ||
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); | ||
221 | } | ||
48 | -- | 222 | -- |
49 | 2.21.0 | 223 | 2.31.1 |
50 | 224 | ||
51 | diff view generated by jsdifflib |
1 | From: Max Reitz <mreitz@redhat.com> | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | We must not write data to inactive nodes, and a COR is certainly | 3 | Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake. |
4 | something we can simply not do without upsetting anyone. So skip COR | 4 | This makes the logic of qemu_co_sleep_ns_wakeable easy to understand. |
5 | operations on inactive nodes. | ||
6 | 5 | ||
7 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 6 | In the future we will introduce an API that can work even if the |
8 | Reviewed-by: Eric Blake <eblake@redhat.com> | 7 | sleep and wake happen from different threads. For now, initializing |
9 | Message-id: 20191001174827.11081-2-mreitz@redhat.com | 8 | w->to_wake after timer_mod is fine because the timer can only fire in |
10 | Message-Id: <20191001174827.11081-2-mreitz@redhat.com> | 9 | the same AioContext. |
10 | |||
11 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
12 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
13 | Message-id: 20210517100548.28806-7-pbonzini@redhat.com | ||
11 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 14 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
12 | --- | 15 | --- |
13 | block/io.c | 41 +++++++++++++++++++++++++++-------------- | 16 | include/qemu/coroutine.h | 5 +++++ |
14 | 1 file changed, 27 insertions(+), 14 deletions(-) | 17 | util/qemu-coroutine-sleep.c | 26 +++++++++++++++++++------- |
18 | 2 files changed, 24 insertions(+), 7 deletions(-) | ||
15 | 19 | ||
16 | diff --git a/block/io.c b/block/io.c | 20 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h |
17 | index XXXXXXX..XXXXXXX 100644 | 21 | index XXXXXXX..XXXXXXX 100644 |
18 | --- a/block/io.c | 22 | --- a/include/qemu/coroutine.h |
19 | +++ b/block/io.c | 23 | +++ b/include/qemu/coroutine.h |
20 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child, | 24 | @@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleep { |
21 | int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer, | 25 | void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, |
22 | BDRV_REQUEST_MAX_BYTES); | 26 | QEMUClockType type, int64_t ns); |
23 | unsigned int progress = 0; | 27 | |
24 | + bool skip_write; | 28 | +/** |
25 | 29 | + * Yield the coroutine until the next call to qemu_co_sleep_wake. | |
26 | if (!drv) { | 30 | + */ |
27 | return -ENOMEDIUM; | 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) | ||
34 | { | ||
35 | QemuCoSleep w = { 0 }; | ||
36 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c | ||
37 | index XXXXXXX..XXXXXXX 100644 | ||
38 | --- a/util/qemu-coroutine-sleep.c | ||
39 | +++ b/util/qemu-coroutine-sleep.c | ||
40 | @@ -XXX,XX +XXX,XX @@ static void co_sleep_cb(void *opaque) | ||
41 | qemu_co_sleep_wake(w); | ||
42 | } | ||
43 | |||
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, | ||
28 | } | 55 | } |
29 | 56 | ||
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 | + | ||
30 | + /* | 76 | + /* |
31 | + * Do not write anything when the BDS is inactive. That is not | 77 | + * The timer will fire in the current AiOContext, so the callback |
32 | + * allowed, and it would not help. | 78 | + * must happen after qemu_co_sleep yields and there is no race |
79 | + * between timer_mod and qemu_co_sleep. | ||
33 | + */ | 80 | + */ |
34 | + skip_write = (bs->open_flags & BDRV_O_INACTIVE); | 81 | + qemu_co_sleep(w); |
35 | + | 82 | + timer_del(&ts); |
36 | /* FIXME We cannot require callers to have write permissions when all they | 83 | +} |
37 | * are doing is a read request. If we did things right, write permissions | ||
38 | * would be obtained anyway, but internally by the copy-on-read code. As | ||
39 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child, | ||
40 | while (cluster_bytes) { | ||
41 | int64_t pnum; | ||
42 | |||
43 | - ret = bdrv_is_allocated(bs, cluster_offset, | ||
44 | - MIN(cluster_bytes, max_transfer), &pnum); | ||
45 | - if (ret < 0) { | ||
46 | - /* Safe to treat errors in querying allocation as if | ||
47 | - * unallocated; we'll probably fail again soon on the | ||
48 | - * read, but at least that will set a decent errno. | ||
49 | - */ | ||
50 | + if (skip_write) { | ||
51 | + ret = 1; /* "already allocated", so nothing will be copied */ | ||
52 | pnum = MIN(cluster_bytes, max_transfer); | ||
53 | - } | ||
54 | + } else { | ||
55 | + ret = bdrv_is_allocated(bs, cluster_offset, | ||
56 | + MIN(cluster_bytes, max_transfer), &pnum); | ||
57 | + if (ret < 0) { | ||
58 | + /* | ||
59 | + * Safe to treat errors in querying allocation as if | ||
60 | + * unallocated; we'll probably fail again soon on the | ||
61 | + * read, but at least that will set a decent errno. | ||
62 | + */ | ||
63 | + pnum = MIN(cluster_bytes, max_transfer); | ||
64 | + } | ||
65 | |||
66 | - /* Stop at EOF if the image ends in the middle of the cluster */ | ||
67 | - if (ret == 0 && pnum == 0) { | ||
68 | - assert(progress >= bytes); | ||
69 | - break; | ||
70 | - } | ||
71 | + /* Stop at EOF if the image ends in the middle of the cluster */ | ||
72 | + if (ret == 0 && pnum == 0) { | ||
73 | + assert(progress >= bytes); | ||
74 | + break; | ||
75 | + } | ||
76 | |||
77 | - assert(skip_bytes < pnum); | ||
78 | + assert(skip_bytes < pnum); | ||
79 | + } | ||
80 | |||
81 | if (ret <= 0) { | ||
82 | QEMUIOVector local_qiov; | ||
83 | -- | 84 | -- |
84 | 2.21.0 | 85 | 2.31.1 |
85 | 86 | ||
86 | diff view generated by jsdifflib |