1 | The following changes since commit 22dbfdecc3c52228d3489da3fe81da92b21197bf: | 1 | The following changes since commit ac5f7bf8e208cd7893dbb1a9520559e569a4677c: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/awilliam/tags/vfio-update-20191010.0' into staging (2019-10-14 15:09:08 +0100) | 3 | Merge tag 'migration-20230424-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-04-24 15:00:39 +0100) |
4 | 4 | ||
5 | are available in the Git repository at: | 5 | are available in the Git repository at: |
6 | 6 | ||
7 | git://repo.or.cz/qemu/kevin.git tags/for-upstream | 7 | https://repo.or.cz/qemu/kevin.git tags/for-upstream |
8 | 8 | ||
9 | for you to fetch changes up to a1406a9262a087d9ec9627b88da13c4590b61dae: | 9 | for you to fetch changes up to 8c1e8fb2e7fc2cbeb57703e143965a4cd3ad301a: |
10 | 10 | ||
11 | iotests: Test large write request to qcow2 file (2019-10-14 17:12:48 +0200) | 11 | block/monitor: Fix crash when executing HMP commit (2023-04-25 15:11:57 +0200) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Block layer patches: | 14 | Block layer patches |
15 | 15 | ||
16 | - block: Fix crash with qcow2 partial cluster COW with small cluster | 16 | - Protect BlockBackend.queued_requests with its own lock |
17 | sizes (misaligned write requests with BDRV_REQ_NO_FALLBACK) | 17 | - Switch to AIO_WAIT_WHILE_UNLOCKED() where possible |
18 | - qcow2: Fix integer overflow potentially causing corruption with huge | 18 | - AioContext removal: LinuxAioState/LuringState/ThreadPool |
19 | requests | 19 | - Add more coroutine_fn annotations, use bdrv/blk_co_* |
20 | - vhdx: Detect truncated image files | 20 | - Fix crash when execute hmp_commit |
21 | - tools: Support help options for --object | ||
22 | - Various block-related replay improvements | ||
23 | - iotests/028: Fix for long $TEST_DIRs | ||
24 | 21 | ||
25 | ---------------------------------------------------------------- | 22 | ---------------------------------------------------------------- |
26 | Alberto Garcia (1): | 23 | Emanuele Giuseppe Esposito (4): |
27 | block: Reject misaligned write requests with BDRV_REQ_NO_FALLBACK | 24 | linux-aio: use LinuxAioState from the running thread |
25 | io_uring: use LuringState from the running thread | ||
26 | thread-pool: use ThreadPool from the running thread | ||
27 | thread-pool: avoid passing the pool parameter every time | ||
28 | 28 | ||
29 | Kevin Wolf (4): | 29 | Paolo Bonzini (9): |
30 | vl: Split off user_creatable_print_help() | 30 | vvfat: mark various functions as coroutine_fn |
31 | qemu-io: Support help options for --object | 31 | blkdebug: add missing coroutine_fn annotation |
32 | qemu-img: Support help options for --object | 32 | mirror: make mirror_flush a coroutine_fn, do not use co_wrappers |
33 | qemu-nbd: Support help options for --object | 33 | nbd: mark more coroutine_fns, do not use co_wrappers |
34 | 9pfs: mark more coroutine_fns | ||
35 | qemu-pr-helper: mark more coroutine_fns | ||
36 | tests: mark more coroutine_fns | ||
37 | qcow2: mark various functions as coroutine_fn and GRAPH_RDLOCK | ||
38 | vmdk: make vmdk_is_cid_valid a coroutine_fn | ||
34 | 39 | ||
35 | Max Reitz (3): | 40 | Stefan Hajnoczi (10): |
36 | iotests/028: Fix for long $TEST_DIRs | 41 | block: make BlockBackend->quiesce_counter atomic |
37 | qcow2: Limit total allocation range to INT_MAX | 42 | block: make BlockBackend->disable_request_queuing atomic |
38 | iotests: Test large write request to qcow2 file | 43 | block: protect BlockBackend->queued_requests with a lock |
44 | block: don't acquire AioContext lock in bdrv_drain_all() | ||
45 | block: convert blk_exp_close_all_type() to AIO_WAIT_WHILE_UNLOCKED() | ||
46 | block: convert bdrv_graph_wrlock() to AIO_WAIT_WHILE_UNLOCKED() | ||
47 | block: convert bdrv_drain_all_begin() to AIO_WAIT_WHILE_UNLOCKED() | ||
48 | hmp: convert handle_hmp_command() to AIO_WAIT_WHILE_UNLOCKED() | ||
49 | monitor: convert monitor_cleanup() to AIO_WAIT_WHILE_UNLOCKED() | ||
50 | block: add missing coroutine_fn to bdrv_sum_allocated_file_size() | ||
39 | 51 | ||
40 | Pavel Dovgaluk (6): | 52 | Wang Liang (1): |
41 | block: implement bdrv_snapshot_goto for blkreplay | 53 | block/monitor: Fix crash when executing HMP commit |
42 | replay: disable default snapshot for record/replay | ||
43 | replay: update docs for record/replay with block devices | ||
44 | replay: don't drain/flush bdrv queue while RR is working | ||
45 | replay: finish record/replay before closing the disks | ||
46 | replay: add BH oneshot event for block layer | ||
47 | 54 | ||
48 | Peter Lieven (1): | 55 | Wilfred Mallawa (1): |
49 | block/vhdx: add check for truncated image files | 56 | include/block: fixup typos |
50 | 57 | ||
51 | docs/replay.txt | 12 +++- | 58 | block/qcow2.h | 15 +++++----- |
52 | include/qom/object_interfaces.h | 12 ++++ | 59 | hw/9pfs/9p.h | 4 +-- |
53 | include/sysemu/replay.h | 4 ++ | 60 | include/block/aio-wait.h | 2 +- |
54 | replay/replay-internal.h | 1 + | 61 | include/block/aio.h | 8 ------ |
55 | block/blkreplay.c | 8 +++ | 62 | include/block/block_int-common.h | 2 +- |
56 | block/block-backend.c | 9 ++- | 63 | include/block/raw-aio.h | 33 +++++++++++++++------- |
57 | block/io.c | 39 ++++++++++++- | 64 | include/block/thread-pool.h | 15 ++++++---- |
58 | block/iscsi.c | 5 +- | 65 | include/sysemu/block-backend-io.h | 5 ++++ |
59 | block/nfs.c | 6 +- | 66 | backends/tpm/tpm_backend.c | 4 +-- |
60 | block/null.c | 4 +- | 67 | block.c | 2 +- |
61 | block/nvme.c | 6 +- | 68 | block/blkdebug.c | 4 +-- |
62 | block/qcow2-cluster.c | 5 +- | 69 | block/block-backend.c | 45 ++++++++++++++++++------------ |
63 | block/rbd.c | 5 +- | 70 | block/export/export.c | 2 +- |
64 | block/vhdx.c | 120 ++++++++++++++++++++++++++++++++++------ | 71 | block/file-posix.c | 45 ++++++++++++------------------ |
65 | block/vxhs.c | 5 +- | 72 | block/file-win32.c | 4 +-- |
66 | cpus.c | 2 - | 73 | block/graph-lock.c | 2 +- |
67 | qemu-img.c | 34 +++++++----- | 74 | block/io.c | 2 +- |
68 | qemu-io.c | 9 ++- | 75 | block/io_uring.c | 23 ++++++++++------ |
69 | qemu-nbd.c | 9 ++- | 76 | block/linux-aio.c | 29 ++++++++++++-------- |
70 | qom/object_interfaces.c | 61 ++++++++++++++++++++ | 77 | block/mirror.c | 4 +-- |
71 | replay/replay-events.c | 16 ++++++ | 78 | block/monitor/block-hmp-cmds.c | 10 ++++--- |
72 | replay/replay.c | 2 + | 79 | block/qcow2-bitmap.c | 2 +- |
73 | stubs/replay-user.c | 9 +++ | 80 | block/qcow2-cluster.c | 21 ++++++++------ |
74 | vl.c | 63 ++++----------------- | 81 | block/qcow2-refcount.c | 8 +++--- |
75 | stubs/Makefile.objs | 1 + | 82 | block/qcow2-snapshot.c | 25 +++++++++-------- |
76 | tests/qemu-iotests/028 | 11 +++- | 83 | block/qcow2-threads.c | 3 +- |
77 | tests/qemu-iotests/028.out | 1 - | 84 | block/qcow2.c | 27 +++++++++--------- |
78 | tests/qemu-iotests/268 | 55 ++++++++++++++++++ | 85 | block/vmdk.c | 2 +- |
79 | tests/qemu-iotests/268.out | 7 +++ | 86 | block/vvfat.c | 58 ++++++++++++++++++++------------------- |
80 | tests/qemu-iotests/270 | 83 +++++++++++++++++++++++++++ | 87 | hw/9pfs/codir.c | 6 ++-- |
81 | tests/qemu-iotests/270.out | 9 +++ | 88 | hw/9pfs/coth.c | 3 +- |
82 | tests/qemu-iotests/group | 2 + | 89 | hw/ppc/spapr_nvdimm.c | 6 ++-- |
83 | 32 files changed, 504 insertions(+), 111 deletions(-) | 90 | hw/virtio/virtio-pmem.c | 3 +- |
84 | create mode 100644 stubs/replay-user.c | 91 | monitor/hmp.c | 2 +- |
85 | create mode 100755 tests/qemu-iotests/268 | 92 | monitor/monitor.c | 4 +-- |
86 | create mode 100644 tests/qemu-iotests/268.out | 93 | nbd/server.c | 48 ++++++++++++++++---------------- |
87 | create mode 100755 tests/qemu-iotests/270 | 94 | scsi/pr-manager.c | 3 +- |
88 | create mode 100644 tests/qemu-iotests/270.out | 95 | scsi/qemu-pr-helper.c | 25 ++++++++--------- |
89 | 96 | tests/unit/test-thread-pool.c | 14 ++++------ | |
97 | util/thread-pool.c | 25 ++++++++--------- | ||
98 | 40 files changed, 283 insertions(+), 262 deletions(-) | diff view generated by jsdifflib |
1 | From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> | 1 | From: Stefan Hajnoczi <stefanha@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Replay is capable of recording normal BH events, but sometimes | 3 | The main loop thread increments/decrements BlockBackend->quiesce_counter |
4 | there are single use callbacks scheduled with aio_bh_schedule_oneshot | 4 | when drained sections begin/end. The counter is read in the I/O code |
5 | function. This patch enables recording and replaying such callbacks. | 5 | path. Therefore this field is used to communicate between threads |
6 | Block layer uses these events for calling the completion function. | 6 | without a lock. |
7 | Replaying these calls makes the execution deterministic. | ||
8 | 7 | ||
9 | Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> | 8 | Acquire/release are not necessary because the BlockBackend->in_flight |
10 | Acked-by: Kevin Wolf <kwolf@redhat.com> | 9 | counter already uses sequentially consistent accesses and running I/O |
10 | requests hold that counter when blk_wait_while_drained() is called. | ||
11 | qatomic_read() can be used. | ||
12 | |||
13 | Use qatomic_fetch_inc()/qatomic_fetch_dec() for modifications even | ||
14 | though sequentially consistent atomic accesses are not strictly required | ||
15 | here. They are, however, nicer to read than multiple calls to | ||
16 | qatomic_read() and qatomic_set(). Since beginning and ending drain is | ||
17 | not a hot path the extra cost doesn't matter. | ||
18 | |||
19 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
20 | Message-Id: <20230307210427.269214-2-stefanha@redhat.com> | ||
21 | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> | ||
22 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 23 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
12 | --- | 24 | --- |
13 | include/sysemu/replay.h | 4 ++++ | 25 | block/block-backend.c | 14 +++++++------- |
14 | replay/replay-internal.h | 1 + | 26 | 1 file changed, 7 insertions(+), 7 deletions(-) |
15 | block/block-backend.c | 9 ++++++--- | ||
16 | block/io.c | 4 ++-- | ||
17 | block/iscsi.c | 5 +++-- | ||
18 | block/nfs.c | 6 ++++-- | ||
19 | block/null.c | 4 +++- | ||
20 | block/nvme.c | 6 ++++-- | ||
21 | block/rbd.c | 5 +++-- | ||
22 | block/vxhs.c | 5 +++-- | ||
23 | replay/replay-events.c | 16 ++++++++++++++++ | ||
24 | stubs/replay-user.c | 9 +++++++++ | ||
25 | stubs/Makefile.objs | 1 + | ||
26 | 13 files changed, 59 insertions(+), 16 deletions(-) | ||
27 | create mode 100644 stubs/replay-user.c | ||
28 | 27 | ||
29 | diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h | ||
30 | index XXXXXXX..XXXXXXX 100644 | ||
31 | --- a/include/sysemu/replay.h | ||
32 | +++ b/include/sysemu/replay.h | ||
33 | @@ -XXX,XX +XXX,XX @@ | ||
34 | #include "qapi/qapi-types-misc.h" | ||
35 | #include "qapi/qapi-types-run-state.h" | ||
36 | #include "qapi/qapi-types-ui.h" | ||
37 | +#include "block/aio.h" | ||
38 | |||
39 | /* replay clock kinds */ | ||
40 | enum ReplayClockKind { | ||
41 | @@ -XXX,XX +XXX,XX @@ void replay_enable_events(void); | ||
42 | bool replay_events_enabled(void); | ||
43 | /*! Adds bottom half event to the queue */ | ||
44 | void replay_bh_schedule_event(QEMUBH *bh); | ||
45 | +/* Adds oneshot bottom half event to the queue */ | ||
46 | +void replay_bh_schedule_oneshot_event(AioContext *ctx, | ||
47 | + QEMUBHFunc *cb, void *opaque); | ||
48 | /*! Adds input event to the queue */ | ||
49 | void replay_input_event(QemuConsole *src, InputEvent *evt); | ||
50 | /*! Adds input sync event to the queue */ | ||
51 | diff --git a/replay/replay-internal.h b/replay/replay-internal.h | ||
52 | index XXXXXXX..XXXXXXX 100644 | ||
53 | --- a/replay/replay-internal.h | ||
54 | +++ b/replay/replay-internal.h | ||
55 | @@ -XXX,XX +XXX,XX @@ enum ReplayEvents { | ||
56 | |||
57 | enum ReplayAsyncEventKind { | ||
58 | REPLAY_ASYNC_EVENT_BH, | ||
59 | + REPLAY_ASYNC_EVENT_BH_ONESHOT, | ||
60 | REPLAY_ASYNC_EVENT_INPUT, | ||
61 | REPLAY_ASYNC_EVENT_INPUT_SYNC, | ||
62 | REPLAY_ASYNC_EVENT_CHAR_READ, | ||
63 | diff --git a/block/block-backend.c b/block/block-backend.c | 28 | diff --git a/block/block-backend.c b/block/block-backend.c |
64 | index XXXXXXX..XXXXXXX 100644 | 29 | index XXXXXXX..XXXXXXX 100644 |
65 | --- a/block/block-backend.c | 30 | --- a/block/block-backend.c |
66 | +++ b/block/block-backend.c | 31 | +++ b/block/block-backend.c |
67 | @@ -XXX,XX +XXX,XX @@ | 32 | @@ -XXX,XX +XXX,XX @@ struct BlockBackend { |
68 | #include "hw/qdev-core.h" | 33 | NotifierList remove_bs_notifiers, insert_bs_notifiers; |
69 | #include "sysemu/blockdev.h" | 34 | QLIST_HEAD(, BlockBackendAioNotifier) aio_notifiers; |
70 | #include "sysemu/runstate.h" | 35 | |
71 | +#include "sysemu/sysemu.h" | 36 | - int quiesce_counter; |
72 | +#include "sysemu/replay.h" | 37 | + int quiesce_counter; /* atomic: written under BQL, read by other threads */ |
73 | #include "qapi/error.h" | 38 | CoQueue queued_requests; |
74 | #include "qapi/qapi-events-block.h" | 39 | bool disable_request_queuing; |
75 | #include "qemu/id.h" | 40 | |
76 | @@ -XXX,XX +XXX,XX @@ BlockAIOCB *blk_abort_aio_request(BlockBackend *blk, | 41 | @@ -XXX,XX +XXX,XX @@ void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops, |
77 | acb->blk = blk; | 42 | blk->dev_opaque = opaque; |
78 | acb->ret = ret; | 43 | |
79 | 44 | /* Are we currently quiesced? Should we enforce this right now? */ | |
80 | - aio_bh_schedule_oneshot(blk_get_aio_context(blk), error_callback_bh, acb); | 45 | - if (blk->quiesce_counter && ops && ops->drained_begin) { |
81 | + replay_bh_schedule_oneshot_event(blk_get_aio_context(blk), | 46 | + if (qatomic_read(&blk->quiesce_counter) && ops && ops->drained_begin) { |
82 | + error_callback_bh, acb); | 47 | ops->drained_begin(opaque); |
83 | return &acb->common; | ||
84 | } | ||
85 | |||
86 | @@ -XXX,XX +XXX,XX @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes, | ||
87 | |||
88 | acb->has_returned = true; | ||
89 | if (acb->rwco.ret != NOT_DONE) { | ||
90 | - aio_bh_schedule_oneshot(blk_get_aio_context(blk), | ||
91 | - blk_aio_complete_bh, acb); | ||
92 | + replay_bh_schedule_oneshot_event(blk_get_aio_context(blk), | ||
93 | + blk_aio_complete_bh, acb); | ||
94 | } | ||
95 | |||
96 | return &acb->common; | ||
97 | diff --git a/block/io.c b/block/io.c | ||
98 | index XXXXXXX..XXXXXXX 100644 | ||
99 | --- a/block/io.c | ||
100 | +++ b/block/io.c | ||
101 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs, | ||
102 | if (bs) { | ||
103 | bdrv_inc_in_flight(bs); | ||
104 | } | ||
105 | - aio_bh_schedule_oneshot(bdrv_get_aio_context(bs), | ||
106 | - bdrv_co_drain_bh_cb, &data); | ||
107 | + replay_bh_schedule_oneshot_event(bdrv_get_aio_context(bs), | ||
108 | + bdrv_co_drain_bh_cb, &data); | ||
109 | |||
110 | qemu_coroutine_yield(); | ||
111 | /* If we are resumed from some other event (such as an aio completion or a | ||
112 | diff --git a/block/iscsi.c b/block/iscsi.c | ||
113 | index XXXXXXX..XXXXXXX 100644 | ||
114 | --- a/block/iscsi.c | ||
115 | +++ b/block/iscsi.c | ||
116 | @@ -XXX,XX +XXX,XX @@ | ||
117 | #include "qemu/module.h" | ||
118 | #include "qemu/option.h" | ||
119 | #include "qemu/uuid.h" | ||
120 | +#include "sysemu/replay.h" | ||
121 | #include "qapi/error.h" | ||
122 | #include "qapi/qapi-commands-misc.h" | ||
123 | #include "qapi/qmp/qdict.h" | ||
124 | @@ -XXX,XX +XXX,XX @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, | ||
125 | } | ||
126 | |||
127 | if (iTask->co) { | ||
128 | - aio_bh_schedule_oneshot(iTask->iscsilun->aio_context, | ||
129 | - iscsi_co_generic_bh_cb, iTask); | ||
130 | + replay_bh_schedule_oneshot_event(iTask->iscsilun->aio_context, | ||
131 | + iscsi_co_generic_bh_cb, iTask); | ||
132 | } else { | ||
133 | iTask->complete = 1; | ||
134 | } | ||
135 | diff --git a/block/nfs.c b/block/nfs.c | ||
136 | index XXXXXXX..XXXXXXX 100644 | ||
137 | --- a/block/nfs.c | ||
138 | +++ b/block/nfs.c | ||
139 | @@ -XXX,XX +XXX,XX @@ | ||
140 | #include "qemu/option.h" | ||
141 | #include "qemu/uri.h" | ||
142 | #include "qemu/cutils.h" | ||
143 | +#include "sysemu/sysemu.h" | ||
144 | +#include "sysemu/replay.h" | ||
145 | #include "qapi/qapi-visit-block-core.h" | ||
146 | #include "qapi/qmp/qdict.h" | ||
147 | #include "qapi/qmp/qstring.h" | ||
148 | @@ -XXX,XX +XXX,XX @@ nfs_co_generic_cb(int ret, struct nfs_context *nfs, void *data, | ||
149 | if (task->ret < 0) { | ||
150 | error_report("NFS Error: %s", nfs_get_error(nfs)); | ||
151 | } | ||
152 | - aio_bh_schedule_oneshot(task->client->aio_context, | ||
153 | - nfs_co_generic_bh_cb, task); | ||
154 | + replay_bh_schedule_oneshot_event(task->client->aio_context, | ||
155 | + nfs_co_generic_bh_cb, task); | ||
156 | } | ||
157 | |||
158 | static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, uint64_t offset, | ||
159 | diff --git a/block/null.c b/block/null.c | ||
160 | index XXXXXXX..XXXXXXX 100644 | ||
161 | --- a/block/null.c | ||
162 | +++ b/block/null.c | ||
163 | @@ -XXX,XX +XXX,XX @@ | ||
164 | #include "qemu/module.h" | ||
165 | #include "qemu/option.h" | ||
166 | #include "block/block_int.h" | ||
167 | +#include "sysemu/replay.h" | ||
168 | |||
169 | #define NULL_OPT_LATENCY "latency-ns" | ||
170 | #define NULL_OPT_ZEROES "read-zeroes" | ||
171 | @@ -XXX,XX +XXX,XX @@ static inline BlockAIOCB *null_aio_common(BlockDriverState *bs, | ||
172 | timer_mod_ns(&acb->timer, | ||
173 | qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + s->latency_ns); | ||
174 | } else { | ||
175 | - aio_bh_schedule_oneshot(bdrv_get_aio_context(bs), null_bh_cb, acb); | ||
176 | + replay_bh_schedule_oneshot_event(bdrv_get_aio_context(bs), | ||
177 | + null_bh_cb, acb); | ||
178 | } | ||
179 | return &acb->common; | ||
180 | } | ||
181 | diff --git a/block/nvme.c b/block/nvme.c | ||
182 | index XXXXXXX..XXXXXXX 100644 | ||
183 | --- a/block/nvme.c | ||
184 | +++ b/block/nvme.c | ||
185 | @@ -XXX,XX +XXX,XX @@ | ||
186 | #include "qemu/option.h" | ||
187 | #include "qemu/vfio-helpers.h" | ||
188 | #include "block/block_int.h" | ||
189 | +#include "sysemu/replay.h" | ||
190 | #include "trace.h" | ||
191 | |||
192 | #include "block/nvme.h" | ||
193 | @@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q) | ||
194 | smp_mb_release(); | ||
195 | *q->cq.doorbell = cpu_to_le32(q->cq.head); | ||
196 | if (!qemu_co_queue_empty(&q->free_req_queue)) { | ||
197 | - aio_bh_schedule_oneshot(s->aio_context, nvme_free_req_queue_cb, q); | ||
198 | + replay_bh_schedule_oneshot_event(s->aio_context, | ||
199 | + nvme_free_req_queue_cb, q); | ||
200 | } | ||
201 | } | ||
202 | q->busy = false; | ||
203 | @@ -XXX,XX +XXX,XX @@ static void nvme_rw_cb(void *opaque, int ret) | ||
204 | /* The rw coroutine hasn't yielded, don't try to enter. */ | ||
205 | return; | ||
206 | } | ||
207 | - aio_bh_schedule_oneshot(data->ctx, nvme_rw_cb_bh, data); | ||
208 | + replay_bh_schedule_oneshot_event(data->ctx, nvme_rw_cb_bh, data); | ||
209 | } | ||
210 | |||
211 | static coroutine_fn int nvme_co_prw_aligned(BlockDriverState *bs, | ||
212 | diff --git a/block/rbd.c b/block/rbd.c | ||
213 | index XXXXXXX..XXXXXXX 100644 | ||
214 | --- a/block/rbd.c | ||
215 | +++ b/block/rbd.c | ||
216 | @@ -XXX,XX +XXX,XX @@ | ||
217 | #include "block/qdict.h" | ||
218 | #include "crypto/secret.h" | ||
219 | #include "qemu/cutils.h" | ||
220 | +#include "sysemu/replay.h" | ||
221 | #include "qapi/qmp/qstring.h" | ||
222 | #include "qapi/qmp/qdict.h" | ||
223 | #include "qapi/qmp/qjson.h" | ||
224 | @@ -XXX,XX +XXX,XX @@ static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB *rcb) | ||
225 | rcb->ret = rbd_aio_get_return_value(c); | ||
226 | rbd_aio_release(c); | ||
227 | |||
228 | - aio_bh_schedule_oneshot(bdrv_get_aio_context(acb->common.bs), | ||
229 | - rbd_finish_bh, rcb); | ||
230 | + replay_bh_schedule_oneshot_event(bdrv_get_aio_context(acb->common.bs), | ||
231 | + rbd_finish_bh, rcb); | ||
232 | } | ||
233 | |||
234 | static int rbd_aio_discard_wrapper(rbd_image_t image, | ||
235 | diff --git a/block/vxhs.c b/block/vxhs.c | ||
236 | index XXXXXXX..XXXXXXX 100644 | ||
237 | --- a/block/vxhs.c | ||
238 | +++ b/block/vxhs.c | ||
239 | @@ -XXX,XX +XXX,XX @@ | ||
240 | #include "qapi/error.h" | ||
241 | #include "qemu/uuid.h" | ||
242 | #include "crypto/tlscredsx509.h" | ||
243 | +#include "sysemu/replay.h" | ||
244 | |||
245 | #define VXHS_OPT_FILENAME "filename" | ||
246 | #define VXHS_OPT_VDISK_ID "vdisk-id" | ||
247 | @@ -XXX,XX +XXX,XX @@ static void vxhs_iio_callback(void *ctx, uint32_t opcode, uint32_t error) | ||
248 | trace_vxhs_iio_callback(error); | ||
249 | } | ||
250 | |||
251 | - aio_bh_schedule_oneshot(bdrv_get_aio_context(acb->common.bs), | ||
252 | - vxhs_complete_aio_bh, acb); | ||
253 | + replay_bh_schedule_oneshot_event(bdrv_get_aio_context(acb->common.bs), | ||
254 | + vxhs_complete_aio_bh, acb); | ||
255 | break; | ||
256 | |||
257 | default: | ||
258 | diff --git a/replay/replay-events.c b/replay/replay-events.c | ||
259 | index XXXXXXX..XXXXXXX 100644 | ||
260 | --- a/replay/replay-events.c | ||
261 | +++ b/replay/replay-events.c | ||
262 | @@ -XXX,XX +XXX,XX @@ static void replay_run_event(Event *event) | ||
263 | case REPLAY_ASYNC_EVENT_BH: | ||
264 | aio_bh_call(event->opaque); | ||
265 | break; | ||
266 | + case REPLAY_ASYNC_EVENT_BH_ONESHOT: | ||
267 | + ((QEMUBHFunc *)event->opaque)(event->opaque2); | ||
268 | + break; | ||
269 | case REPLAY_ASYNC_EVENT_INPUT: | ||
270 | qemu_input_event_send_impl(NULL, (InputEvent *)event->opaque); | ||
271 | qapi_free_InputEvent((InputEvent *)event->opaque); | ||
272 | @@ -XXX,XX +XXX,XX @@ void replay_bh_schedule_event(QEMUBH *bh) | ||
273 | } | 48 | } |
274 | } | 49 | } |
275 | 50 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn blk_wait_while_drained(BlockBackend *blk) | |
276 | +void replay_bh_schedule_oneshot_event(AioContext *ctx, | ||
277 | + QEMUBHFunc *cb, void *opaque) | ||
278 | +{ | ||
279 | + if (events_enabled) { | ||
280 | + uint64_t id = replay_get_current_icount(); | ||
281 | + replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id); | ||
282 | + } else { | ||
283 | + aio_bh_schedule_oneshot(ctx, cb, opaque); | ||
284 | + } | ||
285 | +} | ||
286 | + | ||
287 | void replay_add_input_event(struct InputEvent *event) | ||
288 | { | 51 | { |
289 | replay_add_event(REPLAY_ASYNC_EVENT_INPUT, event, NULL, 0); | 52 | assert(blk->in_flight > 0); |
290 | @@ -XXX,XX +XXX,XX @@ static void replay_save_event(Event *event, int checkpoint) | 53 | |
291 | /* save event-specific data */ | 54 | - if (blk->quiesce_counter && !blk->disable_request_queuing) { |
292 | switch (event->event_kind) { | 55 | + if (qatomic_read(&blk->quiesce_counter) && !blk->disable_request_queuing) { |
293 | case REPLAY_ASYNC_EVENT_BH: | 56 | blk_dec_in_flight(blk); |
294 | + case REPLAY_ASYNC_EVENT_BH_ONESHOT: | 57 | qemu_co_queue_wait(&blk->queued_requests, NULL); |
295 | replay_put_qword(event->id); | 58 | blk_inc_in_flight(blk); |
296 | break; | 59 | @@ -XXX,XX +XXX,XX @@ static void blk_root_drained_begin(BdrvChild *child) |
297 | case REPLAY_ASYNC_EVENT_INPUT: | 60 | BlockBackend *blk = child->opaque; |
298 | @@ -XXX,XX +XXX,XX @@ static Event *replay_read_event(int checkpoint) | 61 | ThrottleGroupMember *tgm = &blk->public.throttle_group_member; |
299 | /* Events that has not to be in the queue */ | 62 | |
300 | switch (replay_state.read_event_kind) { | 63 | - if (++blk->quiesce_counter == 1) { |
301 | case REPLAY_ASYNC_EVENT_BH: | 64 | + if (qatomic_fetch_inc(&blk->quiesce_counter) == 0) { |
302 | + case REPLAY_ASYNC_EVENT_BH_ONESHOT: | 65 | if (blk->dev_ops && blk->dev_ops->drained_begin) { |
303 | if (replay_state.read_event_id == -1) { | 66 | blk->dev_ops->drained_begin(blk->dev_opaque); |
304 | replay_state.read_event_id = replay_get_qword(); | ||
305 | } | 67 | } |
306 | diff --git a/stubs/replay-user.c b/stubs/replay-user.c | 68 | @@ -XXX,XX +XXX,XX @@ static bool blk_root_drained_poll(BdrvChild *child) |
307 | new file mode 100644 | 69 | { |
308 | index XXXXXXX..XXXXXXX | 70 | BlockBackend *blk = child->opaque; |
309 | --- /dev/null | 71 | bool busy = false; |
310 | +++ b/stubs/replay-user.c | 72 | - assert(blk->quiesce_counter); |
311 | @@ -XXX,XX +XXX,XX @@ | 73 | + assert(qatomic_read(&blk->quiesce_counter)); |
312 | +#include "qemu/osdep.h" | 74 | |
313 | +#include "sysemu/replay.h" | 75 | if (blk->dev_ops && blk->dev_ops->drained_poll) { |
314 | +#include "sysemu/sysemu.h" | 76 | busy = blk->dev_ops->drained_poll(blk->dev_opaque); |
315 | + | 77 | @@ -XXX,XX +XXX,XX @@ static bool blk_root_drained_poll(BdrvChild *child) |
316 | +void replay_bh_schedule_oneshot_event(AioContext *ctx, | 78 | static void blk_root_drained_end(BdrvChild *child) |
317 | + QEMUBHFunc *cb, void *opaque) | 79 | { |
318 | +{ | 80 | BlockBackend *blk = child->opaque; |
319 | + aio_bh_schedule_oneshot(ctx, cb, opaque); | 81 | - assert(blk->quiesce_counter); |
320 | +} | 82 | + assert(qatomic_read(&blk->quiesce_counter)); |
321 | diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs | 83 | |
322 | index XXXXXXX..XXXXXXX 100644 | 84 | assert(blk->public.throttle_group_member.io_limits_disabled); |
323 | --- a/stubs/Makefile.objs | 85 | qatomic_dec(&blk->public.throttle_group_member.io_limits_disabled); |
324 | +++ b/stubs/Makefile.objs | 86 | |
325 | @@ -XXX,XX +XXX,XX @@ stub-obj-y += monitor.o | 87 | - if (--blk->quiesce_counter == 0) { |
326 | stub-obj-y += notify-event.o | 88 | + if (qatomic_fetch_dec(&blk->quiesce_counter) == 1) { |
327 | stub-obj-y += qtest.o | 89 | if (blk->dev_ops && blk->dev_ops->drained_end) { |
328 | stub-obj-y += replay.o | 90 | blk->dev_ops->drained_end(blk->dev_opaque); |
329 | +stub-obj-y += replay-user.o | 91 | } |
330 | stub-obj-y += runstate-check.o | ||
331 | stub-obj-y += set-fd-handler.o | ||
332 | stub-obj-y += sysbus.o | ||
333 | -- | 92 | -- |
334 | 2.20.1 | 93 | 2.40.0 |
335 | 94 | ||
336 | 95 | diff view generated by jsdifflib |
1 | From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> | 1 | From: Stefan Hajnoczi <stefanha@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | After recent updates block devices cannot be closed on qemu exit. | 3 | This field is accessed by multiple threads without a lock. Use explicit |
4 | This happens due to the block request polling when replay is not finished. | 4 | qatomic_read()/qatomic_set() calls. There is no need for acquire/release |
5 | Therefore now we stop execution recording before closing the block devices. | 5 | because blk_set_disable_request_queuing() doesn't provide any |
6 | guarantees (it helps that it's used at BlockBackend creation time and | ||
7 | not when there is I/O in flight). | ||
6 | 8 | ||
7 | Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> | 9 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
10 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
11 | Message-Id: <20230307210427.269214-3-stefanha@redhat.com> | ||
12 | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> | ||
13 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 14 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
9 | --- | 15 | --- |
10 | replay/replay.c | 2 ++ | 16 | block/block-backend.c | 7 ++++--- |
11 | vl.c | 1 + | 17 | 1 file changed, 4 insertions(+), 3 deletions(-) |
12 | 2 files changed, 3 insertions(+) | ||
13 | 18 | ||
14 | diff --git a/replay/replay.c b/replay/replay.c | 19 | diff --git a/block/block-backend.c b/block/block-backend.c |
15 | index XXXXXXX..XXXXXXX 100644 | 20 | index XXXXXXX..XXXXXXX 100644 |
16 | --- a/replay/replay.c | 21 | --- a/block/block-backend.c |
17 | +++ b/replay/replay.c | 22 | +++ b/block/block-backend.c |
18 | @@ -XXX,XX +XXX,XX @@ void replay_finish(void) | 23 | @@ -XXX,XX +XXX,XX @@ struct BlockBackend { |
19 | g_free(replay_snapshot); | 24 | |
20 | replay_snapshot = NULL; | 25 | int quiesce_counter; /* atomic: written under BQL, read by other threads */ |
21 | 26 | CoQueue queued_requests; | |
22 | + replay_mode = REPLAY_MODE_NONE; | 27 | - bool disable_request_queuing; |
23 | + | 28 | + bool disable_request_queuing; /* atomic */ |
24 | replay_finish_events(); | 29 | |
30 | VMChangeStateEntry *vmsh; | ||
31 | bool force_allow_inactivate; | ||
32 | @@ -XXX,XX +XXX,XX @@ void blk_set_allow_aio_context_change(BlockBackend *blk, bool allow) | ||
33 | void blk_set_disable_request_queuing(BlockBackend *blk, bool disable) | ||
34 | { | ||
35 | IO_CODE(); | ||
36 | - blk->disable_request_queuing = disable; | ||
37 | + qatomic_set(&blk->disable_request_queuing, disable); | ||
25 | } | 38 | } |
26 | 39 | ||
27 | diff --git a/vl.c b/vl.c | 40 | static int coroutine_fn GRAPH_RDLOCK |
28 | index XXXXXXX..XXXXXXX 100644 | 41 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn blk_wait_while_drained(BlockBackend *blk) |
29 | --- a/vl.c | 42 | { |
30 | +++ b/vl.c | 43 | assert(blk->in_flight > 0); |
31 | @@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv, char **envp) | 44 | |
32 | 45 | - if (qatomic_read(&blk->quiesce_counter) && !blk->disable_request_queuing) { | |
33 | /* No more vcpu or device emulation activity beyond this point */ | 46 | + if (qatomic_read(&blk->quiesce_counter) && |
34 | vm_shutdown(); | 47 | + !qatomic_read(&blk->disable_request_queuing)) { |
35 | + replay_finish(); | 48 | blk_dec_in_flight(blk); |
36 | 49 | qemu_co_queue_wait(&blk->queued_requests, NULL); | |
37 | job_cancel_sync_all(); | 50 | blk_inc_in_flight(blk); |
38 | bdrv_close_all(); | ||
39 | -- | 51 | -- |
40 | 2.20.1 | 52 | 2.40.0 |
41 | 53 | ||
42 | 54 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Stefan Hajnoczi <stefanha@redhat.com> | ||
1 | 2 | ||
3 | The CoQueue API offers thread-safety via the lock argument that | ||
4 | qemu_co_queue_wait() and qemu_co_enter_next() take. BlockBackend | ||
5 | currently does not make use of the lock argument. This means that | ||
6 | multiple threads submitting I/O requests can corrupt the CoQueue's | ||
7 | QSIMPLEQ. | ||
8 | |||
9 | Add a QemuMutex and pass it to CoQueue APIs so that the queue is | ||
10 | protected. While we're at it, also assert that the queue is empty when | ||
11 | the BlockBackend is deleted. | ||
12 | |||
13 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
14 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
15 | Message-Id: <20230307210427.269214-4-stefanha@redhat.com> | ||
16 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
17 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
18 | --- | ||
19 | block/block-backend.c | 18 ++++++++++++++++-- | ||
20 | 1 file changed, 16 insertions(+), 2 deletions(-) | ||
21 | |||
22 | diff --git a/block/block-backend.c b/block/block-backend.c | ||
23 | index XXXXXXX..XXXXXXX 100644 | ||
24 | --- a/block/block-backend.c | ||
25 | +++ b/block/block-backend.c | ||
26 | @@ -XXX,XX +XXX,XX @@ struct BlockBackend { | ||
27 | QLIST_HEAD(, BlockBackendAioNotifier) aio_notifiers; | ||
28 | |||
29 | int quiesce_counter; /* atomic: written under BQL, read by other threads */ | ||
30 | + QemuMutex queued_requests_lock; /* protects queued_requests */ | ||
31 | CoQueue queued_requests; | ||
32 | bool disable_request_queuing; /* atomic */ | ||
33 | |||
34 | @@ -XXX,XX +XXX,XX @@ BlockBackend *blk_new(AioContext *ctx, uint64_t perm, uint64_t shared_perm) | ||
35 | |||
36 | block_acct_init(&blk->stats); | ||
37 | |||
38 | + qemu_mutex_init(&blk->queued_requests_lock); | ||
39 | qemu_co_queue_init(&blk->queued_requests); | ||
40 | notifier_list_init(&blk->remove_bs_notifiers); | ||
41 | notifier_list_init(&blk->insert_bs_notifiers); | ||
42 | @@ -XXX,XX +XXX,XX @@ static void blk_delete(BlockBackend *blk) | ||
43 | assert(QLIST_EMPTY(&blk->remove_bs_notifiers.notifiers)); | ||
44 | assert(QLIST_EMPTY(&blk->insert_bs_notifiers.notifiers)); | ||
45 | assert(QLIST_EMPTY(&blk->aio_notifiers)); | ||
46 | + assert(qemu_co_queue_empty(&blk->queued_requests)); | ||
47 | + qemu_mutex_destroy(&blk->queued_requests_lock); | ||
48 | QTAILQ_REMOVE(&block_backends, blk, link); | ||
49 | drive_info_del(blk->legacy_dinfo); | ||
50 | block_acct_cleanup(&blk->stats); | ||
51 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn blk_wait_while_drained(BlockBackend *blk) | ||
52 | |||
53 | if (qatomic_read(&blk->quiesce_counter) && | ||
54 | !qatomic_read(&blk->disable_request_queuing)) { | ||
55 | + /* | ||
56 | + * Take lock before decrementing in flight counter so main loop thread | ||
57 | + * waits for us to enqueue ourselves before it can leave the drained | ||
58 | + * section. | ||
59 | + */ | ||
60 | + qemu_mutex_lock(&blk->queued_requests_lock); | ||
61 | blk_dec_in_flight(blk); | ||
62 | - qemu_co_queue_wait(&blk->queued_requests, NULL); | ||
63 | + qemu_co_queue_wait(&blk->queued_requests, &blk->queued_requests_lock); | ||
64 | blk_inc_in_flight(blk); | ||
65 | + qemu_mutex_unlock(&blk->queued_requests_lock); | ||
66 | } | ||
67 | } | ||
68 | |||
69 | @@ -XXX,XX +XXX,XX @@ static void blk_root_drained_end(BdrvChild *child) | ||
70 | if (blk->dev_ops && blk->dev_ops->drained_end) { | ||
71 | blk->dev_ops->drained_end(blk->dev_opaque); | ||
72 | } | ||
73 | - while (qemu_co_enter_next(&blk->queued_requests, NULL)) { | ||
74 | + qemu_mutex_lock(&blk->queued_requests_lock); | ||
75 | + while (qemu_co_enter_next(&blk->queued_requests, | ||
76 | + &blk->queued_requests_lock)) { | ||
77 | /* Resume all queued requests */ | ||
78 | } | ||
79 | + qemu_mutex_unlock(&blk->queued_requests_lock); | ||
80 | } | ||
81 | } | ||
82 | |||
83 | -- | ||
84 | 2.40.0 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Stefan Hajnoczi <stefanha@redhat.com> | ||
1 | 2 | ||
3 | There is no need for the AioContext lock in bdrv_drain_all() because | ||
4 | nothing in AIO_WAIT_WHILE() needs the lock and the condition is atomic. | ||
5 | |||
6 | AIO_WAIT_WHILE_UNLOCKED() has no use for the AioContext parameter other | ||
7 | than performing a check that is nowadays already done by the | ||
8 | GLOBAL_STATE_CODE()/IO_CODE() macros. Set the ctx argument to NULL here | ||
9 | to help us keep track of all converted callers. Eventually all callers | ||
10 | will have been converted and then the argument can be dropped entirely. | ||
11 | |||
12 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
13 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
14 | Message-Id: <20230309190855.414275-2-stefanha@redhat.com> | ||
15 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
16 | Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> | ||
17 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
18 | --- | ||
19 | block/block-backend.c | 8 +------- | ||
20 | 1 file changed, 1 insertion(+), 7 deletions(-) | ||
21 | |||
22 | diff --git a/block/block-backend.c b/block/block-backend.c | ||
23 | index XXXXXXX..XXXXXXX 100644 | ||
24 | --- a/block/block-backend.c | ||
25 | +++ b/block/block-backend.c | ||
26 | @@ -XXX,XX +XXX,XX @@ void blk_drain_all(void) | ||
27 | bdrv_drain_all_begin(); | ||
28 | |||
29 | while ((blk = blk_all_next(blk)) != NULL) { | ||
30 | - AioContext *ctx = blk_get_aio_context(blk); | ||
31 | - | ||
32 | - aio_context_acquire(ctx); | ||
33 | - | ||
34 | /* We may have -ENOMEDIUM completions in flight */ | ||
35 | - AIO_WAIT_WHILE(ctx, qatomic_read(&blk->in_flight) > 0); | ||
36 | - | ||
37 | - aio_context_release(ctx); | ||
38 | + AIO_WAIT_WHILE_UNLOCKED(NULL, qatomic_read(&blk->in_flight) > 0); | ||
39 | } | ||
40 | |||
41 | bdrv_drain_all_end(); | ||
42 | -- | ||
43 | 2.40.0 | diff view generated by jsdifflib |
1 | From: Alberto Garcia <berto@igalia.com> | 1 | From: Stefan Hajnoczi <stefanha@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | The BDRV_REQ_NO_FALLBACK flag means that an operation should only be | 3 | There is no change in behavior. Switch to AIO_WAIT_WHILE_UNLOCKED() |
4 | performed if it can be offloaded or otherwise performed efficiently. | 4 | instead of AIO_WAIT_WHILE() to document that this code has already been |
5 | audited and converted. The AioContext argument is already NULL so | ||
6 | aio_context_release() is never called anyway. | ||
5 | 7 | ||
6 | However a misaligned write request requires a RMW so we should return | 8 | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
7 | an error and let the caller decide how to proceed. | 9 | Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
8 | 10 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | |
9 | This hits an assertion since commit c8bb23cbdb if the required | 11 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
10 | alignment is larger than the cluster size: | 12 | Message-Id: <20230309190855.414275-3-stefanha@redhat.com> |
11 | 13 | Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> | |
12 | qemu-img create -f qcow2 -o cluster_size=2k img.qcow2 4G | ||
13 | qemu-io -c "open -o driver=qcow2,file.align=4k blkdebug::img.qcow2" \ | ||
14 | -c 'write 0 512' | ||
15 | qemu-io: block/io.c:1127: bdrv_driver_pwritev: Assertion `!(flags & BDRV_REQ_NO_FALLBACK)' failed. | ||
16 | Aborted | ||
17 | |||
18 | The reason is that when writing to an unallocated cluster we try to | ||
19 | skip the copy-on-write part and zeroize it using BDRV_REQ_NO_FALLBACK | ||
20 | instead, resulting in a write request that is too small (2KB cluster | ||
21 | size vs 4KB required alignment). | ||
22 | |||
23 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
24 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 14 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
25 | --- | 15 | --- |
26 | block/io.c | 7 +++++ | 16 | block/export/export.c | 2 +- |
27 | tests/qemu-iotests/268 | 55 ++++++++++++++++++++++++++++++++++++++ | 17 | 1 file changed, 1 insertion(+), 1 deletion(-) |
28 | tests/qemu-iotests/268.out | 7 +++++ | ||
29 | tests/qemu-iotests/group | 1 + | ||
30 | 4 files changed, 70 insertions(+) | ||
31 | create mode 100755 tests/qemu-iotests/268 | ||
32 | create mode 100644 tests/qemu-iotests/268.out | ||
33 | 18 | ||
34 | diff --git a/block/io.c b/block/io.c | 19 | diff --git a/block/export/export.c b/block/export/export.c |
35 | index XXXXXXX..XXXXXXX 100644 | 20 | index XXXXXXX..XXXXXXX 100644 |
36 | --- a/block/io.c | 21 | --- a/block/export/export.c |
37 | +++ b/block/io.c | 22 | +++ b/block/export/export.c |
38 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child, | 23 | @@ -XXX,XX +XXX,XX @@ void blk_exp_close_all_type(BlockExportType type) |
39 | return ret; | 24 | blk_exp_request_shutdown(exp); |
40 | } | 25 | } |
41 | 26 | ||
42 | + /* If the request is misaligned then we can't make it efficient */ | 27 | - AIO_WAIT_WHILE(NULL, blk_exp_has_type(type)); |
43 | + if ((flags & BDRV_REQ_NO_FALLBACK) && | 28 | + AIO_WAIT_WHILE_UNLOCKED(NULL, blk_exp_has_type(type)); |
44 | + !QEMU_IS_ALIGNED(offset | bytes, align)) | 29 | } |
45 | + { | 30 | |
46 | + return -ENOTSUP; | 31 | void blk_exp_close_all(void) |
47 | + } | ||
48 | + | ||
49 | bdrv_inc_in_flight(bs); | ||
50 | /* | ||
51 | * Align write if necessary by performing a read-modify-write cycle. | ||
52 | diff --git a/tests/qemu-iotests/268 b/tests/qemu-iotests/268 | ||
53 | new file mode 100755 | ||
54 | index XXXXXXX..XXXXXXX | ||
55 | --- /dev/null | ||
56 | +++ b/tests/qemu-iotests/268 | ||
57 | @@ -XXX,XX +XXX,XX @@ | ||
58 | +#!/usr/bin/env bash | ||
59 | +# | ||
60 | +# Test write request with required alignment larger than the cluster size | ||
61 | +# | ||
62 | +# Copyright (C) 2019 Igalia, S.L. | ||
63 | +# Author: Alberto Garcia <berto@igalia.com> | ||
64 | +# | ||
65 | +# This program is free software; you can redistribute it and/or modify | ||
66 | +# it under the terms of the GNU General Public License as published by | ||
67 | +# the Free Software Foundation; either version 2 of the License, or | ||
68 | +# (at your option) any later version. | ||
69 | +# | ||
70 | +# This program is distributed in the hope that it will be useful, | ||
71 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
72 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
73 | +# GNU General Public License for more details. | ||
74 | +# | ||
75 | +# You should have received a copy of the GNU General Public License | ||
76 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
77 | +# | ||
78 | + | ||
79 | +# creator | ||
80 | +owner=berto@igalia.com | ||
81 | + | ||
82 | +seq=`basename $0` | ||
83 | +echo "QA output created by $seq" | ||
84 | + | ||
85 | +status=1 # failure is the default! | ||
86 | + | ||
87 | +_cleanup() | ||
88 | +{ | ||
89 | + _cleanup_test_img | ||
90 | +} | ||
91 | +trap "_cleanup; exit \$status" 0 1 2 3 15 | ||
92 | + | ||
93 | +# get standard environment, filters and checks | ||
94 | +. ./common.rc | ||
95 | +. ./common.filter | ||
96 | + | ||
97 | +_supported_fmt qcow2 | ||
98 | +_supported_proto file | ||
99 | + | ||
100 | +echo | ||
101 | +echo "== Required alignment larger than cluster size ==" | ||
102 | + | ||
103 | +CLUSTER_SIZE=2k _make_test_img 1M | ||
104 | +# Since commit c8bb23cbdb writing to an unallocated cluster fills the | ||
105 | +# empty COW areas with bdrv_write_zeroes(flags=BDRV_REQ_NO_FALLBACK) | ||
106 | +$QEMU_IO -c "open -o driver=$IMGFMT,file.align=4k blkdebug::$TEST_IMG" \ | ||
107 | + -c "write 0 512" | _filter_qemu_io | ||
108 | + | ||
109 | +# success, all done | ||
110 | +echo "*** done" | ||
111 | +rm -f $seq.full | ||
112 | +status=0 | ||
113 | diff --git a/tests/qemu-iotests/268.out b/tests/qemu-iotests/268.out | ||
114 | new file mode 100644 | ||
115 | index XXXXXXX..XXXXXXX | ||
116 | --- /dev/null | ||
117 | +++ b/tests/qemu-iotests/268.out | ||
118 | @@ -XXX,XX +XXX,XX @@ | ||
119 | +QA output created by 268 | ||
120 | + | ||
121 | +== Required alignment larger than cluster size == | ||
122 | +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 | ||
123 | +wrote 512/512 bytes at offset 0 | ||
124 | +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
125 | +*** done | ||
126 | diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group | ||
127 | index XXXXXXX..XXXXXXX 100644 | ||
128 | --- a/tests/qemu-iotests/group | ||
129 | +++ b/tests/qemu-iotests/group | ||
130 | @@ -XXX,XX +XXX,XX @@ | ||
131 | 265 rw auto quick | ||
132 | 266 rw quick | ||
133 | 267 rw auto quick snapshot | ||
134 | +268 rw auto quick | ||
135 | -- | 32 | -- |
136 | 2.20.1 | 33 | 2.40.0 |
137 | 34 | ||
138 | 35 | diff view generated by jsdifflib |
1 | From: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> | 1 | From: Stefan Hajnoczi <stefanha@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | This patch disables setting '-snapshot' option on by default | 3 | The following conversion is safe and does not change behavior: |
4 | in record/replay mode. This is needed for creating vmstates in record | ||
5 | and replay modes. | ||
6 | 4 | ||
7 | Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> | 5 | GLOBAL_STATE_CODE(); |
8 | Acked-by: Kevin Wolf <kwolf@redhat.com> | 6 | ... |
7 | - AIO_WAIT_WHILE(qemu_get_aio_context(), ...); | ||
8 | + AIO_WAIT_WHILE_UNLOCKED(NULL, ...); | ||
9 | |||
10 | Since we're in GLOBAL_STATE_CODE(), qemu_get_aio_context() is our home | ||
11 | thread's AioContext. Thus AIO_WAIT_WHILE() does not unlock the | ||
12 | AioContext: | ||
13 | |||
14 | if (ctx_ && in_aio_context_home_thread(ctx_)) { \ | ||
15 | while ((cond)) { \ | ||
16 | aio_poll(ctx_, true); \ | ||
17 | waited_ = true; \ | ||
18 | } \ | ||
19 | |||
20 | And that means AIO_WAIT_WHILE_UNLOCKED(NULL, ...) can be substituted. | ||
21 | |||
22 | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> | ||
23 | Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org> | ||
24 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
25 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
26 | Message-Id: <20230309190855.414275-4-stefanha@redhat.com> | ||
27 | Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> | ||
9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 28 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
10 | --- | 29 | --- |
11 | vl.c | 10 ++++++++-- | 30 | block/graph-lock.c | 2 +- |
12 | 1 file changed, 8 insertions(+), 2 deletions(-) | 31 | 1 file changed, 1 insertion(+), 1 deletion(-) |
13 | 32 | ||
14 | diff --git a/vl.c b/vl.c | 33 | diff --git a/block/graph-lock.c b/block/graph-lock.c |
15 | index XXXXXXX..XXXXXXX 100644 | 34 | index XXXXXXX..XXXXXXX 100644 |
16 | --- a/vl.c | 35 | --- a/block/graph-lock.c |
17 | +++ b/vl.c | 36 | +++ b/block/graph-lock.c |
18 | @@ -XXX,XX +XXX,XX @@ static void configure_blockdev(BlockdevOptionsQueue *bdo_queue, | 37 | @@ -XXX,XX +XXX,XX @@ void bdrv_graph_wrlock(void) |
19 | qapi_free_BlockdevOptions(bdo->bdo); | 38 | * reader lock. |
20 | g_free(bdo); | 39 | */ |
21 | } | 40 | qatomic_set(&has_writer, 0); |
22 | - if (snapshot || replay_mode != REPLAY_MODE_NONE) { | 41 | - AIO_WAIT_WHILE(qemu_get_aio_context(), reader_count() >= 1); |
23 | + if (snapshot) { | 42 | + AIO_WAIT_WHILE_UNLOCKED(NULL, reader_count() >= 1); |
24 | qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, | 43 | qatomic_set(&has_writer, 1); |
25 | NULL, NULL); | 44 | |
26 | } | 45 | /* |
27 | @@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv, char **envp) | ||
28 | drive_add(IF_PFLASH, -1, optarg, PFLASH_OPTS); | ||
29 | break; | ||
30 | case QEMU_OPTION_snapshot: | ||
31 | - snapshot = 1; | ||
32 | + { | ||
33 | + Error *blocker = NULL; | ||
34 | + snapshot = 1; | ||
35 | + error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, | ||
36 | + "-snapshot"); | ||
37 | + replay_add_blocker(blocker); | ||
38 | + } | ||
39 | break; | ||
40 | case QEMU_OPTION_numa: | ||
41 | opts = qemu_opts_parse_noisily(qemu_find_opts("numa"), | ||
42 | -- | 46 | -- |
43 | 2.20.1 | 47 | 2.40.0 |
44 | 48 | ||
45 | 49 | diff view generated by jsdifflib |
1 | From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> | 1 | From: Stefan Hajnoczi <stefanha@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | In record/replay mode bdrv queue is controlled by replay mechanism. | 3 | Since the AioContext argument was already NULL, AIO_WAIT_WHILE() was |
4 | It does not allow saving or loading the snapshots | 4 | never going to unlock the AioContext. Therefore it is possible to |
5 | when bdrv queue is not empty. Stopping the VM is not blocked by nonempty | 5 | replace AIO_WAIT_WHILE() with AIO_WAIT_WHILE_UNLOCKED(). |
6 | queue, but flushing the queue is still impossible there, | ||
7 | because it may cause deadlocks in replay mode. | ||
8 | This patch disables bdrv_drain_all and bdrv_flush_all in | ||
9 | record/replay mode. | ||
10 | 6 | ||
11 | Stopping the machine when the IO requests are not finished is needed | 7 | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
12 | for the debugging. E.g., breakpoint may be set at the specified step, | 8 | Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
13 | and forcing the IO requests to finish may break the determinism | 9 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> |
14 | of the execution. | 10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
15 | 11 | Message-Id: <20230309190855.414275-5-stefanha@redhat.com> | |
16 | Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> | 12 | Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> |
17 | Acked-by: Kevin Wolf <kwolf@redhat.com> | ||
18 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 13 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
19 | --- | 14 | --- |
20 | block/io.c | 28 ++++++++++++++++++++++++++++ | 15 | block/io.c | 2 +- |
21 | cpus.c | 2 -- | 16 | 1 file changed, 1 insertion(+), 1 deletion(-) |
22 | 2 files changed, 28 insertions(+), 2 deletions(-) | ||
23 | 17 | ||
24 | diff --git a/block/io.c b/block/io.c | 18 | diff --git a/block/io.c b/block/io.c |
25 | index XXXXXXX..XXXXXXX 100644 | 19 | index XXXXXXX..XXXXXXX 100644 |
26 | --- a/block/io.c | 20 | --- a/block/io.c |
27 | +++ b/block/io.c | 21 | +++ b/block/io.c |
28 | @@ -XXX,XX +XXX,XX @@ | ||
29 | #include "qapi/error.h" | ||
30 | #include "qemu/error-report.h" | ||
31 | #include "qemu/main-loop.h" | ||
32 | +#include "sysemu/replay.h" | ||
33 | |||
34 | #define NOT_DONE 0x7fffffff /* used while emulated sync operation in progress */ | ||
35 | |||
36 | @@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void) | 22 | @@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void) |
37 | return; | 23 | bdrv_drain_all_begin_nopoll(); |
38 | } | 24 | |
39 | 25 | /* Now poll the in-flight requests */ | |
40 | + /* | 26 | - AIO_WAIT_WHILE(NULL, bdrv_drain_all_poll()); |
41 | + * bdrv queue is managed by record/replay, | 27 | + AIO_WAIT_WHILE_UNLOCKED(NULL, bdrv_drain_all_poll()); |
42 | + * waiting for finishing the I/O requests may | 28 | |
43 | + * be infinite | ||
44 | + */ | ||
45 | + if (replay_events_enabled()) { | ||
46 | + return; | ||
47 | + } | ||
48 | + | ||
49 | /* AIO_WAIT_WHILE() with a NULL context can only be called from the main | ||
50 | * loop AioContext, so make sure we're in the main context. */ | ||
51 | assert(qemu_get_current_aio_context() == qemu_get_aio_context()); | ||
52 | @@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_end(void) | ||
53 | BlockDriverState *bs = NULL; | ||
54 | int drained_end_counter = 0; | ||
55 | |||
56 | + /* | ||
57 | + * bdrv queue is managed by record/replay, | ||
58 | + * waiting for finishing the I/O requests may | ||
59 | + * be endless | ||
60 | + */ | ||
61 | + if (replay_events_enabled()) { | ||
62 | + return; | ||
63 | + } | ||
64 | + | ||
65 | while ((bs = bdrv_next_all_states(bs))) { | 29 | while ((bs = bdrv_next_all_states(bs))) { |
66 | AioContext *aio_context = bdrv_get_aio_context(bs); | 30 | bdrv_drain_assert_idle(bs); |
67 | |||
68 | @@ -XXX,XX +XXX,XX @@ int bdrv_flush_all(void) | ||
69 | BlockDriverState *bs = NULL; | ||
70 | int result = 0; | ||
71 | |||
72 | + /* | ||
73 | + * bdrv queue is managed by record/replay, | ||
74 | + * creating new flush request for stopping | ||
75 | + * the VM may break the determinism | ||
76 | + */ | ||
77 | + if (replay_events_enabled()) { | ||
78 | + return result; | ||
79 | + } | ||
80 | + | ||
81 | for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { | ||
82 | AioContext *aio_context = bdrv_get_aio_context(bs); | ||
83 | int ret; | ||
84 | diff --git a/cpus.c b/cpus.c | ||
85 | index XXXXXXX..XXXXXXX 100644 | ||
86 | --- a/cpus.c | ||
87 | +++ b/cpus.c | ||
88 | @@ -XXX,XX +XXX,XX @@ static int do_vm_stop(RunState state, bool send_stop) | ||
89 | } | ||
90 | |||
91 | bdrv_drain_all(); | ||
92 | - replay_disable_events(); | ||
93 | ret = bdrv_flush_all(); | ||
94 | |||
95 | return ret; | ||
96 | @@ -XXX,XX +XXX,XX @@ int vm_prepare_start(void) | ||
97 | /* We are sending this now, but the CPUs will be resumed shortly later */ | ||
98 | qapi_event_send_resume(); | ||
99 | |||
100 | - replay_enable_events(); | ||
101 | cpu_enable_ticks(); | ||
102 | runstate_set(RUN_STATE_RUNNING); | ||
103 | vm_state_notify(1, RUN_STATE_RUNNING); | ||
104 | -- | 31 | -- |
105 | 2.20.1 | 32 | 2.40.0 |
106 | 33 | ||
107 | 34 | diff view generated by jsdifflib |
1 | From: Max Reitz <mreitz@redhat.com> | 1 | From: Stefan Hajnoczi <stefanha@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | For long test image paths, the order of the "Formatting" line and the | 3 | The HMP monitor runs in the main loop thread. Calling |
4 | "(qemu)" prompt after a drive_backup HMP command may be reversed. In | 4 | AIO_WAIT_WHILE(qemu_get_aio_context(), ...) from the main loop thread is |
5 | fact, the interaction between the prompt and the line may lead to the | 5 | equivalent to AIO_WAIT_WHILE_UNLOCKED(NULL, ...) because neither unlocks |
6 | "Formatting" to being greppable at all after "read"-ing it (if the | 6 | the AioContext and the latter's assertion that we're in the main loop |
7 | prompt injects an IFS character into the "Formatting" string). | 7 | succeeds. |
8 | 8 | ||
9 | So just wait until we get a prompt. At that point, the block job must | 9 | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
10 | have been started, so "info block-jobs" will only return "No active | 10 | Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
11 | jobs" once it is done. | 11 | Reviewed-by: Markus Armbruster <armbru@redhat.com> |
12 | 12 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | |
13 | Reported-by: Thomas Huth <thuth@redhat.com> | 13 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
14 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 14 | Message-Id: <20230309190855.414275-6-stefanha@redhat.com> |
15 | Reviewed-by: John Snow <jsnow@redhat.com> | 15 | Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> |
16 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 16 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
17 | --- | 17 | --- |
18 | tests/qemu-iotests/028 | 11 ++++++++--- | 18 | monitor/hmp.c | 2 +- |
19 | tests/qemu-iotests/028.out | 1 - | 19 | 1 file changed, 1 insertion(+), 1 deletion(-) |
20 | 2 files changed, 8 insertions(+), 4 deletions(-) | ||
21 | 20 | ||
22 | diff --git a/tests/qemu-iotests/028 b/tests/qemu-iotests/028 | 21 | diff --git a/monitor/hmp.c b/monitor/hmp.c |
23 | index XXXXXXX..XXXXXXX 100755 | ||
24 | --- a/tests/qemu-iotests/028 | ||
25 | +++ b/tests/qemu-iotests/028 | ||
26 | @@ -XXX,XX +XXX,XX @@ fi | ||
27 | # Silence output since it contains the disk image path and QEMU's readline | ||
28 | # character echoing makes it very hard to filter the output. Plus, there | ||
29 | # is no telling how many times the command will repeat before succeeding. | ||
30 | -_send_qemu_cmd $h "drive_backup disk ${TEST_IMG}.copy" "(qemu)" >/dev/null | ||
31 | -_send_qemu_cmd $h "" "Formatting" | _filter_img_create | ||
32 | -qemu_cmd_repeat=20 _send_qemu_cmd $h "info block-jobs" "No active jobs" >/dev/null | ||
33 | +# (Note that creating the image results in a "Formatting..." message over | ||
34 | +# stdout, which is the same channel the monitor uses. We cannot reliably | ||
35 | +# wait for it because the monitor output may interact with it in such a | ||
36 | +# way that _timed_wait_for cannot read it. However, once the block job is | ||
37 | +# done, we know that the "Formatting..." message must have appeared | ||
38 | +# already, so the output is still deterministic.) | ||
39 | +silent=y _send_qemu_cmd $h "drive_backup disk ${TEST_IMG}.copy" "(qemu)" | ||
40 | +silent=y qemu_cmd_repeat=20 _send_qemu_cmd $h "info block-jobs" "No active jobs" | ||
41 | _send_qemu_cmd $h "info block-jobs" "No active jobs" | ||
42 | _send_qemu_cmd $h 'quit' "" | ||
43 | |||
44 | diff --git a/tests/qemu-iotests/028.out b/tests/qemu-iotests/028.out | ||
45 | index XXXXXXX..XXXXXXX 100644 | 22 | index XXXXXXX..XXXXXXX 100644 |
46 | --- a/tests/qemu-iotests/028.out | 23 | --- a/monitor/hmp.c |
47 | +++ b/tests/qemu-iotests/028.out | 24 | +++ b/monitor/hmp.c |
48 | @@ -XXX,XX +XXX,XX @@ No errors were found on the image. | 25 | @@ -XXX,XX +XXX,XX @@ void handle_hmp_command(MonitorHMP *mon, const char *cmdline) |
49 | 26 | Coroutine *co = qemu_coroutine_create(handle_hmp_command_co, &data); | |
50 | block-backup | 27 | monitor_set_cur(co, &mon->common); |
51 | 28 | aio_co_enter(qemu_get_aio_context(), co); | |
52 | -Formatting 'TEST_DIR/t.IMGFMT.copy', fmt=IMGFMT size=4294968832 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT | 29 | - AIO_WAIT_WHILE(qemu_get_aio_context(), !data.done); |
53 | (qemu) info block-jobs | 30 | + AIO_WAIT_WHILE_UNLOCKED(NULL, !data.done); |
54 | No active jobs | 31 | } |
55 | === IO: pattern 195 | 32 | |
33 | qobject_unref(qdict); | ||
56 | -- | 34 | -- |
57 | 2.20.1 | 35 | 2.40.0 |
58 | 36 | ||
59 | 37 | diff view generated by jsdifflib |
1 | From: Peter Lieven <pl@kamp.de> | 1 | From: Stefan Hajnoczi <stefanha@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | qemu is currently not able to detect truncated vhdx image files. | 3 | monitor_cleanup() is called from the main loop thread. Calling |
4 | Add a basic check if all allocated blocks are reachable at open and | 4 | AIO_WAIT_WHILE(qemu_get_aio_context(), ...) from the main loop thread is |
5 | report all errors during bdrv_co_check. | 5 | equivalent to AIO_WAIT_WHILE_UNLOCKED(NULL, ...) because neither unlocks |
6 | the AioContext and the latter's assertion that we're in the main loop | ||
7 | succeeds. | ||
6 | 8 | ||
7 | Signed-off-by: Peter Lieven <pl@kamp.de> | 9 | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
10 | Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org> | ||
11 | Reviewed-by: Markus Armbruster <armbru@redhat.com> | ||
12 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
13 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
14 | Message-Id: <20230309190855.414275-7-stefanha@redhat.com> | ||
15 | Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 16 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
9 | --- | 17 | --- |
10 | block/vhdx.c | 120 +++++++++++++++++++++++++++++++++++++++++++-------- | 18 | monitor/monitor.c | 4 ++-- |
11 | 1 file changed, 103 insertions(+), 17 deletions(-) | 19 | 1 file changed, 2 insertions(+), 2 deletions(-) |
12 | 20 | ||
13 | diff --git a/block/vhdx.c b/block/vhdx.c | 21 | diff --git a/monitor/monitor.c b/monitor/monitor.c |
14 | index XXXXXXX..XXXXXXX 100644 | 22 | index XXXXXXX..XXXXXXX 100644 |
15 | --- a/block/vhdx.c | 23 | --- a/monitor/monitor.c |
16 | +++ b/block/vhdx.c | 24 | +++ b/monitor/monitor.c |
17 | @@ -XXX,XX +XXX,XX @@ | 25 | @@ -XXX,XX +XXX,XX @@ void monitor_cleanup(void) |
18 | #include "qemu/option.h" | 26 | * We need to poll both qemu_aio_context and iohandler_ctx to make |
19 | #include "qemu/crc32c.h" | 27 | * sure that the dispatcher coroutine keeps making progress and |
20 | #include "qemu/bswap.h" | 28 | * eventually terminates. qemu_aio_context is automatically |
21 | +#include "qemu/error-report.h" | 29 | - * polled by calling AIO_WAIT_WHILE on it, but we must poll |
22 | #include "vhdx.h" | 30 | + * polled by calling AIO_WAIT_WHILE_UNLOCKED on it, but we must poll |
23 | #include "migration/blocker.h" | 31 | * iohandler_ctx manually. |
24 | #include "qemu/uuid.h" | 32 | * |
25 | @@ -XXX,XX +XXX,XX @@ static int vhdx_region_check(BDRVVHDXState *s, uint64_t start, uint64_t length) | 33 | * Letting the iothread continue while shutting down the dispatcher |
26 | end = start + length; | 34 | @@ -XXX,XX +XXX,XX @@ void monitor_cleanup(void) |
27 | QLIST_FOREACH(r, &s->regions, entries) { | 35 | aio_co_wake(qmp_dispatcher_co); |
28 | if (!((start >= r->end) || (end <= r->start))) { | ||
29 | + error_report("VHDX region %" PRIu64 "-%" PRIu64 " overlaps with " | ||
30 | + "region %" PRIu64 "-%." PRIu64, start, end, r->start, | ||
31 | + r->end); | ||
32 | ret = -EINVAL; | ||
33 | goto exit; | ||
34 | } | ||
35 | @@ -XXX,XX +XXX,XX @@ static void vhdx_calc_bat_entries(BDRVVHDXState *s) | ||
36 | |||
37 | } | ||
38 | |||
39 | +static int vhdx_check_bat_entries(BlockDriverState *bs, int *errcnt) | ||
40 | +{ | ||
41 | + BDRVVHDXState *s = bs->opaque; | ||
42 | + int64_t image_file_size = bdrv_getlength(bs->file->bs); | ||
43 | + uint64_t payblocks = s->chunk_ratio; | ||
44 | + uint64_t i; | ||
45 | + int ret = 0; | ||
46 | + | ||
47 | + if (image_file_size < 0) { | ||
48 | + error_report("Could not determinate VHDX image file size."); | ||
49 | + return image_file_size; | ||
50 | + } | ||
51 | + | ||
52 | + for (i = 0; i < s->bat_entries; i++) { | ||
53 | + if ((s->bat[i] & VHDX_BAT_STATE_BIT_MASK) == | ||
54 | + PAYLOAD_BLOCK_FULLY_PRESENT) { | ||
55 | + uint64_t offset = s->bat[i] & VHDX_BAT_FILE_OFF_MASK; | ||
56 | + /* | ||
57 | + * Allow that the last block exists only partially. The VHDX spec | ||
58 | + * states that the image file can only grow in blocksize increments, | ||
59 | + * but QEMU created images with partial last blocks in the past. | ||
60 | + */ | ||
61 | + uint32_t block_length = MIN(s->block_size, | ||
62 | + bs->total_sectors * BDRV_SECTOR_SIZE - i * s->block_size); | ||
63 | + /* | ||
64 | + * Check for BAT entry overflow. | ||
65 | + */ | ||
66 | + if (offset > INT64_MAX - s->block_size) { | ||
67 | + error_report("VHDX BAT entry %" PRIu64 " offset overflow.", i); | ||
68 | + ret = -EINVAL; | ||
69 | + if (!errcnt) { | ||
70 | + break; | ||
71 | + } | ||
72 | + (*errcnt)++; | ||
73 | + } | ||
74 | + /* | ||
75 | + * Check if fully allocated BAT entries do not reside after | ||
76 | + * end of the image file. | ||
77 | + */ | ||
78 | + if (offset >= image_file_size) { | ||
79 | + error_report("VHDX BAT entry %" PRIu64 " start offset %" PRIu64 | ||
80 | + " points after end of file (%" PRIi64 "). Image" | ||
81 | + " has probably been truncated.", | ||
82 | + i, offset, image_file_size); | ||
83 | + ret = -EINVAL; | ||
84 | + if (!errcnt) { | ||
85 | + break; | ||
86 | + } | ||
87 | + (*errcnt)++; | ||
88 | + } else if (offset + block_length > image_file_size) { | ||
89 | + error_report("VHDX BAT entry %" PRIu64 " end offset %" PRIu64 | ||
90 | + " points after end of file (%" PRIi64 "). Image" | ||
91 | + " has probably been truncated.", | ||
92 | + i, offset + block_length - 1, image_file_size); | ||
93 | + ret = -EINVAL; | ||
94 | + if (!errcnt) { | ||
95 | + break; | ||
96 | + } | ||
97 | + (*errcnt)++; | ||
98 | + } | ||
99 | + | ||
100 | + /* | ||
101 | + * verify populated BAT field file offsets against | ||
102 | + * region table and log entries | ||
103 | + */ | ||
104 | + if (payblocks--) { | ||
105 | + /* payload bat entries */ | ||
106 | + int ret2; | ||
107 | + ret2 = vhdx_region_check(s, offset, s->block_size); | ||
108 | + if (ret2 < 0) { | ||
109 | + ret = -EINVAL; | ||
110 | + if (!errcnt) { | ||
111 | + break; | ||
112 | + } | ||
113 | + (*errcnt)++; | ||
114 | + } | ||
115 | + } else { | ||
116 | + payblocks = s->chunk_ratio; | ||
117 | + /* | ||
118 | + * Once differencing files are supported, verify sector bitmap | ||
119 | + * blocks here | ||
120 | + */ | ||
121 | + } | ||
122 | + } | ||
123 | + } | ||
124 | + | ||
125 | + return ret; | ||
126 | +} | ||
127 | + | ||
128 | static void vhdx_close(BlockDriverState *bs) | ||
129 | { | ||
130 | BDRVVHDXState *s = bs->opaque; | ||
131 | @@ -XXX,XX +XXX,XX @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags, | ||
132 | goto fail; | ||
133 | } | 36 | } |
134 | 37 | ||
135 | - uint64_t payblocks = s->chunk_ratio; | 38 | - AIO_WAIT_WHILE(qemu_get_aio_context(), |
136 | - /* endian convert, and verify populated BAT field file offsets against | 39 | + AIO_WAIT_WHILE_UNLOCKED(NULL, |
137 | - * region table and log entries */ | 40 | (aio_poll(iohandler_get_aio_context(), false), |
138 | + /* endian convert populated BAT field entires */ | 41 | qatomic_mb_read(&qmp_dispatcher_co_busy))); |
139 | for (i = 0; i < s->bat_entries; i++) { | ||
140 | s->bat[i] = le64_to_cpu(s->bat[i]); | ||
141 | - if (payblocks--) { | ||
142 | - /* payload bat entries */ | ||
143 | - if ((s->bat[i] & VHDX_BAT_STATE_BIT_MASK) == | ||
144 | - PAYLOAD_BLOCK_FULLY_PRESENT) { | ||
145 | - ret = vhdx_region_check(s, s->bat[i] & VHDX_BAT_FILE_OFF_MASK, | ||
146 | - s->block_size); | ||
147 | - if (ret < 0) { | ||
148 | - goto fail; | ||
149 | - } | ||
150 | - } | ||
151 | - } else { | ||
152 | - payblocks = s->chunk_ratio; | ||
153 | - /* Once differencing files are supported, verify sector bitmap | ||
154 | - * blocks here */ | ||
155 | + } | ||
156 | + | ||
157 | + if (!(flags & BDRV_O_CHECK)) { | ||
158 | + ret = vhdx_check_bat_entries(bs, NULL); | ||
159 | + if (ret < 0) { | ||
160 | + goto fail; | ||
161 | } | ||
162 | } | ||
163 | |||
164 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn vhdx_co_check(BlockDriverState *bs, | ||
165 | if (s->log_replayed_on_open) { | ||
166 | result->corruptions_fixed++; | ||
167 | } | ||
168 | + | ||
169 | + vhdx_check_bat_entries(bs, &result->corruptions); | ||
170 | + | ||
171 | return 0; | ||
172 | } | ||
173 | 42 | ||
174 | -- | 43 | -- |
175 | 2.20.1 | 44 | 2.40.0 |
176 | 45 | ||
177 | 46 | diff view generated by jsdifflib |
1 | From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> | 1 | From: Wilfred Mallawa <wilfred.mallawa@wdc.com> |
---|---|---|---|
2 | 2 | ||
3 | This patch updates the description of the command lines for using | 3 | Fixup a few minor typos |
4 | record/replay with attached block devices. | ||
5 | 4 | ||
6 | Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> | 5 | Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> |
6 | Message-Id: <20230313003744.55476-1-wilfred.mallawa@opensource.wdc.com> | ||
7 | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> | ||
8 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
7 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
8 | --- | 10 | --- |
9 | docs/replay.txt | 12 +++++++++--- | 11 | include/block/aio-wait.h | 2 +- |
10 | 1 file changed, 9 insertions(+), 3 deletions(-) | 12 | include/block/block_int-common.h | 2 +- |
13 | 2 files changed, 2 insertions(+), 2 deletions(-) | ||
11 | 14 | ||
12 | diff --git a/docs/replay.txt b/docs/replay.txt | 15 | diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h |
13 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100644 |
14 | --- a/docs/replay.txt | 17 | --- a/include/block/aio-wait.h |
15 | +++ b/docs/replay.txt | 18 | +++ b/include/block/aio-wait.h |
16 | @@ -XXX,XX +XXX,XX @@ Usage of the record/replay: | 19 | @@ -XXX,XX +XXX,XX @@ extern AioWait global_aio_wait; |
17 | * First, record the execution with the following command line: | 20 | * @ctx: the aio context, or NULL if multiple aio contexts (for which the |
18 | qemu-system-i386 \ | 21 | * caller does not hold a lock) are involved in the polling condition. |
19 | -icount shift=7,rr=record,rrfile=replay.bin \ | 22 | * @cond: wait while this conditional expression is true |
20 | - -drive file=disk.qcow2,if=none,id=img-direct \ | 23 | - * @unlock: whether to unlock and then lock again @ctx. This apples |
21 | + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ | 24 | + * @unlock: whether to unlock and then lock again @ctx. This applies |
22 | -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \ | 25 | * only when waiting for another AioContext from the main loop. |
23 | -device ide-hd,drive=img-blkreplay \ | 26 | * Otherwise it's ignored. |
24 | -netdev user,id=net1 -device rtl8139,netdev=net1 \ | 27 | * |
25 | @@ -XXX,XX +XXX,XX @@ Usage of the record/replay: | 28 | diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h |
26 | * After recording, you can replay it by using another command line: | 29 | index XXXXXXX..XXXXXXX 100644 |
27 | qemu-system-i386 \ | 30 | --- a/include/block/block_int-common.h |
28 | -icount shift=7,rr=replay,rrfile=replay.bin \ | 31 | +++ b/include/block/block_int-common.h |
29 | - -drive file=disk.qcow2,if=none,id=img-direct \ | 32 | @@ -XXX,XX +XXX,XX @@ extern QemuOptsList bdrv_create_opts_simple; |
30 | + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ | 33 | /* |
31 | -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \ | 34 | * Common functions that are neither I/O nor Global State. |
32 | -device ide-hd,drive=img-blkreplay \ | 35 | * |
33 | -netdev user,id=net1 -device rtl8139,netdev=net1 \ | 36 | - * See include/block/block-commmon.h for more information about |
34 | @@ -XXX,XX +XXX,XX @@ Block devices record/replay module intercepts calls of | 37 | + * See include/block/block-common.h for more information about |
35 | bdrv coroutine functions at the top of block drivers stack. | 38 | * the Common API. |
36 | To record and replay block operations the drive must be configured | 39 | */ |
37 | as following: | 40 | |
38 | - -drive file=disk.qcow2,if=none,id=img-direct | ||
39 | + -drive file=disk.qcow2,if=none,snapshot,id=img-direct | ||
40 | -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay | ||
41 | -device ide-hd,drive=img-blkreplay | ||
42 | |||
43 | @@ -XXX,XX +XXX,XX @@ This snapshot is created at start of recording and restored at start | ||
44 | of replaying. It also can be loaded while replaying to roll back | ||
45 | the execution. | ||
46 | |||
47 | +'snapshot' flag of the disk image must be removed to save the snapshots | ||
48 | +in the overlay (or original image) instead of using the temporary overlay. | ||
49 | + -drive file=disk.ovl,if=none,id=img-direct | ||
50 | + -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay | ||
51 | + -device ide-hd,drive=img-blkreplay | ||
52 | + | ||
53 | Use QEMU monitor to create additional snapshots. 'savevm <name>' command | ||
54 | created the snapshot and 'loadvm <name>' restores it. To prevent corruption | ||
55 | of the original disk image, use overlay files linked to the original images. | ||
56 | -- | 41 | -- |
57 | 2.20.1 | 42 | 2.40.0 |
58 | 43 | ||
59 | 44 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Stefan Hajnoczi <stefanha@redhat.com> | ||
1 | 2 | ||
3 | Not a coroutine_fn, you say? | ||
4 | |||
5 | static int64_t bdrv_sum_allocated_file_size(BlockDriverState *bs) | ||
6 | { | ||
7 | BdrvChild *child; | ||
8 | int64_t child_size, sum = 0; | ||
9 | |||
10 | QLIST_FOREACH(child, &bs->children, next) { | ||
11 | if (child->role & (BDRV_CHILD_DATA | BDRV_CHILD_METADATA | | ||
12 | BDRV_CHILD_FILTERED)) | ||
13 | { | ||
14 | child_size = bdrv_co_get_allocated_file_size(child->bs); | ||
15 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
16 | |||
17 | Well what do we have here?! | ||
18 | |||
19 | I rest my case, your honor. | ||
20 | |||
21 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
22 | Message-Id: <20230308211435.346375-1-stefanha@redhat.com> | ||
23 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
24 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
25 | --- | ||
26 | block.c | 2 +- | ||
27 | 1 file changed, 1 insertion(+), 1 deletion(-) | ||
28 | |||
29 | diff --git a/block.c b/block.c | ||
30 | index XXXXXXX..XXXXXXX 100644 | ||
31 | --- a/block.c | ||
32 | +++ b/block.c | ||
33 | @@ -XXX,XX +XXX,XX @@ exit: | ||
34 | * sums the size of all data-bearing children. (This excludes backing | ||
35 | * children.) | ||
36 | */ | ||
37 | -static int64_t bdrv_sum_allocated_file_size(BlockDriverState *bs) | ||
38 | +static int64_t coroutine_fn bdrv_sum_allocated_file_size(BlockDriverState *bs) | ||
39 | { | ||
40 | BdrvChild *child; | ||
41 | int64_t child_size, sum = 0; | ||
42 | -- | ||
43 | 2.40.0 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | 1 | From: Emanuele Giuseppe Esposito <eesposit@redhat.com> | |
2 | |||
3 | Remove usage of aio_context_acquire by always submitting asynchronous | ||
4 | AIO to the current thread's LinuxAioState. | ||
5 | |||
6 | In order to prevent mistakes from the caller side, avoid passing LinuxAioState | ||
7 | in laio_io_{plug/unplug} and laio_co_submit, and document the functions | ||
8 | to make clear that they work in the current thread's AioContext. | ||
9 | |||
10 | Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||
11 | Message-Id: <20230203131731.851116-2-eesposit@redhat.com> | ||
12 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
13 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
14 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
15 | --- | ||
16 | include/block/aio.h | 4 ---- | ||
17 | include/block/raw-aio.h | 18 ++++++++++++------ | ||
18 | include/sysemu/block-backend-io.h | 5 +++++ | ||
19 | block/file-posix.c | 10 +++------- | ||
20 | block/linux-aio.c | 29 +++++++++++++++++------------ | ||
21 | 5 files changed, 37 insertions(+), 29 deletions(-) | ||
22 | |||
23 | diff --git a/include/block/aio.h b/include/block/aio.h | ||
24 | index XXXXXXX..XXXXXXX 100644 | ||
25 | --- a/include/block/aio.h | ||
26 | +++ b/include/block/aio.h | ||
27 | @@ -XXX,XX +XXX,XX @@ struct AioContext { | ||
28 | struct ThreadPool *thread_pool; | ||
29 | |||
30 | #ifdef CONFIG_LINUX_AIO | ||
31 | - /* | ||
32 | - * State for native Linux AIO. Uses aio_context_acquire/release for | ||
33 | - * locking. | ||
34 | - */ | ||
35 | struct LinuxAioState *linux_aio; | ||
36 | #endif | ||
37 | #ifdef CONFIG_LINUX_IO_URING | ||
38 | diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h | ||
39 | index XXXXXXX..XXXXXXX 100644 | ||
40 | --- a/include/block/raw-aio.h | ||
41 | +++ b/include/block/raw-aio.h | ||
42 | @@ -XXX,XX +XXX,XX @@ | ||
43 | typedef struct LinuxAioState LinuxAioState; | ||
44 | LinuxAioState *laio_init(Error **errp); | ||
45 | void laio_cleanup(LinuxAioState *s); | ||
46 | -int coroutine_fn laio_co_submit(BlockDriverState *bs, LinuxAioState *s, int fd, | ||
47 | - uint64_t offset, QEMUIOVector *qiov, int type, | ||
48 | - uint64_t dev_max_batch); | ||
49 | + | ||
50 | +/* laio_co_submit: submit I/O requests in the thread's current AioContext. */ | ||
51 | +int coroutine_fn laio_co_submit(int fd, uint64_t offset, QEMUIOVector *qiov, | ||
52 | + int type, uint64_t dev_max_batch); | ||
53 | + | ||
54 | void laio_detach_aio_context(LinuxAioState *s, AioContext *old_context); | ||
55 | void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context); | ||
56 | -void laio_io_plug(BlockDriverState *bs, LinuxAioState *s); | ||
57 | -void laio_io_unplug(BlockDriverState *bs, LinuxAioState *s, | ||
58 | - uint64_t dev_max_batch); | ||
59 | + | ||
60 | +/* | ||
61 | + * laio_io_plug/unplug work in the thread's current AioContext, therefore the | ||
62 | + * caller must ensure that they are paired in the same IOThread. | ||
63 | + */ | ||
64 | +void laio_io_plug(void); | ||
65 | +void laio_io_unplug(uint64_t dev_max_batch); | ||
66 | #endif | ||
67 | /* io_uring.c - Linux io_uring implementation */ | ||
68 | #ifdef CONFIG_LINUX_IO_URING | ||
69 | diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h | ||
70 | index XXXXXXX..XXXXXXX 100644 | ||
71 | --- a/include/sysemu/block-backend-io.h | ||
72 | +++ b/include/sysemu/block-backend-io.h | ||
73 | @@ -XXX,XX +XXX,XX @@ void blk_iostatus_set_err(BlockBackend *blk, int error); | ||
74 | int blk_get_max_iov(BlockBackend *blk); | ||
75 | int blk_get_max_hw_iov(BlockBackend *blk); | ||
76 | |||
77 | +/* | ||
78 | + * blk_io_plug/unplug are thread-local operations. This means that multiple | ||
79 | + * IOThreads can simultaneously call plug/unplug, but the caller must ensure | ||
80 | + * that each unplug() is called in the same IOThread of the matching plug(). | ||
81 | + */ | ||
82 | void coroutine_fn blk_co_io_plug(BlockBackend *blk); | ||
83 | void co_wrapper blk_io_plug(BlockBackend *blk); | ||
84 | |||
85 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
86 | index XXXXXXX..XXXXXXX 100644 | ||
87 | --- a/block/file-posix.c | ||
88 | +++ b/block/file-posix.c | ||
89 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset, | ||
90 | #endif | ||
91 | #ifdef CONFIG_LINUX_AIO | ||
92 | } else if (s->use_linux_aio) { | ||
93 | - LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs)); | ||
94 | assert(qiov->size == bytes); | ||
95 | - return laio_co_submit(bs, aio, s->fd, offset, qiov, type, | ||
96 | - s->aio_max_batch); | ||
97 | + return laio_co_submit(s->fd, offset, qiov, type, s->aio_max_batch); | ||
98 | #endif | ||
99 | } | ||
100 | |||
101 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_io_plug(BlockDriverState *bs) | ||
102 | BDRVRawState __attribute__((unused)) *s = bs->opaque; | ||
103 | #ifdef CONFIG_LINUX_AIO | ||
104 | if (s->use_linux_aio) { | ||
105 | - LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs)); | ||
106 | - laio_io_plug(bs, aio); | ||
107 | + laio_io_plug(); | ||
108 | } | ||
109 | #endif | ||
110 | #ifdef CONFIG_LINUX_IO_URING | ||
111 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs) | ||
112 | BDRVRawState __attribute__((unused)) *s = bs->opaque; | ||
113 | #ifdef CONFIG_LINUX_AIO | ||
114 | if (s->use_linux_aio) { | ||
115 | - LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs)); | ||
116 | - laio_io_unplug(bs, aio, s->aio_max_batch); | ||
117 | + laio_io_unplug(s->aio_max_batch); | ||
118 | } | ||
119 | #endif | ||
120 | #ifdef CONFIG_LINUX_IO_URING | ||
121 | diff --git a/block/linux-aio.c b/block/linux-aio.c | ||
122 | index XXXXXXX..XXXXXXX 100644 | ||
123 | --- a/block/linux-aio.c | ||
124 | +++ b/block/linux-aio.c | ||
125 | @@ -XXX,XX +XXX,XX @@ | ||
126 | #include "qemu/coroutine.h" | ||
127 | #include "qapi/error.h" | ||
128 | |||
129 | +/* Only used for assertions. */ | ||
130 | +#include "qemu/coroutine_int.h" | ||
131 | + | ||
132 | #include <libaio.h> | ||
133 | |||
134 | /* | ||
135 | @@ -XXX,XX +XXX,XX @@ struct LinuxAioState { | ||
136 | io_context_t ctx; | ||
137 | EventNotifier e; | ||
138 | |||
139 | - /* io queue for submit at batch. Protected by AioContext lock. */ | ||
140 | + /* No locking required, only accessed from AioContext home thread */ | ||
141 | LaioQueue io_q; | ||
142 | - | ||
143 | - /* I/O completion processing. Only runs in I/O thread. */ | ||
144 | QEMUBH *completion_bh; | ||
145 | int event_idx; | ||
146 | int event_max; | ||
147 | @@ -XXX,XX +XXX,XX @@ static void qemu_laio_process_completion(struct qemu_laiocb *laiocb) | ||
148 | * later. Coroutines cannot be entered recursively so avoid doing | ||
149 | * that! | ||
150 | */ | ||
151 | + assert(laiocb->co->ctx == laiocb->ctx->aio_context); | ||
152 | if (!qemu_coroutine_entered(laiocb->co)) { | ||
153 | aio_co_wake(laiocb->co); | ||
154 | } | ||
155 | @@ -XXX,XX +XXX,XX @@ static void qemu_laio_process_completions(LinuxAioState *s) | ||
156 | |||
157 | static void qemu_laio_process_completions_and_submit(LinuxAioState *s) | ||
158 | { | ||
159 | - aio_context_acquire(s->aio_context); | ||
160 | qemu_laio_process_completions(s); | ||
161 | |||
162 | if (!s->io_q.plugged && !QSIMPLEQ_EMPTY(&s->io_q.pending)) { | ||
163 | ioq_submit(s); | ||
164 | } | ||
165 | - aio_context_release(s->aio_context); | ||
166 | } | ||
167 | |||
168 | static void qemu_laio_completion_bh(void *opaque) | ||
169 | @@ -XXX,XX +XXX,XX @@ static uint64_t laio_max_batch(LinuxAioState *s, uint64_t dev_max_batch) | ||
170 | return max_batch; | ||
171 | } | ||
172 | |||
173 | -void laio_io_plug(BlockDriverState *bs, LinuxAioState *s) | ||
174 | +void laio_io_plug(void) | ||
175 | { | ||
176 | + AioContext *ctx = qemu_get_current_aio_context(); | ||
177 | + LinuxAioState *s = aio_get_linux_aio(ctx); | ||
178 | + | ||
179 | s->io_q.plugged++; | ||
180 | } | ||
181 | |||
182 | -void laio_io_unplug(BlockDriverState *bs, LinuxAioState *s, | ||
183 | - uint64_t dev_max_batch) | ||
184 | +void laio_io_unplug(uint64_t dev_max_batch) | ||
185 | { | ||
186 | + AioContext *ctx = qemu_get_current_aio_context(); | ||
187 | + LinuxAioState *s = aio_get_linux_aio(ctx); | ||
188 | + | ||
189 | assert(s->io_q.plugged); | ||
190 | s->io_q.plugged--; | ||
191 | |||
192 | @@ -XXX,XX +XXX,XX @@ static int laio_do_submit(int fd, struct qemu_laiocb *laiocb, off_t offset, | ||
193 | return 0; | ||
194 | } | ||
195 | |||
196 | -int coroutine_fn laio_co_submit(BlockDriverState *bs, LinuxAioState *s, int fd, | ||
197 | - uint64_t offset, QEMUIOVector *qiov, int type, | ||
198 | - uint64_t dev_max_batch) | ||
199 | +int coroutine_fn laio_co_submit(int fd, uint64_t offset, QEMUIOVector *qiov, | ||
200 | + int type, uint64_t dev_max_batch) | ||
201 | { | ||
202 | int ret; | ||
203 | + AioContext *ctx = qemu_get_current_aio_context(); | ||
204 | struct qemu_laiocb laiocb = { | ||
205 | .co = qemu_coroutine_self(), | ||
206 | .nbytes = qiov->size, | ||
207 | - .ctx = s, | ||
208 | + .ctx = aio_get_linux_aio(ctx), | ||
209 | .ret = -EINPROGRESS, | ||
210 | .is_read = (type == QEMU_AIO_READ), | ||
211 | .qiov = qiov, | ||
212 | -- | ||
213 | 2.40.0 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||
1 | 2 | ||
3 | Remove usage of aio_context_acquire by always submitting asynchronous | ||
4 | AIO to the current thread's LuringState. | ||
5 | |||
6 | In order to prevent mistakes from the caller side, avoid passing LuringState | ||
7 | in luring_io_{plug/unplug} and luring_co_submit, and document the functions | ||
8 | to make clear that they work in the current thread's AioContext. | ||
9 | |||
10 | Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||
11 | Message-Id: <20230203131731.851116-3-eesposit@redhat.com> | ||
12 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
13 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
14 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
15 | --- | ||
16 | include/block/aio.h | 4 ---- | ||
17 | include/block/raw-aio.h | 15 +++++++++++---- | ||
18 | block/file-posix.c | 12 ++++-------- | ||
19 | block/io_uring.c | 23 +++++++++++++++-------- | ||
20 | 4 files changed, 30 insertions(+), 24 deletions(-) | ||
21 | |||
22 | diff --git a/include/block/aio.h b/include/block/aio.h | ||
23 | index XXXXXXX..XXXXXXX 100644 | ||
24 | --- a/include/block/aio.h | ||
25 | +++ b/include/block/aio.h | ||
26 | @@ -XXX,XX +XXX,XX @@ struct AioContext { | ||
27 | struct LinuxAioState *linux_aio; | ||
28 | #endif | ||
29 | #ifdef CONFIG_LINUX_IO_URING | ||
30 | - /* | ||
31 | - * State for Linux io_uring. Uses aio_context_acquire/release for | ||
32 | - * locking. | ||
33 | - */ | ||
34 | struct LuringState *linux_io_uring; | ||
35 | |||
36 | /* State for file descriptor monitoring using Linux io_uring */ | ||
37 | diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h | ||
38 | index XXXXXXX..XXXXXXX 100644 | ||
39 | --- a/include/block/raw-aio.h | ||
40 | +++ b/include/block/raw-aio.h | ||
41 | @@ -XXX,XX +XXX,XX @@ void laio_io_unplug(uint64_t dev_max_batch); | ||
42 | typedef struct LuringState LuringState; | ||
43 | LuringState *luring_init(Error **errp); | ||
44 | void luring_cleanup(LuringState *s); | ||
45 | -int coroutine_fn luring_co_submit(BlockDriverState *bs, LuringState *s, int fd, | ||
46 | - uint64_t offset, QEMUIOVector *qiov, int type); | ||
47 | + | ||
48 | +/* luring_co_submit: submit I/O requests in the thread's current AioContext. */ | ||
49 | +int coroutine_fn luring_co_submit(BlockDriverState *bs, int fd, uint64_t offset, | ||
50 | + QEMUIOVector *qiov, int type); | ||
51 | void luring_detach_aio_context(LuringState *s, AioContext *old_context); | ||
52 | void luring_attach_aio_context(LuringState *s, AioContext *new_context); | ||
53 | -void luring_io_plug(BlockDriverState *bs, LuringState *s); | ||
54 | -void luring_io_unplug(BlockDriverState *bs, LuringState *s); | ||
55 | + | ||
56 | +/* | ||
57 | + * luring_io_plug/unplug work in the thread's current AioContext, therefore the | ||
58 | + * caller must ensure that they are paired in the same IOThread. | ||
59 | + */ | ||
60 | +void luring_io_plug(void); | ||
61 | +void luring_io_unplug(void); | ||
62 | #endif | ||
63 | |||
64 | #ifdef _WIN32 | ||
65 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
66 | index XXXXXXX..XXXXXXX 100644 | ||
67 | --- a/block/file-posix.c | ||
68 | +++ b/block/file-posix.c | ||
69 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset, | ||
70 | type |= QEMU_AIO_MISALIGNED; | ||
71 | #ifdef CONFIG_LINUX_IO_URING | ||
72 | } else if (s->use_linux_io_uring) { | ||
73 | - LuringState *aio = aio_get_linux_io_uring(bdrv_get_aio_context(bs)); | ||
74 | assert(qiov->size == bytes); | ||
75 | - return luring_co_submit(bs, aio, s->fd, offset, qiov, type); | ||
76 | + return luring_co_submit(bs, s->fd, offset, qiov, type); | ||
77 | #endif | ||
78 | #ifdef CONFIG_LINUX_AIO | ||
79 | } else if (s->use_linux_aio) { | ||
80 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_io_plug(BlockDriverState *bs) | ||
81 | #endif | ||
82 | #ifdef CONFIG_LINUX_IO_URING | ||
83 | if (s->use_linux_io_uring) { | ||
84 | - LuringState *aio = aio_get_linux_io_uring(bdrv_get_aio_context(bs)); | ||
85 | - luring_io_plug(bs, aio); | ||
86 | + luring_io_plug(); | ||
87 | } | ||
88 | #endif | ||
89 | } | ||
90 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs) | ||
91 | #endif | ||
92 | #ifdef CONFIG_LINUX_IO_URING | ||
93 | if (s->use_linux_io_uring) { | ||
94 | - LuringState *aio = aio_get_linux_io_uring(bdrv_get_aio_context(bs)); | ||
95 | - luring_io_unplug(bs, aio); | ||
96 | + luring_io_unplug(); | ||
97 | } | ||
98 | #endif | ||
99 | } | ||
100 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs) | ||
101 | |||
102 | #ifdef CONFIG_LINUX_IO_URING | ||
103 | if (s->use_linux_io_uring) { | ||
104 | - LuringState *aio = aio_get_linux_io_uring(bdrv_get_aio_context(bs)); | ||
105 | - return luring_co_submit(bs, aio, s->fd, 0, NULL, QEMU_AIO_FLUSH); | ||
106 | + return luring_co_submit(bs, s->fd, 0, NULL, QEMU_AIO_FLUSH); | ||
107 | } | ||
108 | #endif | ||
109 | return raw_thread_pool_submit(bs, handle_aiocb_flush, &acb); | ||
110 | diff --git a/block/io_uring.c b/block/io_uring.c | ||
111 | index XXXXXXX..XXXXXXX 100644 | ||
112 | --- a/block/io_uring.c | ||
113 | +++ b/block/io_uring.c | ||
114 | @@ -XXX,XX +XXX,XX @@ | ||
115 | #include "qapi/error.h" | ||
116 | #include "trace.h" | ||
117 | |||
118 | +/* Only used for assertions. */ | ||
119 | +#include "qemu/coroutine_int.h" | ||
120 | + | ||
121 | /* io_uring ring size */ | ||
122 | #define MAX_ENTRIES 128 | ||
123 | |||
124 | @@ -XXX,XX +XXX,XX @@ typedef struct LuringState { | ||
125 | |||
126 | struct io_uring ring; | ||
127 | |||
128 | - /* io queue for submit at batch. Protected by AioContext lock. */ | ||
129 | + /* No locking required, only accessed from AioContext home thread */ | ||
130 | LuringQueue io_q; | ||
131 | |||
132 | - /* I/O completion processing. Only runs in I/O thread. */ | ||
133 | QEMUBH *completion_bh; | ||
134 | } LuringState; | ||
135 | |||
136 | @@ -XXX,XX +XXX,XX @@ end: | ||
137 | * eventually runs later. Coroutines cannot be entered recursively | ||
138 | * so avoid doing that! | ||
139 | */ | ||
140 | + assert(luringcb->co->ctx == s->aio_context); | ||
141 | if (!qemu_coroutine_entered(luringcb->co)) { | ||
142 | aio_co_wake(luringcb->co); | ||
143 | } | ||
144 | @@ -XXX,XX +XXX,XX @@ static int ioq_submit(LuringState *s) | ||
145 | |||
146 | static void luring_process_completions_and_submit(LuringState *s) | ||
147 | { | ||
148 | - aio_context_acquire(s->aio_context); | ||
149 | luring_process_completions(s); | ||
150 | |||
151 | if (!s->io_q.plugged && s->io_q.in_queue > 0) { | ||
152 | ioq_submit(s); | ||
153 | } | ||
154 | - aio_context_release(s->aio_context); | ||
155 | } | ||
156 | |||
157 | static void qemu_luring_completion_bh(void *opaque) | ||
158 | @@ -XXX,XX +XXX,XX @@ static void ioq_init(LuringQueue *io_q) | ||
159 | io_q->blocked = false; | ||
160 | } | ||
161 | |||
162 | -void luring_io_plug(BlockDriverState *bs, LuringState *s) | ||
163 | +void luring_io_plug(void) | ||
164 | { | ||
165 | + AioContext *ctx = qemu_get_current_aio_context(); | ||
166 | + LuringState *s = aio_get_linux_io_uring(ctx); | ||
167 | trace_luring_io_plug(s); | ||
168 | s->io_q.plugged++; | ||
169 | } | ||
170 | |||
171 | -void luring_io_unplug(BlockDriverState *bs, LuringState *s) | ||
172 | +void luring_io_unplug(void) | ||
173 | { | ||
174 | + AioContext *ctx = qemu_get_current_aio_context(); | ||
175 | + LuringState *s = aio_get_linux_io_uring(ctx); | ||
176 | assert(s->io_q.plugged); | ||
177 | trace_luring_io_unplug(s, s->io_q.blocked, s->io_q.plugged, | ||
178 | s->io_q.in_queue, s->io_q.in_flight); | ||
179 | @@ -XXX,XX +XXX,XX @@ static int luring_do_submit(int fd, LuringAIOCB *luringcb, LuringState *s, | ||
180 | return 0; | ||
181 | } | ||
182 | |||
183 | -int coroutine_fn luring_co_submit(BlockDriverState *bs, LuringState *s, int fd, | ||
184 | - uint64_t offset, QEMUIOVector *qiov, int type) | ||
185 | +int coroutine_fn luring_co_submit(BlockDriverState *bs, int fd, uint64_t offset, | ||
186 | + QEMUIOVector *qiov, int type) | ||
187 | { | ||
188 | int ret; | ||
189 | + AioContext *ctx = qemu_get_current_aio_context(); | ||
190 | + LuringState *s = aio_get_linux_io_uring(ctx); | ||
191 | LuringAIOCB luringcb = { | ||
192 | .co = qemu_coroutine_self(), | ||
193 | .ret = -EINPROGRESS, | ||
194 | -- | ||
195 | 2.40.0 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | 1 | From: Emanuele Giuseppe Esposito <eesposit@redhat.com> | |
2 | |||
3 | Use qemu_get_current_aio_context() where possible, since we always | ||
4 | submit work to the current thread anyways. | ||
5 | |||
6 | We want to also be sure that the thread submitting the work is | ||
7 | the same as the one processing the pool, to avoid adding | ||
8 | synchronization to the pool list. | ||
9 | |||
10 | Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||
11 | Message-Id: <20230203131731.851116-4-eesposit@redhat.com> | ||
12 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
13 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
14 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
15 | --- | ||
16 | include/block/thread-pool.h | 5 +++++ | ||
17 | block/file-posix.c | 21 ++++++++++----------- | ||
18 | block/file-win32.c | 2 +- | ||
19 | block/qcow2-threads.c | 2 +- | ||
20 | util/thread-pool.c | 9 ++++----- | ||
21 | 5 files changed, 21 insertions(+), 18 deletions(-) | ||
22 | |||
23 | diff --git a/include/block/thread-pool.h b/include/block/thread-pool.h | ||
24 | index XXXXXXX..XXXXXXX 100644 | ||
25 | --- a/include/block/thread-pool.h | ||
26 | +++ b/include/block/thread-pool.h | ||
27 | @@ -XXX,XX +XXX,XX @@ typedef struct ThreadPool ThreadPool; | ||
28 | ThreadPool *thread_pool_new(struct AioContext *ctx); | ||
29 | void thread_pool_free(ThreadPool *pool); | ||
30 | |||
31 | +/* | ||
32 | + * thread_pool_submit* API: submit I/O requests in the thread's | ||
33 | + * current AioContext. | ||
34 | + */ | ||
35 | BlockAIOCB *thread_pool_submit_aio(ThreadPool *pool, | ||
36 | ThreadPoolFunc *func, void *arg, | ||
37 | BlockCompletionFunc *cb, void *opaque); | ||
38 | int coroutine_fn thread_pool_submit_co(ThreadPool *pool, | ||
39 | ThreadPoolFunc *func, void *arg); | ||
40 | void thread_pool_submit(ThreadPool *pool, ThreadPoolFunc *func, void *arg); | ||
41 | + | ||
42 | void thread_pool_update_params(ThreadPool *pool, struct AioContext *ctx); | ||
43 | |||
44 | #endif | ||
45 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
46 | index XXXXXXX..XXXXXXX 100644 | ||
47 | --- a/block/file-posix.c | ||
48 | +++ b/block/file-posix.c | ||
49 | @@ -XXX,XX +XXX,XX @@ out: | ||
50 | return result; | ||
51 | } | ||
52 | |||
53 | -static int coroutine_fn raw_thread_pool_submit(BlockDriverState *bs, | ||
54 | - ThreadPoolFunc func, void *arg) | ||
55 | +static int coroutine_fn raw_thread_pool_submit(ThreadPoolFunc func, void *arg) | ||
56 | { | ||
57 | /* @bs can be NULL, bdrv_get_aio_context() returns the main context then */ | ||
58 | - ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs)); | ||
59 | + ThreadPool *pool = aio_get_thread_pool(qemu_get_current_aio_context()); | ||
60 | return thread_pool_submit_co(pool, func, arg); | ||
61 | } | ||
62 | |||
63 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset, | ||
64 | }; | ||
65 | |||
66 | assert(qiov->size == bytes); | ||
67 | - return raw_thread_pool_submit(bs, handle_aiocb_rw, &acb); | ||
68 | + return raw_thread_pool_submit(handle_aiocb_rw, &acb); | ||
69 | } | ||
70 | |||
71 | static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset, | ||
72 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs) | ||
73 | return luring_co_submit(bs, s->fd, 0, NULL, QEMU_AIO_FLUSH); | ||
74 | } | ||
75 | #endif | ||
76 | - return raw_thread_pool_submit(bs, handle_aiocb_flush, &acb); | ||
77 | + return raw_thread_pool_submit(handle_aiocb_flush, &acb); | ||
78 | } | ||
79 | |||
80 | static void raw_aio_attach_aio_context(BlockDriverState *bs, | ||
81 | @@ -XXX,XX +XXX,XX @@ raw_regular_truncate(BlockDriverState *bs, int fd, int64_t offset, | ||
82 | }, | ||
83 | }; | ||
84 | |||
85 | - return raw_thread_pool_submit(bs, handle_aiocb_truncate, &acb); | ||
86 | + return raw_thread_pool_submit(handle_aiocb_truncate, &acb); | ||
87 | } | ||
88 | |||
89 | static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset, | ||
90 | @@ -XXX,XX +XXX,XX @@ raw_do_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes, | ||
91 | acb.aio_type |= QEMU_AIO_BLKDEV; | ||
92 | } | ||
93 | |||
94 | - ret = raw_thread_pool_submit(bs, handle_aiocb_discard, &acb); | ||
95 | + ret = raw_thread_pool_submit(handle_aiocb_discard, &acb); | ||
96 | raw_account_discard(s, bytes, ret); | ||
97 | return ret; | ||
98 | } | ||
99 | @@ -XXX,XX +XXX,XX @@ raw_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int64_t bytes, | ||
100 | handler = handle_aiocb_write_zeroes; | ||
101 | } | ||
102 | |||
103 | - return raw_thread_pool_submit(bs, handler, &acb); | ||
104 | + return raw_thread_pool_submit(handler, &acb); | ||
105 | } | ||
106 | |||
107 | static int coroutine_fn raw_co_pwrite_zeroes( | ||
108 | @@ -XXX,XX +XXX,XX @@ raw_co_copy_range_to(BlockDriverState *bs, | ||
109 | }, | ||
110 | }; | ||
111 | |||
112 | - return raw_thread_pool_submit(bs, handle_aiocb_copy_range, &acb); | ||
113 | + return raw_thread_pool_submit(handle_aiocb_copy_range, &acb); | ||
114 | } | ||
115 | |||
116 | BlockDriver bdrv_file = { | ||
117 | @@ -XXX,XX +XXX,XX @@ hdev_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf) | ||
118 | struct sg_io_hdr *io_hdr = buf; | ||
119 | if (io_hdr->cmdp[0] == PERSISTENT_RESERVE_OUT || | ||
120 | io_hdr->cmdp[0] == PERSISTENT_RESERVE_IN) { | ||
121 | - return pr_manager_execute(s->pr_mgr, bdrv_get_aio_context(bs), | ||
122 | + return pr_manager_execute(s->pr_mgr, qemu_get_current_aio_context(), | ||
123 | s->fd, io_hdr); | ||
124 | } | ||
125 | } | ||
126 | @@ -XXX,XX +XXX,XX @@ hdev_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf) | ||
127 | }, | ||
128 | }; | ||
129 | |||
130 | - return raw_thread_pool_submit(bs, handle_aiocb_ioctl, &acb); | ||
131 | + return raw_thread_pool_submit(handle_aiocb_ioctl, &acb); | ||
132 | } | ||
133 | #endif /* linux */ | ||
134 | |||
135 | diff --git a/block/file-win32.c b/block/file-win32.c | ||
136 | index XXXXXXX..XXXXXXX 100644 | ||
137 | --- a/block/file-win32.c | ||
138 | +++ b/block/file-win32.c | ||
139 | @@ -XXX,XX +XXX,XX @@ static BlockAIOCB *paio_submit(BlockDriverState *bs, HANDLE hfile, | ||
140 | acb->aio_offset = offset; | ||
141 | |||
142 | trace_file_paio_submit(acb, opaque, offset, count, type); | ||
143 | - pool = aio_get_thread_pool(bdrv_get_aio_context(bs)); | ||
144 | + pool = aio_get_thread_pool(qemu_get_current_aio_context()); | ||
145 | return thread_pool_submit_aio(pool, aio_worker, acb, cb, opaque); | ||
146 | } | ||
147 | |||
148 | diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c | ||
149 | index XXXXXXX..XXXXXXX 100644 | ||
150 | --- a/block/qcow2-threads.c | ||
151 | +++ b/block/qcow2-threads.c | ||
152 | @@ -XXX,XX +XXX,XX @@ qcow2_co_process(BlockDriverState *bs, ThreadPoolFunc *func, void *arg) | ||
153 | { | ||
154 | int ret; | ||
155 | BDRVQcow2State *s = bs->opaque; | ||
156 | - ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs)); | ||
157 | + ThreadPool *pool = aio_get_thread_pool(qemu_get_current_aio_context()); | ||
158 | |||
159 | qemu_co_mutex_lock(&s->lock); | ||
160 | while (s->nb_threads >= QCOW2_MAX_THREADS) { | ||
161 | diff --git a/util/thread-pool.c b/util/thread-pool.c | ||
162 | index XXXXXXX..XXXXXXX 100644 | ||
163 | --- a/util/thread-pool.c | ||
164 | +++ b/util/thread-pool.c | ||
165 | @@ -XXX,XX +XXX,XX @@ struct ThreadPoolElement { | ||
166 | /* Access to this list is protected by lock. */ | ||
167 | QTAILQ_ENTRY(ThreadPoolElement) reqs; | ||
168 | |||
169 | - /* Access to this list is protected by the global mutex. */ | ||
170 | + /* This list is only written by the thread pool's mother thread. */ | ||
171 | QLIST_ENTRY(ThreadPoolElement) all; | ||
172 | }; | ||
173 | |||
174 | @@ -XXX,XX +XXX,XX @@ static void thread_pool_completion_bh(void *opaque) | ||
175 | ThreadPool *pool = opaque; | ||
176 | ThreadPoolElement *elem, *next; | ||
177 | |||
178 | - aio_context_acquire(pool->ctx); | ||
179 | restart: | ||
180 | QLIST_FOREACH_SAFE(elem, &pool->head, all, next) { | ||
181 | if (elem->state != THREAD_DONE) { | ||
182 | @@ -XXX,XX +XXX,XX @@ restart: | ||
183 | */ | ||
184 | qemu_bh_schedule(pool->completion_bh); | ||
185 | |||
186 | - aio_context_release(pool->ctx); | ||
187 | elem->common.cb(elem->common.opaque, elem->ret); | ||
188 | - aio_context_acquire(pool->ctx); | ||
189 | |||
190 | /* We can safely cancel the completion_bh here regardless of someone | ||
191 | * else having scheduled it meanwhile because we reenter the | ||
192 | @@ -XXX,XX +XXX,XX @@ restart: | ||
193 | qemu_aio_unref(elem); | ||
194 | } | ||
195 | } | ||
196 | - aio_context_release(pool->ctx); | ||
197 | } | ||
198 | |||
199 | static void thread_pool_cancel(BlockAIOCB *acb) | ||
200 | @@ -XXX,XX +XXX,XX @@ BlockAIOCB *thread_pool_submit_aio(ThreadPool *pool, | ||
201 | { | ||
202 | ThreadPoolElement *req; | ||
203 | |||
204 | + /* Assert that the thread submitting work is the same running the pool */ | ||
205 | + assert(pool->ctx == qemu_get_current_aio_context()); | ||
206 | + | ||
207 | req = qemu_aio_get(&thread_pool_aiocb_info, NULL, cb, opaque); | ||
208 | req->func = func; | ||
209 | req->arg = arg; | ||
210 | -- | ||
211 | 2.40.0 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||
1 | 2 | ||
3 | thread_pool_submit_aio() is always called on a pool taken from | ||
4 | qemu_get_current_aio_context(), and that is the only intended | ||
5 | use: each pool runs only in the same thread that is submitting | ||
6 | work to it, it can't run anywhere else. | ||
7 | |||
8 | Therefore simplify the thread_pool_submit* API and remove the | ||
9 | ThreadPool function parameter. | ||
10 | |||
11 | Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||
12 | Message-Id: <20230203131731.851116-5-eesposit@redhat.com> | ||
13 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
14 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
15 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
16 | --- | ||
17 | include/block/thread-pool.h | 10 ++++------ | ||
18 | backends/tpm/tpm_backend.c | 4 +--- | ||
19 | block/file-posix.c | 4 +--- | ||
20 | block/file-win32.c | 4 +--- | ||
21 | block/qcow2-threads.c | 3 +-- | ||
22 | hw/9pfs/coth.c | 3 +-- | ||
23 | hw/ppc/spapr_nvdimm.c | 6 ++---- | ||
24 | hw/virtio/virtio-pmem.c | 3 +-- | ||
25 | scsi/pr-manager.c | 3 +-- | ||
26 | scsi/qemu-pr-helper.c | 3 +-- | ||
27 | tests/unit/test-thread-pool.c | 12 +++++------- | ||
28 | util/thread-pool.c | 16 ++++++++-------- | ||
29 | 12 files changed, 27 insertions(+), 44 deletions(-) | ||
30 | |||
31 | diff --git a/include/block/thread-pool.h b/include/block/thread-pool.h | ||
32 | index XXXXXXX..XXXXXXX 100644 | ||
33 | --- a/include/block/thread-pool.h | ||
34 | +++ b/include/block/thread-pool.h | ||
35 | @@ -XXX,XX +XXX,XX @@ void thread_pool_free(ThreadPool *pool); | ||
36 | * thread_pool_submit* API: submit I/O requests in the thread's | ||
37 | * current AioContext. | ||
38 | */ | ||
39 | -BlockAIOCB *thread_pool_submit_aio(ThreadPool *pool, | ||
40 | - ThreadPoolFunc *func, void *arg, | ||
41 | - BlockCompletionFunc *cb, void *opaque); | ||
42 | -int coroutine_fn thread_pool_submit_co(ThreadPool *pool, | ||
43 | - ThreadPoolFunc *func, void *arg); | ||
44 | -void thread_pool_submit(ThreadPool *pool, ThreadPoolFunc *func, void *arg); | ||
45 | +BlockAIOCB *thread_pool_submit_aio(ThreadPoolFunc *func, void *arg, | ||
46 | + BlockCompletionFunc *cb, void *opaque); | ||
47 | +int coroutine_fn thread_pool_submit_co(ThreadPoolFunc *func, void *arg); | ||
48 | +void thread_pool_submit(ThreadPoolFunc *func, void *arg); | ||
49 | |||
50 | void thread_pool_update_params(ThreadPool *pool, struct AioContext *ctx); | ||
51 | |||
52 | diff --git a/backends/tpm/tpm_backend.c b/backends/tpm/tpm_backend.c | ||
53 | index XXXXXXX..XXXXXXX 100644 | ||
54 | --- a/backends/tpm/tpm_backend.c | ||
55 | +++ b/backends/tpm/tpm_backend.c | ||
56 | @@ -XXX,XX +XXX,XX @@ bool tpm_backend_had_startup_error(TPMBackend *s) | ||
57 | |||
58 | void tpm_backend_deliver_request(TPMBackend *s, TPMBackendCmd *cmd) | ||
59 | { | ||
60 | - ThreadPool *pool = aio_get_thread_pool(qemu_get_aio_context()); | ||
61 | - | ||
62 | if (s->cmd != NULL) { | ||
63 | error_report("There is a TPM request pending"); | ||
64 | return; | ||
65 | @@ -XXX,XX +XXX,XX @@ void tpm_backend_deliver_request(TPMBackend *s, TPMBackendCmd *cmd) | ||
66 | |||
67 | s->cmd = cmd; | ||
68 | object_ref(OBJECT(s)); | ||
69 | - thread_pool_submit_aio(pool, tpm_backend_worker_thread, s, | ||
70 | + thread_pool_submit_aio(tpm_backend_worker_thread, s, | ||
71 | tpm_backend_request_completed, s); | ||
72 | } | ||
73 | |||
74 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
75 | index XXXXXXX..XXXXXXX 100644 | ||
76 | --- a/block/file-posix.c | ||
77 | +++ b/block/file-posix.c | ||
78 | @@ -XXX,XX +XXX,XX @@ out: | ||
79 | |||
80 | static int coroutine_fn raw_thread_pool_submit(ThreadPoolFunc func, void *arg) | ||
81 | { | ||
82 | - /* @bs can be NULL, bdrv_get_aio_context() returns the main context then */ | ||
83 | - ThreadPool *pool = aio_get_thread_pool(qemu_get_current_aio_context()); | ||
84 | - return thread_pool_submit_co(pool, func, arg); | ||
85 | + return thread_pool_submit_co(func, arg); | ||
86 | } | ||
87 | |||
88 | /* | ||
89 | diff --git a/block/file-win32.c b/block/file-win32.c | ||
90 | index XXXXXXX..XXXXXXX 100644 | ||
91 | --- a/block/file-win32.c | ||
92 | +++ b/block/file-win32.c | ||
93 | @@ -XXX,XX +XXX,XX @@ static BlockAIOCB *paio_submit(BlockDriverState *bs, HANDLE hfile, | ||
94 | BlockCompletionFunc *cb, void *opaque, int type) | ||
95 | { | ||
96 | RawWin32AIOData *acb = g_new(RawWin32AIOData, 1); | ||
97 | - ThreadPool *pool; | ||
98 | |||
99 | acb->bs = bs; | ||
100 | acb->hfile = hfile; | ||
101 | @@ -XXX,XX +XXX,XX @@ static BlockAIOCB *paio_submit(BlockDriverState *bs, HANDLE hfile, | ||
102 | acb->aio_offset = offset; | ||
103 | |||
104 | trace_file_paio_submit(acb, opaque, offset, count, type); | ||
105 | - pool = aio_get_thread_pool(qemu_get_current_aio_context()); | ||
106 | - return thread_pool_submit_aio(pool, aio_worker, acb, cb, opaque); | ||
107 | + return thread_pool_submit_aio(aio_worker, acb, cb, opaque); | ||
108 | } | ||
109 | |||
110 | int qemu_ftruncate64(int fd, int64_t length) | ||
111 | diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c | ||
112 | index XXXXXXX..XXXXXXX 100644 | ||
113 | --- a/block/qcow2-threads.c | ||
114 | +++ b/block/qcow2-threads.c | ||
115 | @@ -XXX,XX +XXX,XX @@ qcow2_co_process(BlockDriverState *bs, ThreadPoolFunc *func, void *arg) | ||
116 | { | ||
117 | int ret; | ||
118 | BDRVQcow2State *s = bs->opaque; | ||
119 | - ThreadPool *pool = aio_get_thread_pool(qemu_get_current_aio_context()); | ||
120 | |||
121 | qemu_co_mutex_lock(&s->lock); | ||
122 | while (s->nb_threads >= QCOW2_MAX_THREADS) { | ||
123 | @@ -XXX,XX +XXX,XX @@ qcow2_co_process(BlockDriverState *bs, ThreadPoolFunc *func, void *arg) | ||
124 | s->nb_threads++; | ||
125 | qemu_co_mutex_unlock(&s->lock); | ||
126 | |||
127 | - ret = thread_pool_submit_co(pool, func, arg); | ||
128 | + ret = thread_pool_submit_co(func, arg); | ||
129 | |||
130 | qemu_co_mutex_lock(&s->lock); | ||
131 | s->nb_threads--; | ||
132 | diff --git a/hw/9pfs/coth.c b/hw/9pfs/coth.c | ||
133 | index XXXXXXX..XXXXXXX 100644 | ||
134 | --- a/hw/9pfs/coth.c | ||
135 | +++ b/hw/9pfs/coth.c | ||
136 | @@ -XXX,XX +XXX,XX @@ static int coroutine_enter_func(void *arg) | ||
137 | void co_run_in_worker_bh(void *opaque) | ||
138 | { | ||
139 | Coroutine *co = opaque; | ||
140 | - thread_pool_submit_aio(aio_get_thread_pool(qemu_get_aio_context()), | ||
141 | - coroutine_enter_func, co, coroutine_enter_cb, co); | ||
142 | + thread_pool_submit_aio(coroutine_enter_func, co, coroutine_enter_cb, co); | ||
143 | } | ||
144 | diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c | ||
145 | index XXXXXXX..XXXXXXX 100644 | ||
146 | --- a/hw/ppc/spapr_nvdimm.c | ||
147 | +++ b/hw/ppc/spapr_nvdimm.c | ||
148 | @@ -XXX,XX +XXX,XX @@ static int spapr_nvdimm_flush_post_load(void *opaque, int version_id) | ||
149 | { | ||
150 | SpaprNVDIMMDevice *s_nvdimm = (SpaprNVDIMMDevice *)opaque; | ||
151 | SpaprNVDIMMDeviceFlushState *state; | ||
152 | - ThreadPool *pool = aio_get_thread_pool(qemu_get_aio_context()); | ||
153 | HostMemoryBackend *backend = MEMORY_BACKEND(PC_DIMM(s_nvdimm)->hostmem); | ||
154 | bool is_pmem = object_property_get_bool(OBJECT(backend), "pmem", NULL); | ||
155 | bool pmem_override = object_property_get_bool(OBJECT(s_nvdimm), | ||
156 | @@ -XXX,XX +XXX,XX @@ static int spapr_nvdimm_flush_post_load(void *opaque, int version_id) | ||
157 | } | ||
158 | |||
159 | QLIST_FOREACH(state, &s_nvdimm->pending_nvdimm_flush_states, node) { | ||
160 | - thread_pool_submit_aio(pool, flush_worker_cb, state, | ||
161 | + thread_pool_submit_aio(flush_worker_cb, state, | ||
162 | spapr_nvdimm_flush_completion_cb, state); | ||
163 | } | ||
164 | |||
165 | @@ -XXX,XX +XXX,XX @@ static target_ulong h_scm_flush(PowerPCCPU *cpu, SpaprMachineState *spapr, | ||
166 | PCDIMMDevice *dimm; | ||
167 | HostMemoryBackend *backend = NULL; | ||
168 | SpaprNVDIMMDeviceFlushState *state; | ||
169 | - ThreadPool *pool = aio_get_thread_pool(qemu_get_aio_context()); | ||
170 | int fd; | ||
171 | |||
172 | if (!drc || !drc->dev || | ||
173 | @@ -XXX,XX +XXX,XX @@ static target_ulong h_scm_flush(PowerPCCPU *cpu, SpaprMachineState *spapr, | ||
174 | |||
175 | state->drcidx = drc_index; | ||
176 | |||
177 | - thread_pool_submit_aio(pool, flush_worker_cb, state, | ||
178 | + thread_pool_submit_aio(flush_worker_cb, state, | ||
179 | spapr_nvdimm_flush_completion_cb, state); | ||
180 | |||
181 | continue_token = state->continue_token; | ||
182 | diff --git a/hw/virtio/virtio-pmem.c b/hw/virtio/virtio-pmem.c | ||
183 | index XXXXXXX..XXXXXXX 100644 | ||
184 | --- a/hw/virtio/virtio-pmem.c | ||
185 | +++ b/hw/virtio/virtio-pmem.c | ||
186 | @@ -XXX,XX +XXX,XX @@ static void virtio_pmem_flush(VirtIODevice *vdev, VirtQueue *vq) | ||
187 | VirtIODeviceRequest *req_data; | ||
188 | VirtIOPMEM *pmem = VIRTIO_PMEM(vdev); | ||
189 | HostMemoryBackend *backend = MEMORY_BACKEND(pmem->memdev); | ||
190 | - ThreadPool *pool = aio_get_thread_pool(qemu_get_aio_context()); | ||
191 | |||
192 | trace_virtio_pmem_flush_request(); | ||
193 | req_data = virtqueue_pop(vq, sizeof(VirtIODeviceRequest)); | ||
194 | @@ -XXX,XX +XXX,XX @@ static void virtio_pmem_flush(VirtIODevice *vdev, VirtQueue *vq) | ||
195 | req_data->fd = memory_region_get_fd(&backend->mr); | ||
196 | req_data->pmem = pmem; | ||
197 | req_data->vdev = vdev; | ||
198 | - thread_pool_submit_aio(pool, worker_cb, req_data, done_cb, req_data); | ||
199 | + thread_pool_submit_aio(worker_cb, req_data, done_cb, req_data); | ||
200 | } | ||
201 | |||
202 | static void virtio_pmem_get_config(VirtIODevice *vdev, uint8_t *config) | ||
203 | diff --git a/scsi/pr-manager.c b/scsi/pr-manager.c | ||
204 | index XXXXXXX..XXXXXXX 100644 | ||
205 | --- a/scsi/pr-manager.c | ||
206 | +++ b/scsi/pr-manager.c | ||
207 | @@ -XXX,XX +XXX,XX @@ static int pr_manager_worker(void *opaque) | ||
208 | int coroutine_fn pr_manager_execute(PRManager *pr_mgr, AioContext *ctx, int fd, | ||
209 | struct sg_io_hdr *hdr) | ||
210 | { | ||
211 | - ThreadPool *pool = aio_get_thread_pool(ctx); | ||
212 | PRManagerData data = { | ||
213 | .pr_mgr = pr_mgr, | ||
214 | .fd = fd, | ||
215 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn pr_manager_execute(PRManager *pr_mgr, AioContext *ctx, int fd, | ||
216 | |||
217 | /* The matching object_unref is in pr_manager_worker. */ | ||
218 | object_ref(OBJECT(pr_mgr)); | ||
219 | - return thread_pool_submit_co(pool, pr_manager_worker, &data); | ||
220 | + return thread_pool_submit_co(pr_manager_worker, &data); | ||
221 | } | ||
222 | |||
223 | bool pr_manager_is_connected(PRManager *pr_mgr) | ||
224 | diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c | ||
225 | index XXXXXXX..XXXXXXX 100644 | ||
226 | --- a/scsi/qemu-pr-helper.c | ||
227 | +++ b/scsi/qemu-pr-helper.c | ||
228 | @@ -XXX,XX +XXX,XX @@ static int do_sgio_worker(void *opaque) | ||
229 | static int do_sgio(int fd, const uint8_t *cdb, uint8_t *sense, | ||
230 | uint8_t *buf, int *sz, int dir) | ||
231 | { | ||
232 | - ThreadPool *pool = aio_get_thread_pool(qemu_get_aio_context()); | ||
233 | int r; | ||
234 | |||
235 | PRHelperSGIOData data = { | ||
236 | @@ -XXX,XX +XXX,XX @@ static int do_sgio(int fd, const uint8_t *cdb, uint8_t *sense, | ||
237 | .dir = dir, | ||
238 | }; | ||
239 | |||
240 | - r = thread_pool_submit_co(pool, do_sgio_worker, &data); | ||
241 | + r = thread_pool_submit_co(do_sgio_worker, &data); | ||
242 | *sz = data.sz; | ||
243 | return r; | ||
244 | } | ||
245 | diff --git a/tests/unit/test-thread-pool.c b/tests/unit/test-thread-pool.c | ||
246 | index XXXXXXX..XXXXXXX 100644 | ||
247 | --- a/tests/unit/test-thread-pool.c | ||
248 | +++ b/tests/unit/test-thread-pool.c | ||
249 | @@ -XXX,XX +XXX,XX @@ | ||
250 | #include "qemu/main-loop.h" | ||
251 | |||
252 | static AioContext *ctx; | ||
253 | -static ThreadPool *pool; | ||
254 | static int active; | ||
255 | |||
256 | typedef struct { | ||
257 | @@ -XXX,XX +XXX,XX @@ static void done_cb(void *opaque, int ret) | ||
258 | static void test_submit(void) | ||
259 | { | ||
260 | WorkerTestData data = { .n = 0 }; | ||
261 | - thread_pool_submit(pool, worker_cb, &data); | ||
262 | + thread_pool_submit(worker_cb, &data); | ||
263 | while (data.n == 0) { | ||
264 | aio_poll(ctx, true); | ||
265 | } | ||
266 | @@ -XXX,XX +XXX,XX @@ static void test_submit(void) | ||
267 | static void test_submit_aio(void) | ||
268 | { | ||
269 | WorkerTestData data = { .n = 0, .ret = -EINPROGRESS }; | ||
270 | - data.aiocb = thread_pool_submit_aio(pool, worker_cb, &data, | ||
271 | + data.aiocb = thread_pool_submit_aio(worker_cb, &data, | ||
272 | done_cb, &data); | ||
273 | |||
274 | /* The callbacks are not called until after the first wait. */ | ||
275 | @@ -XXX,XX +XXX,XX @@ static void co_test_cb(void *opaque) | ||
276 | active = 1; | ||
277 | data->n = 0; | ||
278 | data->ret = -EINPROGRESS; | ||
279 | - thread_pool_submit_co(pool, worker_cb, data); | ||
280 | + thread_pool_submit_co(worker_cb, data); | ||
281 | |||
282 | /* The test continues in test_submit_co, after qemu_coroutine_enter... */ | ||
283 | |||
284 | @@ -XXX,XX +XXX,XX @@ static void test_submit_many(void) | ||
285 | for (i = 0; i < 100; i++) { | ||
286 | data[i].n = 0; | ||
287 | data[i].ret = -EINPROGRESS; | ||
288 | - thread_pool_submit_aio(pool, worker_cb, &data[i], done_cb, &data[i]); | ||
289 | + thread_pool_submit_aio(worker_cb, &data[i], done_cb, &data[i]); | ||
290 | } | ||
291 | |||
292 | active = 100; | ||
293 | @@ -XXX,XX +XXX,XX @@ static void do_test_cancel(bool sync) | ||
294 | for (i = 0; i < 100; i++) { | ||
295 | data[i].n = 0; | ||
296 | data[i].ret = -EINPROGRESS; | ||
297 | - data[i].aiocb = thread_pool_submit_aio(pool, long_cb, &data[i], | ||
298 | + data[i].aiocb = thread_pool_submit_aio(long_cb, &data[i], | ||
299 | done_cb, &data[i]); | ||
300 | } | ||
301 | |||
302 | @@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv) | ||
303 | { | ||
304 | qemu_init_main_loop(&error_abort); | ||
305 | ctx = qemu_get_current_aio_context(); | ||
306 | - pool = aio_get_thread_pool(ctx); | ||
307 | |||
308 | g_test_init(&argc, &argv, NULL); | ||
309 | g_test_add_func("/thread-pool/submit", test_submit); | ||
310 | diff --git a/util/thread-pool.c b/util/thread-pool.c | ||
311 | index XXXXXXX..XXXXXXX 100644 | ||
312 | --- a/util/thread-pool.c | ||
313 | +++ b/util/thread-pool.c | ||
314 | @@ -XXX,XX +XXX,XX @@ static const AIOCBInfo thread_pool_aiocb_info = { | ||
315 | .get_aio_context = thread_pool_get_aio_context, | ||
316 | }; | ||
317 | |||
318 | -BlockAIOCB *thread_pool_submit_aio(ThreadPool *pool, | ||
319 | - ThreadPoolFunc *func, void *arg, | ||
320 | - BlockCompletionFunc *cb, void *opaque) | ||
321 | +BlockAIOCB *thread_pool_submit_aio(ThreadPoolFunc *func, void *arg, | ||
322 | + BlockCompletionFunc *cb, void *opaque) | ||
323 | { | ||
324 | ThreadPoolElement *req; | ||
325 | + AioContext *ctx = qemu_get_current_aio_context(); | ||
326 | + ThreadPool *pool = aio_get_thread_pool(ctx); | ||
327 | |||
328 | /* Assert that the thread submitting work is the same running the pool */ | ||
329 | assert(pool->ctx == qemu_get_current_aio_context()); | ||
330 | @@ -XXX,XX +XXX,XX @@ static void thread_pool_co_cb(void *opaque, int ret) | ||
331 | aio_co_wake(co->co); | ||
332 | } | ||
333 | |||
334 | -int coroutine_fn thread_pool_submit_co(ThreadPool *pool, ThreadPoolFunc *func, | ||
335 | - void *arg) | ||
336 | +int coroutine_fn thread_pool_submit_co(ThreadPoolFunc *func, void *arg) | ||
337 | { | ||
338 | ThreadPoolCo tpc = { .co = qemu_coroutine_self(), .ret = -EINPROGRESS }; | ||
339 | assert(qemu_in_coroutine()); | ||
340 | - thread_pool_submit_aio(pool, func, arg, thread_pool_co_cb, &tpc); | ||
341 | + thread_pool_submit_aio(func, arg, thread_pool_co_cb, &tpc); | ||
342 | qemu_coroutine_yield(); | ||
343 | return tpc.ret; | ||
344 | } | ||
345 | |||
346 | -void thread_pool_submit(ThreadPool *pool, ThreadPoolFunc *func, void *arg) | ||
347 | +void thread_pool_submit(ThreadPoolFunc *func, void *arg) | ||
348 | { | ||
349 | - thread_pool_submit_aio(pool, func, arg, NULL, NULL); | ||
350 | + thread_pool_submit_aio(func, arg, NULL, NULL); | ||
351 | } | ||
352 | |||
353 | void thread_pool_update_params(ThreadPool *pool, AioContext *ctx) | ||
354 | -- | ||
355 | 2.40.0 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | 1 | From: Paolo Bonzini <pbonzini@redhat.com> | |
2 | |||
3 | Functions that can do I/O are prime candidates for being coroutine_fns. Make the | ||
4 | change for those that are themselves called only from coroutine_fns. | ||
5 | |||
6 | In addition, coroutine_fns should do I/O using bdrv_co_*() functions, for | ||
7 | which it is required to hold the BlockDriverState graph lock. So also nnotate | ||
8 | functions on the I/O path with TSA attributes, making it possible to | ||
9 | switch them to use bdrv_co_*() functions. | ||
10 | |||
11 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
12 | Message-Id: <20230309084456.304669-2-pbonzini@redhat.com> | ||
13 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
14 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
15 | --- | ||
16 | block/vvfat.c | 58 ++++++++++++++++++++++++++------------------------- | ||
17 | 1 file changed, 30 insertions(+), 28 deletions(-) | ||
18 | |||
19 | diff --git a/block/vvfat.c b/block/vvfat.c | ||
20 | index XXXXXXX..XXXXXXX 100644 | ||
21 | --- a/block/vvfat.c | ||
22 | +++ b/block/vvfat.c | ||
23 | @@ -XXX,XX +XXX,XX @@ static BDRVVVFATState *vvv = NULL; | ||
24 | #endif | ||
25 | |||
26 | static int enable_write_target(BlockDriverState *bs, Error **errp); | ||
27 | -static int is_consistent(BDRVVVFATState *s); | ||
28 | +static int coroutine_fn is_consistent(BDRVVVFATState *s); | ||
29 | |||
30 | static QemuOptsList runtime_opts = { | ||
31 | .name = "vvfat", | ||
32 | @@ -XXX,XX +XXX,XX @@ static void print_mapping(const mapping_t* mapping) | ||
33 | } | ||
34 | #endif | ||
35 | |||
36 | -static int vvfat_read(BlockDriverState *bs, int64_t sector_num, | ||
37 | - uint8_t *buf, int nb_sectors) | ||
38 | +static int coroutine_fn GRAPH_RDLOCK | ||
39 | +vvfat_read(BlockDriverState *bs, int64_t sector_num, uint8_t *buf, int nb_sectors) | ||
40 | { | ||
41 | BDRVVVFATState *s = bs->opaque; | ||
42 | int i; | ||
43 | @@ -XXX,XX +XXX,XX @@ static int vvfat_read(BlockDriverState *bs, int64_t sector_num, | ||
44 | DLOG(fprintf(stderr, "sectors %" PRId64 "+%" PRId64 | ||
45 | " allocated\n", sector_num, | ||
46 | n >> BDRV_SECTOR_BITS)); | ||
47 | - if (bdrv_pread(s->qcow, sector_num * BDRV_SECTOR_SIZE, n, | ||
48 | - buf + i * 0x200, 0) < 0) { | ||
49 | + if (bdrv_co_pread(s->qcow, sector_num * BDRV_SECTOR_SIZE, n, | ||
50 | + buf + i * 0x200, 0) < 0) { | ||
51 | return -1; | ||
52 | } | ||
53 | i += (n >> BDRV_SECTOR_BITS) - 1; | ||
54 | @@ -XXX,XX +XXX,XX @@ static int vvfat_read(BlockDriverState *bs, int64_t sector_num, | ||
55 | return 0; | ||
56 | } | ||
57 | |||
58 | -static int coroutine_fn | ||
59 | +static int coroutine_fn GRAPH_RDLOCK | ||
60 | vvfat_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes, | ||
61 | QEMUIOVector *qiov, BdrvRequestFlags flags) | ||
62 | { | ||
63 | @@ -XXX,XX +XXX,XX @@ static inline uint32_t modified_fat_get(BDRVVVFATState* s, | ||
64 | } | ||
65 | } | ||
66 | |||
67 | -static inline bool cluster_was_modified(BDRVVVFATState *s, | ||
68 | - uint32_t cluster_num) | ||
69 | +static inline bool coroutine_fn GRAPH_RDLOCK | ||
70 | +cluster_was_modified(BDRVVVFATState *s, uint32_t cluster_num) | ||
71 | { | ||
72 | int was_modified = 0; | ||
73 | int i; | ||
74 | @@ -XXX,XX +XXX,XX @@ typedef enum { | ||
75 | * Further, the files/directories handled by this function are | ||
76 | * assumed to be *not* deleted (and *only* those). | ||
77 | */ | ||
78 | -static uint32_t get_cluster_count_for_direntry(BDRVVVFATState* s, | ||
79 | - direntry_t* direntry, const char* path) | ||
80 | +static uint32_t coroutine_fn GRAPH_RDLOCK | ||
81 | +get_cluster_count_for_direntry(BDRVVVFATState* s, direntry_t* direntry, const char* path) | ||
82 | { | ||
83 | /* | ||
84 | * This is a little bit tricky: | ||
85 | @@ -XXX,XX +XXX,XX @@ static uint32_t get_cluster_count_for_direntry(BDRVVVFATState* s, | ||
86 | if (res) { | ||
87 | return -1; | ||
88 | } | ||
89 | - res = bdrv_pwrite(s->qcow, offset * BDRV_SECTOR_SIZE, | ||
90 | - BDRV_SECTOR_SIZE, s->cluster_buffer, | ||
91 | - 0); | ||
92 | + res = bdrv_co_pwrite(s->qcow, offset * BDRV_SECTOR_SIZE, | ||
93 | + BDRV_SECTOR_SIZE, s->cluster_buffer, | ||
94 | + 0); | ||
95 | if (res < 0) { | ||
96 | return -2; | ||
97 | } | ||
98 | @@ -XXX,XX +XXX,XX @@ static uint32_t get_cluster_count_for_direntry(BDRVVVFATState* s, | ||
99 | * It returns 0 upon inconsistency or error, and the number of clusters | ||
100 | * used by the directory, its subdirectories and their files. | ||
101 | */ | ||
102 | -static int check_directory_consistency(BDRVVVFATState *s, | ||
103 | - int cluster_num, const char* path) | ||
104 | +static int coroutine_fn GRAPH_RDLOCK | ||
105 | +check_directory_consistency(BDRVVVFATState *s, int cluster_num, const char* path) | ||
106 | { | ||
107 | int ret = 0; | ||
108 | unsigned char* cluster = g_malloc(s->cluster_size); | ||
109 | @@ -XXX,XX +XXX,XX @@ DLOG(fprintf(stderr, "check direntry %d:\n", i); print_direntry(direntries + i)) | ||
110 | } | ||
111 | |||
112 | /* returns 1 on success */ | ||
113 | -static int is_consistent(BDRVVVFATState* s) | ||
114 | +static int coroutine_fn GRAPH_RDLOCK | ||
115 | +is_consistent(BDRVVVFATState* s) | ||
116 | { | ||
117 | int i, check; | ||
118 | int used_clusters_count = 0; | ||
119 | @@ -XXX,XX +XXX,XX @@ static int commit_mappings(BDRVVVFATState* s, | ||
120 | return 0; | ||
121 | } | ||
122 | |||
123 | -static int commit_direntries(BDRVVVFATState* s, | ||
124 | - int dir_index, int parent_mapping_index) | ||
125 | +static int coroutine_fn GRAPH_RDLOCK | ||
126 | +commit_direntries(BDRVVVFATState* s, int dir_index, int parent_mapping_index) | ||
127 | { | ||
128 | direntry_t* direntry = array_get(&(s->directory), dir_index); | ||
129 | uint32_t first_cluster = dir_index == 0 ? 0 : begin_of_direntry(direntry); | ||
130 | @@ -XXX,XX +XXX,XX @@ static int commit_direntries(BDRVVVFATState* s, | ||
131 | |||
132 | /* commit one file (adjust contents, adjust mapping), | ||
133 | return first_mapping_index */ | ||
134 | -static int commit_one_file(BDRVVVFATState* s, | ||
135 | - int dir_index, uint32_t offset) | ||
136 | +static int coroutine_fn GRAPH_RDLOCK | ||
137 | +commit_one_file(BDRVVVFATState* s, int dir_index, uint32_t offset) | ||
138 | { | ||
139 | direntry_t* direntry = array_get(&(s->directory), dir_index); | ||
140 | uint32_t c = begin_of_direntry(direntry); | ||
141 | @@ -XXX,XX +XXX,XX @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s) | ||
142 | /* | ||
143 | * TODO: make sure that the short name is not matching *another* file | ||
144 | */ | ||
145 | -static int handle_commits(BDRVVVFATState* s) | ||
146 | +static int coroutine_fn GRAPH_RDLOCK handle_commits(BDRVVVFATState* s) | ||
147 | { | ||
148 | int i, fail = 0; | ||
149 | |||
150 | @@ -XXX,XX +XXX,XX @@ static int handle_deletes(BDRVVVFATState* s) | ||
151 | * - recurse direntries from root (using bs->bdrv_pread) | ||
152 | * - delete files corresponding to mappings marked as deleted | ||
153 | */ | ||
154 | -static int do_commit(BDRVVVFATState* s) | ||
155 | +static int coroutine_fn GRAPH_RDLOCK do_commit(BDRVVVFATState* s) | ||
156 | { | ||
157 | int ret = 0; | ||
158 | |||
159 | @@ -XXX,XX +XXX,XX @@ DLOG(checkpoint()); | ||
160 | return 0; | ||
161 | } | ||
162 | |||
163 | -static int try_commit(BDRVVVFATState* s) | ||
164 | +static int coroutine_fn GRAPH_RDLOCK try_commit(BDRVVVFATState* s) | ||
165 | { | ||
166 | vvfat_close_current_file(s); | ||
167 | DLOG(checkpoint()); | ||
168 | @@ -XXX,XX +XXX,XX @@ DLOG(checkpoint()); | ||
169 | return do_commit(s); | ||
170 | } | ||
171 | |||
172 | -static int vvfat_write(BlockDriverState *bs, int64_t sector_num, | ||
173 | - const uint8_t *buf, int nb_sectors) | ||
174 | +static int coroutine_fn GRAPH_RDLOCK | ||
175 | +vvfat_write(BlockDriverState *bs, int64_t sector_num, | ||
176 | + const uint8_t *buf, int nb_sectors) | ||
177 | { | ||
178 | BDRVVVFATState *s = bs->opaque; | ||
179 | int i, ret; | ||
180 | @@ -XXX,XX +XXX,XX @@ DLOG(checkpoint()); | ||
181 | * Use qcow backend. Commit later. | ||
182 | */ | ||
183 | DLOG(fprintf(stderr, "Write to qcow backend: %d + %d\n", (int)sector_num, nb_sectors)); | ||
184 | - ret = bdrv_pwrite(s->qcow, sector_num * BDRV_SECTOR_SIZE, | ||
185 | - nb_sectors * BDRV_SECTOR_SIZE, buf, 0); | ||
186 | + ret = bdrv_co_pwrite(s->qcow, sector_num * BDRV_SECTOR_SIZE, | ||
187 | + nb_sectors * BDRV_SECTOR_SIZE, buf, 0); | ||
188 | if (ret < 0) { | ||
189 | fprintf(stderr, "Error writing to qcow backend\n"); | ||
190 | return ret; | ||
191 | @@ -XXX,XX +XXX,XX @@ DLOG(checkpoint()); | ||
192 | return 0; | ||
193 | } | ||
194 | |||
195 | -static int coroutine_fn | ||
196 | +static int coroutine_fn GRAPH_RDLOCK | ||
197 | vvfat_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes, | ||
198 | QEMUIOVector *qiov, BdrvRequestFlags flags) | ||
199 | { | ||
200 | -- | ||
201 | 2.40.0 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
1 | 2 | ||
3 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
4 | Message-Id: <20230309084456.304669-3-pbonzini@redhat.com> | ||
5 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
6 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
7 | --- | ||
8 | block/blkdebug.c | 4 ++-- | ||
9 | 1 file changed, 2 insertions(+), 2 deletions(-) | ||
10 | |||
11 | diff --git a/block/blkdebug.c b/block/blkdebug.c | ||
12 | index XXXXXXX..XXXXXXX 100644 | ||
13 | --- a/block/blkdebug.c | ||
14 | +++ b/block/blkdebug.c | ||
15 | @@ -XXX,XX +XXX,XX @@ out: | ||
16 | return ret; | ||
17 | } | ||
18 | |||
19 | -static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes, | ||
20 | - BlkdebugIOType iotype) | ||
21 | +static int coroutine_fn rule_check(BlockDriverState *bs, uint64_t offset, | ||
22 | + uint64_t bytes, BlkdebugIOType iotype) | ||
23 | { | ||
24 | BDRVBlkdebugState *s = bs->opaque; | ||
25 | BlkdebugRule *rule = NULL; | ||
26 | -- | ||
27 | 2.40.0 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
1 | 2 | ||
3 | mirror_flush calls a mixed function blk_flush but it is only called | ||
4 | from mirror_run; so call the coroutine version and make mirror_flush | ||
5 | a coroutine_fn too. | ||
6 | |||
7 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
8 | Message-Id: <20230309084456.304669-4-pbonzini@redhat.com> | ||
9 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
11 | --- | ||
12 | block/mirror.c | 4 ++-- | ||
13 | 1 file changed, 2 insertions(+), 2 deletions(-) | ||
14 | |||
15 | diff --git a/block/mirror.c b/block/mirror.c | ||
16 | index XXXXXXX..XXXXXXX 100644 | ||
17 | --- a/block/mirror.c | ||
18 | +++ b/block/mirror.c | ||
19 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) | ||
20 | /* Called when going out of the streaming phase to flush the bulk of the | ||
21 | * data to the medium, or just before completing. | ||
22 | */ | ||
23 | -static int mirror_flush(MirrorBlockJob *s) | ||
24 | +static int coroutine_fn mirror_flush(MirrorBlockJob *s) | ||
25 | { | ||
26 | - int ret = blk_flush(s->target); | ||
27 | + int ret = blk_co_flush(s->target); | ||
28 | if (ret < 0) { | ||
29 | if (mirror_error_action(s, false, -ret) == BLOCK_ERROR_ACTION_REPORT) { | ||
30 | s->ret = ret; | ||
31 | -- | ||
32 | 2.40.0 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
1 | 2 | ||
3 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
4 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
5 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
6 | --- | ||
7 | nbd/server.c | 48 ++++++++++++++++++++++++------------------------ | ||
8 | 1 file changed, 24 insertions(+), 24 deletions(-) | ||
9 | |||
10 | diff --git a/nbd/server.c b/nbd/server.c | ||
11 | index XXXXXXX..XXXXXXX 100644 | ||
12 | --- a/nbd/server.c | ||
13 | +++ b/nbd/server.c | ||
14 | @@ -XXX,XX +XXX,XX @@ nbd_read_eof(NBDClient *client, void *buffer, size_t size, Error **errp) | ||
15 | return 1; | ||
16 | } | ||
17 | |||
18 | -static int nbd_receive_request(NBDClient *client, NBDRequest *request, | ||
19 | - Error **errp) | ||
20 | +static int coroutine_fn nbd_receive_request(NBDClient *client, NBDRequest *request, | ||
21 | + Error **errp) | ||
22 | { | ||
23 | uint8_t buf[NBD_REQUEST_SIZE]; | ||
24 | uint32_t magic; | ||
25 | @@ -XXX,XX +XXX,XX @@ static inline void set_be_simple_reply(NBDSimpleReply *reply, uint64_t error, | ||
26 | stq_be_p(&reply->handle, handle); | ||
27 | } | ||
28 | |||
29 | -static int nbd_co_send_simple_reply(NBDClient *client, | ||
30 | - uint64_t handle, | ||
31 | - uint32_t error, | ||
32 | - void *data, | ||
33 | - size_t len, | ||
34 | - Error **errp) | ||
35 | +static int coroutine_fn nbd_co_send_simple_reply(NBDClient *client, | ||
36 | + uint64_t handle, | ||
37 | + uint32_t error, | ||
38 | + void *data, | ||
39 | + size_t len, | ||
40 | + Error **errp) | ||
41 | { | ||
42 | NBDSimpleReply reply; | ||
43 | int nbd_err = system_errno_to_nbd_errno(error); | ||
44 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client, | ||
45 | stl_be_p(&chunk.length, pnum); | ||
46 | ret = nbd_co_send_iov(client, iov, 1, errp); | ||
47 | } else { | ||
48 | - ret = blk_pread(exp->common.blk, offset + progress, pnum, | ||
49 | - data + progress, 0); | ||
50 | + ret = blk_co_pread(exp->common.blk, offset + progress, pnum, | ||
51 | + data + progress, 0); | ||
52 | if (ret < 0) { | ||
53 | error_setg_errno(errp, -ret, "reading from file failed"); | ||
54 | break; | ||
55 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn blockalloc_to_extents(BlockBackend *blk, | ||
56 | * @ea is converted to BE by the function | ||
57 | * @last controls whether NBD_REPLY_FLAG_DONE is sent. | ||
58 | */ | ||
59 | -static int nbd_co_send_extents(NBDClient *client, uint64_t handle, | ||
60 | - NBDExtentArray *ea, | ||
61 | - bool last, uint32_t context_id, Error **errp) | ||
62 | +static int coroutine_fn | ||
63 | +nbd_co_send_extents(NBDClient *client, uint64_t handle, NBDExtentArray *ea, | ||
64 | + bool last, uint32_t context_id, Error **errp) | ||
65 | { | ||
66 | NBDStructuredMeta chunk; | ||
67 | struct iovec iov[] = { | ||
68 | @@ -XXX,XX +XXX,XX @@ static void bitmap_to_extents(BdrvDirtyBitmap *bitmap, | ||
69 | bdrv_dirty_bitmap_unlock(bitmap); | ||
70 | } | ||
71 | |||
72 | -static int nbd_co_send_bitmap(NBDClient *client, uint64_t handle, | ||
73 | - BdrvDirtyBitmap *bitmap, uint64_t offset, | ||
74 | - uint32_t length, bool dont_fragment, bool last, | ||
75 | - uint32_t context_id, Error **errp) | ||
76 | +static int coroutine_fn nbd_co_send_bitmap(NBDClient *client, uint64_t handle, | ||
77 | + BdrvDirtyBitmap *bitmap, uint64_t offset, | ||
78 | + uint32_t length, bool dont_fragment, bool last, | ||
79 | + uint32_t context_id, Error **errp) | ||
80 | { | ||
81 | unsigned int nb_extents = dont_fragment ? 1 : NBD_MAX_BLOCK_STATUS_EXTENTS; | ||
82 | g_autoptr(NBDExtentArray) ea = nbd_extent_array_new(nb_extents); | ||
83 | @@ -XXX,XX +XXX,XX @@ static int nbd_co_send_bitmap(NBDClient *client, uint64_t handle, | ||
84 | * to the client (although the caller may still need to disconnect after | ||
85 | * reporting the error). | ||
86 | */ | ||
87 | -static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request, | ||
88 | - Error **errp) | ||
89 | +static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, NBDRequest *request, | ||
90 | + Error **errp) | ||
91 | { | ||
92 | NBDClient *client = req->client; | ||
93 | int valid_flags; | ||
94 | @@ -XXX,XX +XXX,XX @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request, | ||
95 | data, request->len, errp); | ||
96 | } | ||
97 | |||
98 | - ret = blk_pread(exp->common.blk, request->from, request->len, data, 0); | ||
99 | + ret = blk_co_pread(exp->common.blk, request->from, request->len, data, 0); | ||
100 | if (ret < 0) { | ||
101 | return nbd_send_generic_reply(client, request->handle, ret, | ||
102 | "reading from file failed", errp); | ||
103 | @@ -XXX,XX +XXX,XX @@ static coroutine_fn int nbd_handle_request(NBDClient *client, | ||
104 | if (request->flags & NBD_CMD_FLAG_FUA) { | ||
105 | flags |= BDRV_REQ_FUA; | ||
106 | } | ||
107 | - ret = blk_pwrite(exp->common.blk, request->from, request->len, data, | ||
108 | - flags); | ||
109 | + ret = blk_co_pwrite(exp->common.blk, request->from, request->len, data, | ||
110 | + flags); | ||
111 | return nbd_send_generic_reply(client, request->handle, ret, | ||
112 | "writing to file failed", errp); | ||
113 | |||
114 | @@ -XXX,XX +XXX,XX @@ static coroutine_fn int nbd_handle_request(NBDClient *client, | ||
115 | if (request->flags & NBD_CMD_FLAG_FAST_ZERO) { | ||
116 | flags |= BDRV_REQ_NO_FALLBACK; | ||
117 | } | ||
118 | - ret = blk_pwrite_zeroes(exp->common.blk, request->from, request->len, | ||
119 | - flags); | ||
120 | + ret = blk_co_pwrite_zeroes(exp->common.blk, request->from, request->len, | ||
121 | + flags); | ||
122 | return nbd_send_generic_reply(client, request->handle, ret, | ||
123 | "writing to file failed", errp); | ||
124 | |||
125 | -- | ||
126 | 2.40.0 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
1 | 2 | ||
3 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
4 | Message-Id: <20230309084456.304669-6-pbonzini@redhat.com> | ||
5 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
6 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
7 | --- | ||
8 | hw/9pfs/9p.h | 4 ++-- | ||
9 | hw/9pfs/codir.c | 6 +++--- | ||
10 | 2 files changed, 5 insertions(+), 5 deletions(-) | ||
11 | |||
12 | diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h | ||
13 | index XXXXXXX..XXXXXXX 100644 | ||
14 | --- a/hw/9pfs/9p.h | ||
15 | +++ b/hw/9pfs/9p.h | ||
16 | @@ -XXX,XX +XXX,XX @@ typedef struct V9fsDir { | ||
17 | QemuMutex readdir_mutex_L; | ||
18 | } V9fsDir; | ||
19 | |||
20 | -static inline void v9fs_readdir_lock(V9fsDir *dir) | ||
21 | +static inline void coroutine_fn v9fs_readdir_lock(V9fsDir *dir) | ||
22 | { | ||
23 | if (dir->proto_version == V9FS_PROTO_2000U) { | ||
24 | qemu_co_mutex_lock(&dir->readdir_mutex_u); | ||
25 | @@ -XXX,XX +XXX,XX @@ static inline void v9fs_readdir_lock(V9fsDir *dir) | ||
26 | } | ||
27 | } | ||
28 | |||
29 | -static inline void v9fs_readdir_unlock(V9fsDir *dir) | ||
30 | +static inline void coroutine_fn v9fs_readdir_unlock(V9fsDir *dir) | ||
31 | { | ||
32 | if (dir->proto_version == V9FS_PROTO_2000U) { | ||
33 | qemu_co_mutex_unlock(&dir->readdir_mutex_u); | ||
34 | diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c | ||
35 | index XXXXXXX..XXXXXXX 100644 | ||
36 | --- a/hw/9pfs/codir.c | ||
37 | +++ b/hw/9pfs/codir.c | ||
38 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn v9fs_co_readdir(V9fsPDU *pdu, V9fsFidState *fidp, | ||
39 | * | ||
40 | * See v9fs_co_readdir_many() (as its only user) below for details. | ||
41 | */ | ||
42 | -static int do_readdir_many(V9fsPDU *pdu, V9fsFidState *fidp, | ||
43 | - struct V9fsDirEnt **entries, off_t offset, | ||
44 | - int32_t maxsize, bool dostat) | ||
45 | +static int coroutine_fn | ||
46 | +do_readdir_many(V9fsPDU *pdu, V9fsFidState *fidp, struct V9fsDirEnt **entries, | ||
47 | + off_t offset, int32_t maxsize, bool dostat) | ||
48 | { | ||
49 | V9fsState *s = pdu->s; | ||
50 | V9fsString name; | ||
51 | -- | ||
52 | 2.40.0 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
1 | 2 | ||
3 | do_sgio can suspend via the coroutine function thread_pool_submit_co, so it | ||
4 | has to be coroutine_fn as well---and the same is true of all its direct and | ||
5 | indirect callers. | ||
6 | |||
7 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
8 | Message-Id: <20230309084456.304669-7-pbonzini@redhat.com> | ||
9 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
11 | --- | ||
12 | scsi/qemu-pr-helper.c | 22 +++++++++++----------- | ||
13 | 1 file changed, 11 insertions(+), 11 deletions(-) | ||
14 | |||
15 | diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c | ||
16 | index XXXXXXX..XXXXXXX 100644 | ||
17 | --- a/scsi/qemu-pr-helper.c | ||
18 | +++ b/scsi/qemu-pr-helper.c | ||
19 | @@ -XXX,XX +XXX,XX @@ static int do_sgio_worker(void *opaque) | ||
20 | return status; | ||
21 | } | ||
22 | |||
23 | -static int do_sgio(int fd, const uint8_t *cdb, uint8_t *sense, | ||
24 | - uint8_t *buf, int *sz, int dir) | ||
25 | +static int coroutine_fn do_sgio(int fd, const uint8_t *cdb, uint8_t *sense, | ||
26 | + uint8_t *buf, int *sz, int dir) | ||
27 | { | ||
28 | int r; | ||
29 | |||
30 | @@ -XXX,XX +XXX,XX @@ static SCSISense mpath_generic_sense(int r) | ||
31 | } | ||
32 | } | ||
33 | |||
34 | -static int mpath_reconstruct_sense(int fd, int r, uint8_t *sense) | ||
35 | +static int coroutine_fn mpath_reconstruct_sense(int fd, int r, uint8_t *sense) | ||
36 | { | ||
37 | switch (r) { | ||
38 | case MPATH_PR_SUCCESS: | ||
39 | @@ -XXX,XX +XXX,XX @@ static int mpath_reconstruct_sense(int fd, int r, uint8_t *sense) | ||
40 | } | ||
41 | } | ||
42 | |||
43 | -static int multipath_pr_in(int fd, const uint8_t *cdb, uint8_t *sense, | ||
44 | - uint8_t *data, int sz) | ||
45 | +static int coroutine_fn multipath_pr_in(int fd, const uint8_t *cdb, uint8_t *sense, | ||
46 | + uint8_t *data, int sz) | ||
47 | { | ||
48 | int rq_servact = cdb[1]; | ||
49 | struct prin_resp resp; | ||
50 | @@ -XXX,XX +XXX,XX @@ static int multipath_pr_in(int fd, const uint8_t *cdb, uint8_t *sense, | ||
51 | return mpath_reconstruct_sense(fd, r, sense); | ||
52 | } | ||
53 | |||
54 | -static int multipath_pr_out(int fd, const uint8_t *cdb, uint8_t *sense, | ||
55 | - const uint8_t *param, int sz) | ||
56 | +static int coroutine_fn multipath_pr_out(int fd, const uint8_t *cdb, uint8_t *sense, | ||
57 | + const uint8_t *param, int sz) | ||
58 | { | ||
59 | int rq_servact = cdb[1]; | ||
60 | int rq_scope = cdb[2] >> 4; | ||
61 | @@ -XXX,XX +XXX,XX @@ static int multipath_pr_out(int fd, const uint8_t *cdb, uint8_t *sense, | ||
62 | } | ||
63 | #endif | ||
64 | |||
65 | -static int do_pr_in(int fd, const uint8_t *cdb, uint8_t *sense, | ||
66 | - uint8_t *data, int *resp_sz) | ||
67 | +static int coroutine_fn do_pr_in(int fd, const uint8_t *cdb, uint8_t *sense, | ||
68 | + uint8_t *data, int *resp_sz) | ||
69 | { | ||
70 | #ifdef CONFIG_MPATH | ||
71 | if (is_mpath(fd)) { | ||
72 | @@ -XXX,XX +XXX,XX @@ static int do_pr_in(int fd, const uint8_t *cdb, uint8_t *sense, | ||
73 | SG_DXFER_FROM_DEV); | ||
74 | } | ||
75 | |||
76 | -static int do_pr_out(int fd, const uint8_t *cdb, uint8_t *sense, | ||
77 | - const uint8_t *param, int sz) | ||
78 | +static int coroutine_fn do_pr_out(int fd, const uint8_t *cdb, uint8_t *sense, | ||
79 | + const uint8_t *param, int sz) | ||
80 | { | ||
81 | int resp_sz; | ||
82 | |||
83 | -- | ||
84 | 2.40.0 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
1 | 2 | ||
3 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
4 | Message-Id: <20230309084456.304669-8-pbonzini@redhat.com> | ||
5 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
6 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
7 | --- | ||
8 | tests/unit/test-thread-pool.c | 2 +- | ||
9 | 1 file changed, 1 insertion(+), 1 deletion(-) | ||
10 | |||
11 | diff --git a/tests/unit/test-thread-pool.c b/tests/unit/test-thread-pool.c | ||
12 | index XXXXXXX..XXXXXXX 100644 | ||
13 | --- a/tests/unit/test-thread-pool.c | ||
14 | +++ b/tests/unit/test-thread-pool.c | ||
15 | @@ -XXX,XX +XXX,XX @@ static void test_submit_aio(void) | ||
16 | g_assert_cmpint(data.ret, ==, 0); | ||
17 | } | ||
18 | |||
19 | -static void co_test_cb(void *opaque) | ||
20 | +static void coroutine_fn co_test_cb(void *opaque) | ||
21 | { | ||
22 | WorkerTestData *data = opaque; | ||
23 | |||
24 | -- | ||
25 | 2.40.0 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
1 | 2 | ||
3 | Functions that can do I/O (including calling bdrv_is_allocated | ||
4 | and bdrv_block_status functions) are prime candidates for being | ||
5 | coroutine_fns. Make the change for those that are themselves called | ||
6 | only from coroutine_fns. Also annotate that they are called with the | ||
7 | graph rdlock taken, thus allowing them to call bdrv_co_*() functions | ||
8 | for I/O. | ||
9 | |||
10 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
11 | Message-Id: <20230309084456.304669-9-pbonzini@redhat.com> | ||
12 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
13 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
14 | --- | ||
15 | block/qcow2.h | 15 ++++++++------- | ||
16 | block/qcow2-bitmap.c | 2 +- | ||
17 | block/qcow2-cluster.c | 21 +++++++++++++-------- | ||
18 | block/qcow2-refcount.c | 8 ++++---- | ||
19 | block/qcow2-snapshot.c | 25 +++++++++++++------------ | ||
20 | block/qcow2.c | 27 ++++++++++++++------------- | ||
21 | 6 files changed, 53 insertions(+), 45 deletions(-) | ||
22 | |||
23 | diff --git a/block/qcow2.h b/block/qcow2.h | ||
24 | index XXXXXXX..XXXXXXX 100644 | ||
25 | --- a/block/qcow2.h | ||
26 | +++ b/block/qcow2.h | ||
27 | @@ -XXX,XX +XXX,XX @@ int64_t qcow2_refcount_area(BlockDriverState *bs, uint64_t offset, | ||
28 | uint64_t new_refblock_offset); | ||
29 | |||
30 | int64_t qcow2_alloc_clusters(BlockDriverState *bs, uint64_t size); | ||
31 | -int64_t qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset, | ||
32 | - int64_t nb_clusters); | ||
33 | -int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size); | ||
34 | +int64_t coroutine_fn qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset, | ||
35 | + int64_t nb_clusters); | ||
36 | +int64_t coroutine_fn qcow2_alloc_bytes(BlockDriverState *bs, int size); | ||
37 | void qcow2_free_clusters(BlockDriverState *bs, | ||
38 | int64_t offset, int64_t size, | ||
39 | enum qcow2_discard_type type); | ||
40 | @@ -XXX,XX +XXX,XX @@ int qcow2_change_refcount_order(BlockDriverState *bs, int refcount_order, | ||
41 | BlockDriverAmendStatusCB *status_cb, | ||
42 | void *cb_opaque, Error **errp); | ||
43 | int coroutine_fn GRAPH_RDLOCK qcow2_shrink_reftable(BlockDriverState *bs); | ||
44 | -int64_t qcow2_get_last_cluster(BlockDriverState *bs, int64_t size); | ||
45 | +int64_t coroutine_fn qcow2_get_last_cluster(BlockDriverState *bs, int64_t size); | ||
46 | int coroutine_fn qcow2_detect_metadata_preallocation(BlockDriverState *bs); | ||
47 | |||
48 | /* qcow2-cluster.c functions */ | ||
49 | @@ -XXX,XX +XXX,XX @@ void qcow2_parse_compressed_l2_entry(BlockDriverState *bs, uint64_t l2_entry, | ||
50 | int coroutine_fn GRAPH_RDLOCK | ||
51 | qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m); | ||
52 | |||
53 | -void qcow2_alloc_cluster_abort(BlockDriverState *bs, QCowL2Meta *m); | ||
54 | +void coroutine_fn qcow2_alloc_cluster_abort(BlockDriverState *bs, QCowL2Meta *m); | ||
55 | int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset, | ||
56 | uint64_t bytes, enum qcow2_discard_type type, | ||
57 | bool full_discard); | ||
58 | @@ -XXX,XX +XXX,XX @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, | ||
59 | Error **errp); | ||
60 | |||
61 | void qcow2_free_snapshots(BlockDriverState *bs); | ||
62 | -int qcow2_read_snapshots(BlockDriverState *bs, Error **errp); | ||
63 | +int coroutine_fn GRAPH_RDLOCK | ||
64 | +qcow2_read_snapshots(BlockDriverState *bs, Error **errp); | ||
65 | int qcow2_write_snapshots(BlockDriverState *bs); | ||
66 | |||
67 | int coroutine_fn GRAPH_RDLOCK | ||
68 | @@ -XXX,XX +XXX,XX @@ bool coroutine_fn qcow2_load_dirty_bitmaps(BlockDriverState *bs, | ||
69 | bool qcow2_get_bitmap_info_list(BlockDriverState *bs, | ||
70 | Qcow2BitmapInfoList **info_list, Error **errp); | ||
71 | int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp); | ||
72 | -int qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp); | ||
73 | +int coroutine_fn qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp); | ||
74 | bool qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, | ||
75 | bool release_stored, Error **errp); | ||
76 | int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp); | ||
77 | diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c | ||
78 | index XXXXXXX..XXXXXXX 100644 | ||
79 | --- a/block/qcow2-bitmap.c | ||
80 | +++ b/block/qcow2-bitmap.c | ||
81 | @@ -XXX,XX +XXX,XX @@ out: | ||
82 | } | ||
83 | |||
84 | /* Checks to see if it's safe to resize bitmaps */ | ||
85 | -int qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp) | ||
86 | +int coroutine_fn qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp) | ||
87 | { | ||
88 | BDRVQcow2State *s = bs->opaque; | ||
89 | Qcow2BitmapList *bm_list; | ||
90 | diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c | ||
91 | index XXXXXXX..XXXXXXX 100644 | ||
92 | --- a/block/qcow2-cluster.c | ||
93 | +++ b/block/qcow2-cluster.c | ||
94 | @@ -XXX,XX +XXX,XX @@ err: | ||
95 | * Frees the allocated clusters because the request failed and they won't | ||
96 | * actually be linked. | ||
97 | */ | ||
98 | -void qcow2_alloc_cluster_abort(BlockDriverState *bs, QCowL2Meta *m) | ||
99 | +void coroutine_fn qcow2_alloc_cluster_abort(BlockDriverState *bs, QCowL2Meta *m) | ||
100 | { | ||
101 | BDRVQcow2State *s = bs->opaque; | ||
102 | if (!has_data_file(bs) && !m->keep_old_clusters) { | ||
103 | @@ -XXX,XX +XXX,XX @@ void qcow2_alloc_cluster_abort(BlockDriverState *bs, QCowL2Meta *m) | ||
104 | * | ||
105 | * Returns 0 on success, -errno on failure. | ||
106 | */ | ||
107 | -static int calculate_l2_meta(BlockDriverState *bs, uint64_t host_cluster_offset, | ||
108 | - uint64_t guest_offset, unsigned bytes, | ||
109 | - uint64_t *l2_slice, QCowL2Meta **m, bool keep_old) | ||
110 | +static int coroutine_fn calculate_l2_meta(BlockDriverState *bs, | ||
111 | + uint64_t host_cluster_offset, | ||
112 | + uint64_t guest_offset, unsigned bytes, | ||
113 | + uint64_t *l2_slice, QCowL2Meta **m, | ||
114 | + bool keep_old) | ||
115 | { | ||
116 | BDRVQcow2State *s = bs->opaque; | ||
117 | int sc_index, l2_index = offset_to_l2_slice_index(s, guest_offset); | ||
118 | @@ -XXX,XX +XXX,XX @@ out: | ||
119 | * function has been waiting for another request and the allocation must be | ||
120 | * restarted, but the whole request should not be failed. | ||
121 | */ | ||
122 | -static int do_alloc_cluster_offset(BlockDriverState *bs, uint64_t guest_offset, | ||
123 | - uint64_t *host_offset, uint64_t *nb_clusters) | ||
124 | +static int coroutine_fn do_alloc_cluster_offset(BlockDriverState *bs, | ||
125 | + uint64_t guest_offset, | ||
126 | + uint64_t *host_offset, | ||
127 | + uint64_t *nb_clusters) | ||
128 | { | ||
129 | BDRVQcow2State *s = bs->opaque; | ||
130 | |||
131 | @@ -XXX,XX +XXX,XX @@ static int zero_in_l2_slice(BlockDriverState *bs, uint64_t offset, | ||
132 | return nb_clusters; | ||
133 | } | ||
134 | |||
135 | -static int zero_l2_subclusters(BlockDriverState *bs, uint64_t offset, | ||
136 | - unsigned nb_subclusters) | ||
137 | +static int coroutine_fn | ||
138 | +zero_l2_subclusters(BlockDriverState *bs, uint64_t offset, | ||
139 | + unsigned nb_subclusters) | ||
140 | { | ||
141 | BDRVQcow2State *s = bs->opaque; | ||
142 | uint64_t *l2_slice; | ||
143 | diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c | ||
144 | index XXXXXXX..XXXXXXX 100644 | ||
145 | --- a/block/qcow2-refcount.c | ||
146 | +++ b/block/qcow2-refcount.c | ||
147 | @@ -XXX,XX +XXX,XX @@ int64_t qcow2_alloc_clusters(BlockDriverState *bs, uint64_t size) | ||
148 | return offset; | ||
149 | } | ||
150 | |||
151 | -int64_t qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset, | ||
152 | - int64_t nb_clusters) | ||
153 | +int64_t coroutine_fn qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset, | ||
154 | + int64_t nb_clusters) | ||
155 | { | ||
156 | BDRVQcow2State *s = bs->opaque; | ||
157 | uint64_t cluster_index, refcount; | ||
158 | @@ -XXX,XX +XXX,XX @@ int64_t qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset, | ||
159 | |||
160 | /* only used to allocate compressed sectors. We try to allocate | ||
161 | contiguous sectors. size must be <= cluster_size */ | ||
162 | -int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size) | ||
163 | +int64_t coroutine_fn qcow2_alloc_bytes(BlockDriverState *bs, int size) | ||
164 | { | ||
165 | BDRVQcow2State *s = bs->opaque; | ||
166 | int64_t offset; | ||
167 | @@ -XXX,XX +XXX,XX @@ out: | ||
168 | return ret; | ||
169 | } | ||
170 | |||
171 | -int64_t qcow2_get_last_cluster(BlockDriverState *bs, int64_t size) | ||
172 | +int64_t coroutine_fn qcow2_get_last_cluster(BlockDriverState *bs, int64_t size) | ||
173 | { | ||
174 | BDRVQcow2State *s = bs->opaque; | ||
175 | int64_t i; | ||
176 | diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c | ||
177 | index XXXXXXX..XXXXXXX 100644 | ||
178 | --- a/block/qcow2-snapshot.c | ||
179 | +++ b/block/qcow2-snapshot.c | ||
180 | @@ -XXX,XX +XXX,XX @@ void qcow2_free_snapshots(BlockDriverState *bs) | ||
181 | * qcow2_check_refcounts() does not do anything with snapshots' | ||
182 | * extra data.) | ||
183 | */ | ||
184 | -static int qcow2_do_read_snapshots(BlockDriverState *bs, bool repair, | ||
185 | - int *nb_clusters_reduced, | ||
186 | - int *extra_data_dropped, | ||
187 | - Error **errp) | ||
188 | +static coroutine_fn GRAPH_RDLOCK | ||
189 | +int qcow2_do_read_snapshots(BlockDriverState *bs, bool repair, | ||
190 | + int *nb_clusters_reduced, | ||
191 | + int *extra_data_dropped, | ||
192 | + Error **errp) | ||
193 | { | ||
194 | BDRVQcow2State *s = bs->opaque; | ||
195 | QCowSnapshotHeader h; | ||
196 | @@ -XXX,XX +XXX,XX @@ static int qcow2_do_read_snapshots(BlockDriverState *bs, bool repair, | ||
197 | |||
198 | /* Read statically sized part of the snapshot header */ | ||
199 | offset = ROUND_UP(offset, 8); | ||
200 | - ret = bdrv_pread(bs->file, offset, sizeof(h), &h, 0); | ||
201 | + ret = bdrv_co_pread(bs->file, offset, sizeof(h), &h, 0); | ||
202 | if (ret < 0) { | ||
203 | error_setg_errno(errp, -ret, "Failed to read snapshot table"); | ||
204 | goto fail; | ||
205 | @@ -XXX,XX +XXX,XX @@ static int qcow2_do_read_snapshots(BlockDriverState *bs, bool repair, | ||
206 | } | ||
207 | |||
208 | /* Read known extra data */ | ||
209 | - ret = bdrv_pread(bs->file, offset, | ||
210 | - MIN(sizeof(extra), sn->extra_data_size), &extra, 0); | ||
211 | + ret = bdrv_co_pread(bs->file, offset, | ||
212 | + MIN(sizeof(extra), sn->extra_data_size), &extra, 0); | ||
213 | if (ret < 0) { | ||
214 | error_setg_errno(errp, -ret, "Failed to read snapshot table"); | ||
215 | goto fail; | ||
216 | @@ -XXX,XX +XXX,XX @@ static int qcow2_do_read_snapshots(BlockDriverState *bs, bool repair, | ||
217 | /* Store unknown extra data */ | ||
218 | unknown_extra_data_size = sn->extra_data_size - sizeof(extra); | ||
219 | sn->unknown_extra_data = g_malloc(unknown_extra_data_size); | ||
220 | - ret = bdrv_pread(bs->file, offset, unknown_extra_data_size, | ||
221 | - sn->unknown_extra_data, 0); | ||
222 | + ret = bdrv_co_pread(bs->file, offset, unknown_extra_data_size, | ||
223 | + sn->unknown_extra_data, 0); | ||
224 | if (ret < 0) { | ||
225 | error_setg_errno(errp, -ret, | ||
226 | "Failed to read snapshot table"); | ||
227 | @@ -XXX,XX +XXX,XX @@ static int qcow2_do_read_snapshots(BlockDriverState *bs, bool repair, | ||
228 | |||
229 | /* Read snapshot ID */ | ||
230 | sn->id_str = g_malloc(id_str_size + 1); | ||
231 | - ret = bdrv_pread(bs->file, offset, id_str_size, sn->id_str, 0); | ||
232 | + ret = bdrv_co_pread(bs->file, offset, id_str_size, sn->id_str, 0); | ||
233 | if (ret < 0) { | ||
234 | error_setg_errno(errp, -ret, "Failed to read snapshot table"); | ||
235 | goto fail; | ||
236 | @@ -XXX,XX +XXX,XX @@ static int qcow2_do_read_snapshots(BlockDriverState *bs, bool repair, | ||
237 | |||
238 | /* Read snapshot name */ | ||
239 | sn->name = g_malloc(name_size + 1); | ||
240 | - ret = bdrv_pread(bs->file, offset, name_size, sn->name, 0); | ||
241 | + ret = bdrv_co_pread(bs->file, offset, name_size, sn->name, 0); | ||
242 | if (ret < 0) { | ||
243 | error_setg_errno(errp, -ret, "Failed to read snapshot table"); | ||
244 | goto fail; | ||
245 | @@ -XXX,XX +XXX,XX @@ fail: | ||
246 | return ret; | ||
247 | } | ||
248 | |||
249 | -int qcow2_read_snapshots(BlockDriverState *bs, Error **errp) | ||
250 | +int coroutine_fn qcow2_read_snapshots(BlockDriverState *bs, Error **errp) | ||
251 | { | ||
252 | return qcow2_do_read_snapshots(bs, false, NULL, NULL, errp); | ||
253 | } | ||
254 | diff --git a/block/qcow2.c b/block/qcow2.c | ||
255 | index XXXXXXX..XXXXXXX 100644 | ||
256 | --- a/block/qcow2.c | ||
257 | +++ b/block/qcow2.c | ||
258 | @@ -XXX,XX +XXX,XX @@ qcow2_extract_crypto_opts(QemuOpts *opts, const char *fmt, Error **errp) | ||
259 | * unknown magic is skipped (future extension this version knows nothing about) | ||
260 | * return 0 upon success, non-0 otherwise | ||
261 | */ | ||
262 | -static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, | ||
263 | - uint64_t end_offset, void **p_feature_table, | ||
264 | - int flags, bool *need_update_header, | ||
265 | - Error **errp) | ||
266 | +static int coroutine_fn GRAPH_RDLOCK | ||
267 | +qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, | ||
268 | + uint64_t end_offset, void **p_feature_table, | ||
269 | + int flags, bool *need_update_header, Error **errp) | ||
270 | { | ||
271 | BDRVQcow2State *s = bs->opaque; | ||
272 | QCowExtension ext; | ||
273 | @@ -XXX,XX +XXX,XX @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, | ||
274 | printf("attempting to read extended header in offset %lu\n", offset); | ||
275 | #endif | ||
276 | |||
277 | - ret = bdrv_pread(bs->file, offset, sizeof(ext), &ext, 0); | ||
278 | + ret = bdrv_co_pread(bs->file, offset, sizeof(ext), &ext, 0); | ||
279 | if (ret < 0) { | ||
280 | error_setg_errno(errp, -ret, "qcow2_read_extension: ERROR: " | ||
281 | "pread fail from offset %" PRIu64, offset); | ||
282 | @@ -XXX,XX +XXX,XX @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, | ||
283 | sizeof(bs->backing_format)); | ||
284 | return 2; | ||
285 | } | ||
286 | - ret = bdrv_pread(bs->file, offset, ext.len, bs->backing_format, 0); | ||
287 | + ret = bdrv_co_pread(bs->file, offset, ext.len, bs->backing_format, 0); | ||
288 | if (ret < 0) { | ||
289 | error_setg_errno(errp, -ret, "ERROR: ext_backing_format: " | ||
290 | "Could not read format name"); | ||
291 | @@ -XXX,XX +XXX,XX @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, | ||
292 | case QCOW2_EXT_MAGIC_FEATURE_TABLE: | ||
293 | if (p_feature_table != NULL) { | ||
294 | void *feature_table = g_malloc0(ext.len + 2 * sizeof(Qcow2Feature)); | ||
295 | - ret = bdrv_pread(bs->file, offset, ext.len, feature_table, 0); | ||
296 | + ret = bdrv_co_pread(bs->file, offset, ext.len, feature_table, 0); | ||
297 | if (ret < 0) { | ||
298 | error_setg_errno(errp, -ret, "ERROR: ext_feature_table: " | ||
299 | "Could not read table"); | ||
300 | @@ -XXX,XX +XXX,XX @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, | ||
301 | return -EINVAL; | ||
302 | } | ||
303 | |||
304 | - ret = bdrv_pread(bs->file, offset, ext.len, &s->crypto_header, 0); | ||
305 | + ret = bdrv_co_pread(bs->file, offset, ext.len, &s->crypto_header, 0); | ||
306 | if (ret < 0) { | ||
307 | error_setg_errno(errp, -ret, | ||
308 | "Unable to read CRYPTO header extension"); | ||
309 | @@ -XXX,XX +XXX,XX @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, | ||
310 | break; | ||
311 | } | ||
312 | |||
313 | - ret = bdrv_pread(bs->file, offset, ext.len, &bitmaps_ext, 0); | ||
314 | + ret = bdrv_co_pread(bs->file, offset, ext.len, &bitmaps_ext, 0); | ||
315 | if (ret < 0) { | ||
316 | error_setg_errno(errp, -ret, "bitmaps_ext: " | ||
317 | "Could not read ext header"); | ||
318 | @@ -XXX,XX +XXX,XX @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, | ||
319 | case QCOW2_EXT_MAGIC_DATA_FILE: | ||
320 | { | ||
321 | s->image_data_file = g_malloc0(ext.len + 1); | ||
322 | - ret = bdrv_pread(bs->file, offset, ext.len, s->image_data_file, 0); | ||
323 | + ret = bdrv_co_pread(bs->file, offset, ext.len, s->image_data_file, 0); | ||
324 | if (ret < 0) { | ||
325 | error_setg_errno(errp, -ret, | ||
326 | "ERROR: Could not read data file name"); | ||
327 | @@ -XXX,XX +XXX,XX @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, | ||
328 | uext->len = ext.len; | ||
329 | QLIST_INSERT_HEAD(&s->unknown_header_ext, uext, next); | ||
330 | |||
331 | - ret = bdrv_pread(bs->file, offset, uext->len, uext->data, 0); | ||
332 | + ret = bdrv_co_pread(bs->file, offset, uext->len, uext->data, 0); | ||
333 | if (ret < 0) { | ||
334 | error_setg_errno(errp, -ret, "ERROR: unknown extension: " | ||
335 | "Could not read data"); | ||
336 | @@ -XXX,XX +XXX,XX @@ static void qcow2_update_options_abort(BlockDriverState *bs, | ||
337 | qapi_free_QCryptoBlockOpenOptions(r->crypto_opts); | ||
338 | } | ||
339 | |||
340 | -static int qcow2_update_options(BlockDriverState *bs, QDict *options, | ||
341 | - int flags, Error **errp) | ||
342 | +static int coroutine_fn | ||
343 | +qcow2_update_options(BlockDriverState *bs, QDict *options, int flags, | ||
344 | + Error **errp) | ||
345 | { | ||
346 | Qcow2ReopenState r = {}; | ||
347 | int ret; | ||
348 | -- | ||
349 | 2.40.0 | diff view generated by jsdifflib |
1 | From: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | This patch enables making snapshots with blkreplay used in | 3 | Functions that can do I/O are prime candidates for being coroutine_fns. Make the |
4 | block devices. | 4 | change for the one that is itself called only from coroutine_fns. Unfortunately |
5 | This function is required to make bdrv_snapshot_goto without | 5 | vmdk does not use a coroutine_fn for the bulk of the open (like qcow2 does) so |
6 | calling .bdrv_open which is not implemented. | 6 | vmdk_read_cid cannot have the same treatment. |
7 | 7 | ||
8 | Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> | 8 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
9 | Acked-by: Kevin Wolf <kwolf@redhat.com> | 9 | Message-Id: <20230309084456.304669-10-pbonzini@redhat.com> |
10 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
11 | --- | 12 | --- |
12 | block/blkreplay.c | 8 ++++++++ | 13 | block/vmdk.c | 2 +- |
13 | 1 file changed, 8 insertions(+) | 14 | 1 file changed, 1 insertion(+), 1 deletion(-) |
14 | 15 | ||
15 | diff --git a/block/blkreplay.c b/block/blkreplay.c | 16 | diff --git a/block/vmdk.c b/block/vmdk.c |
16 | index XXXXXXX..XXXXXXX 100644 | 17 | index XXXXXXX..XXXXXXX 100644 |
17 | --- a/block/blkreplay.c | 18 | --- a/block/vmdk.c |
18 | +++ b/block/blkreplay.c | 19 | +++ b/block/vmdk.c |
19 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkreplay_co_flush(BlockDriverState *bs) | 20 | @@ -XXX,XX +XXX,XX @@ out: |
20 | return ret; | 21 | return ret; |
21 | } | 22 | } |
22 | 23 | ||
23 | +static int blkreplay_snapshot_goto(BlockDriverState *bs, | 24 | -static int vmdk_is_cid_valid(BlockDriverState *bs) |
24 | + const char *snapshot_id) | 25 | +static int coroutine_fn vmdk_is_cid_valid(BlockDriverState *bs) |
25 | +{ | 26 | { |
26 | + return bdrv_snapshot_goto(bs->file->bs, snapshot_id, NULL); | 27 | BDRVVmdkState *s = bs->opaque; |
27 | +} | 28 | uint32_t cur_pcid; |
28 | + | ||
29 | static BlockDriver bdrv_blkreplay = { | ||
30 | .format_name = "blkreplay", | ||
31 | .instance_size = 0, | ||
32 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_blkreplay = { | ||
33 | .bdrv_co_pwrite_zeroes = blkreplay_co_pwrite_zeroes, | ||
34 | .bdrv_co_pdiscard = blkreplay_co_pdiscard, | ||
35 | .bdrv_co_flush = blkreplay_co_flush, | ||
36 | + | ||
37 | + .bdrv_snapshot_goto = blkreplay_snapshot_goto, | ||
38 | }; | ||
39 | |||
40 | static void bdrv_blkreplay_init(void) | ||
41 | -- | 29 | -- |
42 | 2.20.1 | 30 | 2.40.0 |
43 | |||
44 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Wang Liang <wangliangzz@inspur.com> | ||
1 | 2 | ||
3 | hmp_commit() calls blk_is_available() from a non-coroutine context (and in | ||
4 | the main loop). blk_is_available() is a co_wrapper_mixed_bdrv_rdlock | ||
5 | function, and in the non-coroutine context it calls AIO_WAIT_WHILE(), | ||
6 | which crashes if the aio_context lock is not taken before. | ||
7 | |||
8 | Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1615 | ||
9 | Signed-off-by: Wang Liang <wangliangzz@inspur.com> | ||
10 | Message-Id: <20230424103902.45265-1-wangliangzz@126.com> | ||
11 | Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||
12 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
13 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
14 | --- | ||
15 | block/monitor/block-hmp-cmds.c | 10 ++++++---- | ||
16 | 1 file changed, 6 insertions(+), 4 deletions(-) | ||
17 | |||
18 | diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c | ||
19 | index XXXXXXX..XXXXXXX 100644 | ||
20 | --- a/block/monitor/block-hmp-cmds.c | ||
21 | +++ b/block/monitor/block-hmp-cmds.c | ||
22 | @@ -XXX,XX +XXX,XX @@ void hmp_commit(Monitor *mon, const QDict *qdict) | ||
23 | error_report("Device '%s' not found", device); | ||
24 | return; | ||
25 | } | ||
26 | - if (!blk_is_available(blk)) { | ||
27 | - error_report("Device '%s' has no medium", device); | ||
28 | - return; | ||
29 | - } | ||
30 | |||
31 | bs = bdrv_skip_implicit_filters(blk_bs(blk)); | ||
32 | aio_context = bdrv_get_aio_context(bs); | ||
33 | aio_context_acquire(aio_context); | ||
34 | |||
35 | + if (!blk_is_available(blk)) { | ||
36 | + error_report("Device '%s' has no medium", device); | ||
37 | + aio_context_release(aio_context); | ||
38 | + return; | ||
39 | + } | ||
40 | + | ||
41 | ret = bdrv_commit(bs); | ||
42 | |||
43 | aio_context_release(aio_context); | ||
44 | -- | ||
45 | 2.40.0 | diff view generated by jsdifflib |