1 | The following changes since commit b150cb8f67bf491a49a1cb1c7da151eeacbdbcc9: | 1 | The following changes since commit 848a6caa88b9f082c89c9b41afa975761262981d: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2020-09-29 13:18:54 +0100) | 3 | Merge tag 'migration-20230602-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-06-02 17:33:29 -0700) |
4 | 4 | ||
5 | are available in the Git repository at: | 5 | are available in the Git repository at: |
6 | 6 | ||
7 | https://gitlab.com/stefanha/qemu.git tags/block-pull-request | 7 | https://gitlab.com/hreitz/qemu.git tags/pull-block-2023-06-05 |
8 | 8 | ||
9 | for you to fetch changes up to bc47831ff28d6f5830c9c8d74220131dc54c5253: | 9 | for you to fetch changes up to 42a2890a76f4783cd1c212f27856edcf2b5e8a75: |
10 | 10 | ||
11 | util/vfio-helpers: Rework the IOVA allocator to avoid IOVA reserved regions (2020-09-30 10:23:05 +0100) | 11 | qcow2: add discard-no-unref option (2023-06-05 13:15:42 +0200) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Pull request | 14 | Block patches |
15 | 15 | ||
16 | Note I have switched from GitHub to GitLab. | 16 | - Fix padding of unaligned vectored requests to match the host alignment |
17 | for vectors with 1023 or 1024 buffers | ||
18 | - Refactor and fix bugs in parallels's image check functionality | ||
19 | - Add an option to the qcow2 driver to retain (qcow2-level) allocations | ||
20 | on discard requests from the guest (while still forwarding the discard | ||
21 | to the lower level and marking the range as zero) | ||
17 | 22 | ||
18 | ---------------------------------------------------------------- | 23 | ---------------------------------------------------------------- |
24 | Alexander Ivanov (12): | ||
25 | parallels: Out of image offset in BAT leads to image inflation | ||
26 | parallels: Fix high_off calculation in parallels_co_check() | ||
27 | parallels: Fix image_end_offset and data_end after out-of-image check | ||
28 | parallels: create parallels_set_bat_entry_helper() to assign BAT value | ||
29 | parallels: Use generic infrastructure for BAT writing in | ||
30 | parallels_co_check() | ||
31 | parallels: Move check of unclean image to a separate function | ||
32 | parallels: Move check of cluster outside image to a separate function | ||
33 | parallels: Fix statistics calculation | ||
34 | parallels: Move check of leaks to a separate function | ||
35 | parallels: Move statistic collection to a separate function | ||
36 | parallels: Replace qemu_co_mutex_lock by WITH_QEMU_LOCK_GUARD | ||
37 | parallels: Incorrect condition in out-of-image check | ||
19 | 38 | ||
20 | Eric Auger (2): | 39 | Hanna Czenczek (4): |
21 | util/vfio-helpers: Collect IOVA reserved regions | 40 | util/iov: Make qiov_slice() public |
22 | util/vfio-helpers: Rework the IOVA allocator to avoid IOVA reserved | 41 | block: Collapse padded I/O vecs exceeding IOV_MAX |
23 | regions | 42 | util/iov: Remove qemu_iovec_init_extended() |
43 | iotests/iov-padding: New test | ||
24 | 44 | ||
25 | Philippe Mathieu-Daudé (6): | 45 | Jean-Louis Dupond (1): |
26 | util/vfio-helpers: Pass page protections to qemu_vfio_pci_map_bar() | 46 | qcow2: add discard-no-unref option |
27 | block/nvme: Map doorbells pages write-only | ||
28 | block/nvme: Reduce I/O registers scope | ||
29 | block/nvme: Drop NVMeRegs structure, directly use NvmeBar | ||
30 | block/nvme: Use register definitions from 'block/nvme.h' | ||
31 | block/nvme: Replace magic value by SCALE_MS definition | ||
32 | 47 | ||
33 | Stefano Garzarella (1): | 48 | qapi/block-core.json | 12 ++ |
34 | docs: add 'io_uring' option to 'aio' param in qemu-options.hx | 49 | block/qcow2.h | 3 + |
35 | 50 | include/qemu/iov.h | 8 +- | |
36 | Vladimir Sementsov-Ogievskiy (8): | 51 | block/io.c | 166 ++++++++++++++++++-- |
37 | block: return error-code from bdrv_invalidate_cache | 52 | block/parallels.c | 190 ++++++++++++++++------- |
38 | block/io: refactor coroutine wrappers | 53 | block/qcow2-cluster.c | 32 +++- |
39 | block: declare some coroutine functions in block/coroutines.h | 54 | block/qcow2.c | 18 +++ |
40 | scripts: add block-coroutine-wrapper.py | 55 | util/iov.c | 89 ++--------- |
41 | block: generate coroutine-wrapper code | 56 | qemu-options.hx | 12 ++ |
42 | block: drop bdrv_prwv | 57 | tests/qemu-iotests/tests/iov-padding | 85 ++++++++++ |
43 | block/io: refactor save/load vmstate | 58 | tests/qemu-iotests/tests/iov-padding.out | 59 +++++++ |
44 | include/block/block.h: drop non-ascii quotation mark | 59 | 11 files changed, 523 insertions(+), 151 deletions(-) |
45 | 60 | create mode 100755 tests/qemu-iotests/tests/iov-padding | |
46 | block/block-gen.h | 49 ++++ | 61 | create mode 100644 tests/qemu-iotests/tests/iov-padding.out |
47 | block/coroutines.h | 65 +++++ | ||
48 | include/block/block.h | 36 ++- | ||
49 | include/qemu/vfio-helpers.h | 2 +- | ||
50 | block.c | 97 +------ | ||
51 | block/io.c | 339 ++++--------------------- | ||
52 | block/nvme.c | 73 +++--- | ||
53 | tests/test-bdrv-drain.c | 2 +- | ||
54 | util/vfio-helpers.c | 133 +++++++++- | ||
55 | block/meson.build | 8 + | ||
56 | docs/devel/block-coroutine-wrapper.rst | 54 ++++ | ||
57 | docs/devel/index.rst | 1 + | ||
58 | qemu-options.hx | 10 +- | ||
59 | scripts/block-coroutine-wrapper.py | 188 ++++++++++++++ | ||
60 | 14 files changed, 629 insertions(+), 428 deletions(-) | ||
61 | create mode 100644 block/block-gen.h | ||
62 | create mode 100644 block/coroutines.h | ||
63 | create mode 100644 docs/devel/block-coroutine-wrapper.rst | ||
64 | create mode 100644 scripts/block-coroutine-wrapper.py | ||
65 | 62 | ||
66 | -- | 63 | -- |
67 | 2.26.2 | 64 | 2.40.1 |
68 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | We want to inline qemu_iovec_init_extended() in block/io.c for padding |
---|---|---|---|
2 | requests, and having access to qiov_slice() is useful for this. As a | ||
3 | public function, it is renamed to qemu_iovec_slice(). | ||
2 | 4 | ||
3 | This is the only coroutine wrapper from block.c and block/io.c which | 5 | (We will need to count the number of I/O vector elements of a slice |
4 | doesn't return a value, so let's convert it to the common behavior, to | 6 | there, and then later process this slice. Without qiov_slice(), we |
5 | simplify moving to generated coroutine wrappers in a further commit. | 7 | would need to call qemu_iovec_subvec_niov(), and all further |
8 | IOV-processing functions may need to skip prefixing elements to | ||
9 | accomodate for a qiov_offset. Because qemu_iovec_subvec_niov() | ||
10 | internally calls qiov_slice(), we can just have the block/io.c code call | ||
11 | qiov_slice() itself, thus get the number of elements, and also create an | ||
12 | iovec array with the superfluous prefixing elements stripped, so the | ||
13 | following processing functions no longer need to skip them.) | ||
6 | 14 | ||
7 | Also, bdrv_invalidate_cache is a void function, returning error only | 15 | Reviewed-by: Eric Blake <eblake@redhat.com> |
8 | through **errp parameter, which is considered to be bad practice, as | 16 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> |
9 | it forces callers to define and propagate local_err variable, so | 17 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> |
10 | conversion is good anyway. | 18 | Message-Id: <20230411173418.19549-2-hreitz@redhat.com> |
19 | --- | ||
20 | include/qemu/iov.h | 3 +++ | ||
21 | util/iov.c | 14 +++++++------- | ||
22 | 2 files changed, 10 insertions(+), 7 deletions(-) | ||
11 | 23 | ||
12 | This patch leaves the conversion of .bdrv_co_invalidate_cache() driver | 24 | diff --git a/include/qemu/iov.h b/include/qemu/iov.h |
13 | callbacks and bdrv_invalidate_cache_all() for another day. | ||
14 | |||
15 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
17 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
18 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
19 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
20 | Message-Id: <20200924185414.28642-2-vsementsov@virtuozzo.com> | ||
21 | --- | ||
22 | include/block/block.h | 2 +- | ||
23 | block.c | 32 ++++++++++++++++++-------------- | ||
24 | 2 files changed, 19 insertions(+), 15 deletions(-) | ||
25 | |||
26 | diff --git a/include/block/block.h b/include/block/block.h | ||
27 | index XXXXXXX..XXXXXXX 100644 | 25 | index XXXXXXX..XXXXXXX 100644 |
28 | --- a/include/block/block.h | 26 | --- a/include/qemu/iov.h |
29 | +++ b/include/block/block.h | 27 | +++ b/include/qemu/iov.h |
30 | @@ -XXX,XX +XXX,XX @@ void bdrv_aio_cancel_async(BlockAIOCB *acb); | 28 | @@ -XXX,XX +XXX,XX @@ int qemu_iovec_init_extended( |
31 | int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf); | 29 | void *tail_buf, size_t tail_len); |
32 | 30 | void qemu_iovec_init_slice(QEMUIOVector *qiov, QEMUIOVector *source, | |
33 | /* Invalidate any cached metadata used by image formats */ | 31 | size_t offset, size_t len); |
34 | -void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp); | 32 | +struct iovec *qemu_iovec_slice(QEMUIOVector *qiov, |
35 | +int bdrv_invalidate_cache(BlockDriverState *bs, Error **errp); | 33 | + size_t offset, size_t len, |
36 | void bdrv_invalidate_cache_all(Error **errp); | 34 | + size_t *head, size_t *tail, int *niov); |
37 | int bdrv_inactivate_all(void); | 35 | int qemu_iovec_subvec_niov(QEMUIOVector *qiov, size_t offset, size_t len); |
38 | 36 | void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len); | |
39 | diff --git a/block.c b/block.c | 37 | void qemu_iovec_concat(QEMUIOVector *dst, |
38 | diff --git a/util/iov.c b/util/iov.c | ||
40 | index XXXXXXX..XXXXXXX 100644 | 39 | index XXXXXXX..XXXXXXX 100644 |
41 | --- a/block.c | 40 | --- a/util/iov.c |
42 | +++ b/block.c | 41 | +++ b/util/iov.c |
43 | @@ -XXX,XX +XXX,XX @@ void bdrv_init_with_whitelist(void) | 42 | @@ -XXX,XX +XXX,XX @@ static struct iovec *iov_skip_offset(struct iovec *iov, size_t offset, |
44 | bdrv_init(); | ||
45 | } | 43 | } |
46 | 44 | ||
47 | -static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, | 45 | /* |
48 | - Error **errp) | 46 | - * qiov_slice |
49 | +static int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, | 47 | + * qemu_iovec_slice |
50 | + Error **errp) | 48 | * |
49 | * Find subarray of iovec's, containing requested range. @head would | ||
50 | * be offset in first iov (returned by the function), @tail would be | ||
51 | * count of extra bytes in last iovec (returned iov + @niov - 1). | ||
52 | */ | ||
53 | -static struct iovec *qiov_slice(QEMUIOVector *qiov, | ||
54 | - size_t offset, size_t len, | ||
55 | - size_t *head, size_t *tail, int *niov) | ||
56 | +struct iovec *qemu_iovec_slice(QEMUIOVector *qiov, | ||
57 | + size_t offset, size_t len, | ||
58 | + size_t *head, size_t *tail, int *niov) | ||
51 | { | 59 | { |
52 | BdrvChild *child, *parent; | 60 | struct iovec *iov, *end_iov; |
53 | uint64_t perm, shared_perm; | 61 | |
54 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, | 62 | @@ -XXX,XX +XXX,XX @@ int qemu_iovec_subvec_niov(QEMUIOVector *qiov, size_t offset, size_t len) |
55 | BdrvDirtyBitmap *bm; | 63 | size_t head, tail; |
56 | 64 | int niov; | |
57 | if (!bs->drv) { | 65 | |
58 | - return; | 66 | - qiov_slice(qiov, offset, len, &head, &tail, &niov); |
59 | + return -ENOMEDIUM; | 67 | + qemu_iovec_slice(qiov, offset, len, &head, &tail, &niov); |
68 | |||
69 | return niov; | ||
70 | } | ||
71 | @@ -XXX,XX +XXX,XX @@ int qemu_iovec_init_extended( | ||
60 | } | 72 | } |
61 | 73 | ||
62 | QLIST_FOREACH(child, &bs->children, next) { | 74 | if (mid_len) { |
63 | bdrv_co_invalidate_cache(child->bs, &local_err); | 75 | - mid_iov = qiov_slice(mid_qiov, mid_offset, mid_len, |
64 | if (local_err) { | 76 | - &mid_head, &mid_tail, &mid_niov); |
65 | error_propagate(errp, local_err); | 77 | + mid_iov = qemu_iovec_slice(mid_qiov, mid_offset, mid_len, |
66 | - return; | 78 | + &mid_head, &mid_tail, &mid_niov); |
67 | + return -EINVAL; | ||
68 | } | ||
69 | } | 79 | } |
70 | 80 | ||
71 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, | 81 | total_niov = !!head_len + mid_niov + !!tail_len; |
72 | ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL, errp); | ||
73 | if (ret < 0) { | ||
74 | bs->open_flags |= BDRV_O_INACTIVE; | ||
75 | - return; | ||
76 | + return ret; | ||
77 | } | ||
78 | bdrv_set_perm(bs, perm, shared_perm); | ||
79 | |||
80 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, | ||
81 | if (local_err) { | ||
82 | bs->open_flags |= BDRV_O_INACTIVE; | ||
83 | error_propagate(errp, local_err); | ||
84 | - return; | ||
85 | + return -EINVAL; | ||
86 | } | ||
87 | } | ||
88 | |||
89 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, | ||
90 | if (ret < 0) { | ||
91 | bs->open_flags |= BDRV_O_INACTIVE; | ||
92 | error_setg_errno(errp, -ret, "Could not refresh total sector count"); | ||
93 | - return; | ||
94 | + return ret; | ||
95 | } | ||
96 | } | ||
97 | |||
98 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, | ||
99 | if (local_err) { | ||
100 | bs->open_flags |= BDRV_O_INACTIVE; | ||
101 | error_propagate(errp, local_err); | ||
102 | - return; | ||
103 | + return -EINVAL; | ||
104 | } | ||
105 | } | ||
106 | } | ||
107 | + | ||
108 | + return 0; | ||
109 | } | ||
110 | |||
111 | typedef struct InvalidateCacheCo { | ||
112 | BlockDriverState *bs; | ||
113 | Error **errp; | ||
114 | bool done; | ||
115 | + int ret; | ||
116 | } InvalidateCacheCo; | ||
117 | |||
118 | static void coroutine_fn bdrv_invalidate_cache_co_entry(void *opaque) | ||
119 | { | ||
120 | InvalidateCacheCo *ico = opaque; | ||
121 | - bdrv_co_invalidate_cache(ico->bs, ico->errp); | ||
122 | + ico->ret = bdrv_co_invalidate_cache(ico->bs, ico->errp); | ||
123 | ico->done = true; | ||
124 | aio_wait_kick(); | ||
125 | } | ||
126 | |||
127 | -void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp) | ||
128 | +int bdrv_invalidate_cache(BlockDriverState *bs, Error **errp) | ||
129 | { | ||
130 | Coroutine *co; | ||
131 | InvalidateCacheCo ico = { | ||
132 | @@ -XXX,XX +XXX,XX @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp) | ||
133 | bdrv_coroutine_enter(bs, co); | ||
134 | BDRV_POLL_WHILE(bs, !ico.done); | ||
135 | } | ||
136 | + | ||
137 | + return ico.ret; | ||
138 | } | ||
139 | |||
140 | void bdrv_invalidate_cache_all(Error **errp) | ||
141 | { | ||
142 | BlockDriverState *bs; | ||
143 | - Error *local_err = NULL; | ||
144 | BdrvNextIterator it; | ||
145 | |||
146 | for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { | ||
147 | AioContext *aio_context = bdrv_get_aio_context(bs); | ||
148 | + int ret; | ||
149 | |||
150 | aio_context_acquire(aio_context); | ||
151 | - bdrv_invalidate_cache(bs, &local_err); | ||
152 | + ret = bdrv_invalidate_cache(bs, errp); | ||
153 | aio_context_release(aio_context); | ||
154 | - if (local_err) { | ||
155 | - error_propagate(errp, local_err); | ||
156 | + if (ret < 0) { | ||
157 | bdrv_next_cleanup(&it); | ||
158 | return; | ||
159 | } | ||
160 | -- | 82 | -- |
161 | 2.26.2 | 83 | 2.40.1 |
162 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | When processing vectored guest requests that are not aligned to the |
---|---|---|---|
2 | 2 | storage request alignment, we pad them by adding head and/or tail | |
3 | Most of our coroutine wrappers already follow this convention: | 3 | buffers for a read-modify-write cycle. |
4 | 4 | ||
5 | We have 'coroutine_fn bdrv_co_<something>(<normal argument list>)' as | 5 | The guest can submit I/O vectors up to IOV_MAX (1024) in length, but |
6 | the core function, and a wrapper 'bdrv_<something>(<same argument | 6 | with this padding, the vector can exceed that limit. As of |
7 | list>)' which does parameter packing and calls bdrv_run_co(). | 7 | 4c002cef0e9abe7135d7916c51abce47f7fc1ee2 ("util/iov: make |
8 | 8 | qemu_iovec_init_extended() honest"), we refuse to pad vectors beyond the | |
9 | The only outsiders are the bdrv_prwv_co and | 9 | limit, instead returning an error to the guest. |
10 | bdrv_common_block_status_above wrappers. Let's refactor them to behave | 10 | |
11 | as the others, it simplifies further conversion of coroutine wrappers. | 11 | To the guest, this appears as a random I/O error. We should not return |
12 | 12 | an I/O error to the guest when it issued a perfectly valid request. | |
13 | This patch adds an indirection layer, but it will be compensated by | 13 | |
14 | a further commit, which will drop bdrv_co_prwv together with the | 14 | Before 4c002cef0e9abe7135d7916c51abce47f7fc1ee2, we just made the vector |
15 | is_write logic, to keep the read and write paths separate. | 15 | longer than IOV_MAX, which generally seems to work (because the guest |
16 | 16 | assumes a smaller alignment than we really have, file-posix's | |
17 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 17 | raw_co_prw() will generally see bdrv_qiov_is_aligned() return false, and |
18 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 18 | so emulate the request, so that the IOV_MAX does not matter). However, |
19 | Reviewed-by: Eric Blake <eblake@redhat.com> | 19 | that does not seem exactly great. |
20 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 20 | |
21 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | 21 | I see two ways to fix this problem: |
22 | Message-Id: <20200924185414.28642-3-vsementsov@virtuozzo.com> | 22 | 1. We split such long requests into two requests. |
23 | 2. We join some elements of the vector into new buffers to make it | ||
24 | shorter. | ||
25 | |||
26 | I am wary of (1), because it seems like it may have unintended side | ||
27 | effects. | ||
28 | |||
29 | (2) on the other hand seems relatively simple to implement, with | ||
30 | hopefully few side effects, so this patch does that. | ||
31 | |||
32 | To do this, the use of qemu_iovec_init_extended() in bdrv_pad_request() | ||
33 | is effectively replaced by the new function bdrv_create_padded_qiov(), | ||
34 | which not only wraps the request IOV with padding head/tail, but also | ||
35 | ensures that the resulting vector will not have more than IOV_MAX | ||
36 | elements. Putting that functionality into qemu_iovec_init_extended() is | ||
37 | infeasible because it requires allocating a bounce buffer; doing so | ||
38 | would require many more parameters (buffer alignment, how to initialize | ||
39 | the buffer, and out parameters like the buffer, its length, and the | ||
40 | original elements), which is not reasonable. | ||
41 | |||
42 | Conversely, it is not difficult to move qemu_iovec_init_extended()'s | ||
43 | functionality into bdrv_create_padded_qiov() by using public | ||
44 | qemu_iovec_* functions, so that is what this patch does. | ||
45 | |||
46 | Because bdrv_pad_request() was the only "serious" user of | ||
47 | qemu_iovec_init_extended(), the next patch will remove the latter | ||
48 | function, so the functionality is not implemented twice. | ||
49 | |||
50 | Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2141964 | ||
51 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> | ||
52 | Message-Id: <20230411173418.19549-3-hreitz@redhat.com> | ||
53 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> | ||
23 | --- | 54 | --- |
24 | block/io.c | 60 +++++++++++++++++++++++++++++------------------------- | 55 | block/io.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++++----- |
25 | 1 file changed, 32 insertions(+), 28 deletions(-) | 56 | 1 file changed, 151 insertions(+), 15 deletions(-) |
26 | 57 | ||
27 | diff --git a/block/io.c b/block/io.c | 58 | diff --git a/block/io.c b/block/io.c |
28 | index XXXXXXX..XXXXXXX 100644 | 59 | index XXXXXXX..XXXXXXX 100644 |
29 | --- a/block/io.c | 60 | --- a/block/io.c |
30 | +++ b/block/io.c | 61 | +++ b/block/io.c |
31 | @@ -XXX,XX +XXX,XX @@ typedef struct RwCo { | 62 | @@ -XXX,XX +XXX,XX @@ out: |
32 | BdrvRequestFlags flags; | 63 | * @merge_reads is true for small requests, |
33 | } RwCo; | 64 | * if @buf_len == @head + bytes + @tail. In this case it is possible that both |
34 | 65 | * head and tail exist but @buf_len == align and @tail_buf == @buf. | |
35 | +static int coroutine_fn bdrv_co_prwv(BdrvChild *child, int64_t offset, | 66 | + * |
36 | + QEMUIOVector *qiov, bool is_write, | 67 | + * @write is true for write requests, false for read requests. |
37 | + BdrvRequestFlags flags) | 68 | + * |
69 | + * If padding makes the vector too long (exceeding IOV_MAX), then we need to | ||
70 | + * merge existing vector elements into a single one. @collapse_bounce_buf acts | ||
71 | + * as the bounce buffer in such cases. @pre_collapse_qiov has the pre-collapse | ||
72 | + * I/O vector elements so for read requests, the data can be copied back after | ||
73 | + * the read is done. | ||
74 | */ | ||
75 | typedef struct BdrvRequestPadding { | ||
76 | uint8_t *buf; | ||
77 | @@ -XXX,XX +XXX,XX @@ typedef struct BdrvRequestPadding { | ||
78 | size_t head; | ||
79 | size_t tail; | ||
80 | bool merge_reads; | ||
81 | + bool write; | ||
82 | QEMUIOVector local_qiov; | ||
83 | + | ||
84 | + uint8_t *collapse_bounce_buf; | ||
85 | + size_t collapse_len; | ||
86 | + QEMUIOVector pre_collapse_qiov; | ||
87 | } BdrvRequestPadding; | ||
88 | |||
89 | static bool bdrv_init_padding(BlockDriverState *bs, | ||
90 | int64_t offset, int64_t bytes, | ||
91 | + bool write, | ||
92 | BdrvRequestPadding *pad) | ||
93 | { | ||
94 | int64_t align = bs->bl.request_alignment; | ||
95 | @@ -XXX,XX +XXX,XX @@ static bool bdrv_init_padding(BlockDriverState *bs, | ||
96 | pad->tail_buf = pad->buf + pad->buf_len - align; | ||
97 | } | ||
98 | |||
99 | + pad->write = write; | ||
100 | + | ||
101 | return true; | ||
102 | } | ||
103 | |||
104 | @@ -XXX,XX +XXX,XX @@ zero_mem: | ||
105 | return 0; | ||
106 | } | ||
107 | |||
108 | -static void bdrv_padding_destroy(BdrvRequestPadding *pad) | ||
109 | +/** | ||
110 | + * Free *pad's associated buffers, and perform any necessary finalization steps. | ||
111 | + */ | ||
112 | +static void bdrv_padding_finalize(BdrvRequestPadding *pad) | ||
113 | { | ||
114 | + if (pad->collapse_bounce_buf) { | ||
115 | + if (!pad->write) { | ||
116 | + /* | ||
117 | + * If padding required elements in the vector to be collapsed into a | ||
118 | + * bounce buffer, copy the bounce buffer content back | ||
119 | + */ | ||
120 | + qemu_iovec_from_buf(&pad->pre_collapse_qiov, 0, | ||
121 | + pad->collapse_bounce_buf, pad->collapse_len); | ||
122 | + } | ||
123 | + qemu_vfree(pad->collapse_bounce_buf); | ||
124 | + qemu_iovec_destroy(&pad->pre_collapse_qiov); | ||
125 | + } | ||
126 | if (pad->buf) { | ||
127 | qemu_vfree(pad->buf); | ||
128 | qemu_iovec_destroy(&pad->local_qiov); | ||
129 | @@ -XXX,XX +XXX,XX @@ static void bdrv_padding_destroy(BdrvRequestPadding *pad) | ||
130 | memset(pad, 0, sizeof(*pad)); | ||
131 | } | ||
132 | |||
133 | +/* | ||
134 | + * Create pad->local_qiov by wrapping @iov in the padding head and tail, while | ||
135 | + * ensuring that the resulting vector will not exceed IOV_MAX elements. | ||
136 | + * | ||
137 | + * To ensure this, when necessary, the first two or three elements of @iov are | ||
138 | + * merged into pad->collapse_bounce_buf and replaced by a reference to that | ||
139 | + * bounce buffer in pad->local_qiov. | ||
140 | + * | ||
141 | + * After performing a read request, the data from the bounce buffer must be | ||
142 | + * copied back into pad->pre_collapse_qiov (e.g. by bdrv_padding_finalize()). | ||
143 | + */ | ||
144 | +static int bdrv_create_padded_qiov(BlockDriverState *bs, | ||
145 | + BdrvRequestPadding *pad, | ||
146 | + struct iovec *iov, int niov, | ||
147 | + size_t iov_offset, size_t bytes) | ||
38 | +{ | 148 | +{ |
39 | + if (is_write) { | 149 | + int padded_niov, surplus_count, collapse_count; |
40 | + return bdrv_co_pwritev(child, offset, qiov->size, qiov, flags); | 150 | + |
41 | + } else { | 151 | + /* Assert this invariant */ |
42 | + return bdrv_co_preadv(child, offset, qiov->size, qiov, flags); | 152 | + assert(niov <= IOV_MAX); |
43 | + } | 153 | + |
154 | + /* | ||
155 | + * Cannot pad if resulting length would exceed SIZE_MAX. Returning an error | ||
156 | + * to the guest is not ideal, but there is little else we can do. At least | ||
157 | + * this will practically never happen on 64-bit systems. | ||
158 | + */ | ||
159 | + if (SIZE_MAX - pad->head < bytes || | ||
160 | + SIZE_MAX - pad->head - bytes < pad->tail) | ||
161 | + { | ||
162 | + return -EINVAL; | ||
163 | + } | ||
164 | + | ||
165 | + /* Length of the resulting IOV if we just concatenated everything */ | ||
166 | + padded_niov = !!pad->head + niov + !!pad->tail; | ||
167 | + | ||
168 | + qemu_iovec_init(&pad->local_qiov, MIN(padded_niov, IOV_MAX)); | ||
169 | + | ||
170 | + if (pad->head) { | ||
171 | + qemu_iovec_add(&pad->local_qiov, pad->buf, pad->head); | ||
172 | + } | ||
173 | + | ||
174 | + /* | ||
175 | + * If padded_niov > IOV_MAX, we cannot just concatenate everything. | ||
176 | + * Instead, merge the first two or three elements of @iov to reduce the | ||
177 | + * number of vector elements as necessary. | ||
178 | + */ | ||
179 | + if (padded_niov > IOV_MAX) { | ||
180 | + /* | ||
181 | + * Only head and tail can have lead to the number of entries exceeding | ||
182 | + * IOV_MAX, so we can exceed it by the head and tail at most. We need | ||
183 | + * to reduce the number of elements by `surplus_count`, so we merge that | ||
184 | + * many elements plus one into one element. | ||
185 | + */ | ||
186 | + surplus_count = padded_niov - IOV_MAX; | ||
187 | + assert(surplus_count <= !!pad->head + !!pad->tail); | ||
188 | + collapse_count = surplus_count + 1; | ||
189 | + | ||
190 | + /* | ||
191 | + * Move the elements to collapse into `pad->pre_collapse_qiov`, then | ||
192 | + * advance `iov` (and associated variables) by those elements. | ||
193 | + */ | ||
194 | + qemu_iovec_init(&pad->pre_collapse_qiov, collapse_count); | ||
195 | + qemu_iovec_concat_iov(&pad->pre_collapse_qiov, iov, | ||
196 | + collapse_count, iov_offset, SIZE_MAX); | ||
197 | + iov += collapse_count; | ||
198 | + iov_offset = 0; | ||
199 | + niov -= collapse_count; | ||
200 | + bytes -= pad->pre_collapse_qiov.size; | ||
201 | + | ||
202 | + /* | ||
203 | + * Construct the bounce buffer to match the length of the to-collapse | ||
204 | + * vector elements, and for write requests, initialize it with the data | ||
205 | + * from those elements. Then add it to `pad->local_qiov`. | ||
206 | + */ | ||
207 | + pad->collapse_len = pad->pre_collapse_qiov.size; | ||
208 | + pad->collapse_bounce_buf = qemu_blockalign(bs, pad->collapse_len); | ||
209 | + if (pad->write) { | ||
210 | + qemu_iovec_to_buf(&pad->pre_collapse_qiov, 0, | ||
211 | + pad->collapse_bounce_buf, pad->collapse_len); | ||
212 | + } | ||
213 | + qemu_iovec_add(&pad->local_qiov, | ||
214 | + pad->collapse_bounce_buf, pad->collapse_len); | ||
215 | + } | ||
216 | + | ||
217 | + qemu_iovec_concat_iov(&pad->local_qiov, iov, niov, iov_offset, bytes); | ||
218 | + | ||
219 | + if (pad->tail) { | ||
220 | + qemu_iovec_add(&pad->local_qiov, | ||
221 | + pad->buf + pad->buf_len - pad->tail, pad->tail); | ||
222 | + } | ||
223 | + | ||
224 | + assert(pad->local_qiov.niov == MIN(padded_niov, IOV_MAX)); | ||
225 | + return 0; | ||
44 | +} | 226 | +} |
45 | + | 227 | + |
46 | static int coroutine_fn bdrv_rw_co_entry(void *opaque) | ||
47 | { | ||
48 | RwCo *rwco = opaque; | ||
49 | |||
50 | - if (!rwco->is_write) { | ||
51 | - return bdrv_co_preadv(rwco->child, rwco->offset, | ||
52 | - rwco->qiov->size, rwco->qiov, | ||
53 | - rwco->flags); | ||
54 | - } else { | ||
55 | - return bdrv_co_pwritev(rwco->child, rwco->offset, | ||
56 | - rwco->qiov->size, rwco->qiov, | ||
57 | - rwco->flags); | ||
58 | - } | ||
59 | + return bdrv_co_prwv(rwco->child, rwco->offset, rwco->qiov, | ||
60 | + rwco->is_write, rwco->flags); | ||
61 | } | ||
62 | |||
63 | /* | 228 | /* |
64 | * Process a vectored synchronous request using coroutines | 229 | * bdrv_pad_request |
65 | */ | 230 | * |
66 | -static int bdrv_prwv_co(BdrvChild *child, int64_t offset, | 231 | @@ -XXX,XX +XXX,XX @@ static void bdrv_padding_destroy(BdrvRequestPadding *pad) |
67 | - QEMUIOVector *qiov, bool is_write, | 232 | * read of padding, bdrv_padding_rmw_read() should be called separately if |
68 | - BdrvRequestFlags flags) | 233 | * needed. |
69 | +static int bdrv_prwv(BdrvChild *child, int64_t offset, | 234 | * |
70 | + QEMUIOVector *qiov, bool is_write, | 235 | + * @write is true for write requests, false for read requests. |
71 | + BdrvRequestFlags flags) | 236 | + * |
72 | { | 237 | * Request parameters (@qiov, &qiov_offset, &offset, &bytes) are in-out: |
73 | RwCo rwco = { | 238 | * - on function start they represent original request |
74 | .child = child, | 239 | * - on failure or when padding is not needed they are unchanged |
75 | @@ -XXX,XX +XXX,XX @@ int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset, | 240 | @@ -XXX,XX +XXX,XX @@ static void bdrv_padding_destroy(BdrvRequestPadding *pad) |
76 | { | 241 | static int bdrv_pad_request(BlockDriverState *bs, |
77 | QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, bytes); | 242 | QEMUIOVector **qiov, size_t *qiov_offset, |
78 | 243 | int64_t *offset, int64_t *bytes, | |
79 | - return bdrv_prwv_co(child, offset, &qiov, true, | 244 | + bool write, |
80 | - BDRV_REQ_ZERO_WRITE | flags); | 245 | BdrvRequestPadding *pad, bool *padded, |
81 | + return bdrv_prwv(child, offset, &qiov, true, BDRV_REQ_ZERO_WRITE | flags); | 246 | BdrvRequestFlags *flags) |
82 | } | ||
83 | |||
84 | /* | ||
85 | @@ -XXX,XX +XXX,XX @@ int bdrv_preadv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov) | ||
86 | { | 247 | { |
87 | int ret; | 248 | int ret; |
88 | 249 | + struct iovec *sliced_iov; | |
89 | - ret = bdrv_prwv_co(child, offset, qiov, false, 0); | 250 | + int sliced_niov; |
90 | + ret = bdrv_prwv(child, offset, qiov, false, 0); | 251 | + size_t sliced_head, sliced_tail; |
252 | |||
253 | bdrv_check_qiov_request(*offset, *bytes, *qiov, *qiov_offset, &error_abort); | ||
254 | |||
255 | - if (!bdrv_init_padding(bs, *offset, *bytes, pad)) { | ||
256 | + if (!bdrv_init_padding(bs, *offset, *bytes, write, pad)) { | ||
257 | if (padded) { | ||
258 | *padded = false; | ||
259 | } | ||
260 | return 0; | ||
261 | } | ||
262 | |||
263 | - ret = qemu_iovec_init_extended(&pad->local_qiov, pad->buf, pad->head, | ||
264 | - *qiov, *qiov_offset, *bytes, | ||
265 | - pad->buf + pad->buf_len - pad->tail, | ||
266 | - pad->tail); | ||
267 | + sliced_iov = qemu_iovec_slice(*qiov, *qiov_offset, *bytes, | ||
268 | + &sliced_head, &sliced_tail, | ||
269 | + &sliced_niov); | ||
270 | + | ||
271 | + /* Guaranteed by bdrv_check_qiov_request() */ | ||
272 | + assert(*bytes <= SIZE_MAX); | ||
273 | + ret = bdrv_create_padded_qiov(bs, pad, sliced_iov, sliced_niov, | ||
274 | + sliced_head, *bytes); | ||
91 | if (ret < 0) { | 275 | if (ret < 0) { |
276 | - bdrv_padding_destroy(pad); | ||
277 | + bdrv_padding_finalize(pad); | ||
92 | return ret; | 278 | return ret; |
93 | } | 279 | } |
94 | @@ -XXX,XX +XXX,XX @@ int bdrv_pwritev(BdrvChild *child, int64_t offset, QEMUIOVector *qiov) | 280 | *bytes += pad->head + pad->tail; |
95 | { | 281 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_preadv_part(BdrvChild *child, |
96 | int ret; | 282 | flags |= BDRV_REQ_COPY_ON_READ; |
97 | 283 | } | |
98 | - ret = bdrv_prwv_co(child, offset, qiov, true, 0); | 284 | |
99 | + ret = bdrv_prwv(child, offset, qiov, true, 0); | 285 | - ret = bdrv_pad_request(bs, &qiov, &qiov_offset, &offset, &bytes, &pad, |
286 | - NULL, &flags); | ||
287 | + ret = bdrv_pad_request(bs, &qiov, &qiov_offset, &offset, &bytes, false, | ||
288 | + &pad, NULL, &flags); | ||
100 | if (ret < 0) { | 289 | if (ret < 0) { |
101 | return ret; | 290 | goto fail; |
102 | } | 291 | } |
103 | @@ -XXX,XX +XXX,XX @@ early_out: | 292 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_preadv_part(BdrvChild *child, |
293 | bs->bl.request_alignment, | ||
294 | qiov, qiov_offset, flags); | ||
295 | tracked_request_end(&req); | ||
296 | - bdrv_padding_destroy(&pad); | ||
297 | + bdrv_padding_finalize(&pad); | ||
298 | |||
299 | fail: | ||
300 | bdrv_dec_in_flight(bs); | ||
301 | @@ -XXX,XX +XXX,XX @@ bdrv_co_do_zero_pwritev(BdrvChild *child, int64_t offset, int64_t bytes, | ||
302 | /* This flag doesn't make sense for padding or zero writes */ | ||
303 | flags &= ~BDRV_REQ_REGISTERED_BUF; | ||
304 | |||
305 | - padding = bdrv_init_padding(bs, offset, bytes, &pad); | ||
306 | + padding = bdrv_init_padding(bs, offset, bytes, true, &pad); | ||
307 | if (padding) { | ||
308 | assert(!(flags & BDRV_REQ_NO_WAIT)); | ||
309 | bdrv_make_request_serialising(req, align); | ||
310 | @@ -XXX,XX +XXX,XX @@ bdrv_co_do_zero_pwritev(BdrvChild *child, int64_t offset, int64_t bytes, | ||
311 | } | ||
312 | |||
313 | out: | ||
314 | - bdrv_padding_destroy(&pad); | ||
315 | + bdrv_padding_finalize(&pad); | ||
316 | |||
104 | return ret; | 317 | return ret; |
105 | } | 318 | } |
106 | 319 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child, | |
107 | -static int coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs, | 320 | * bdrv_co_do_zero_pwritev() does aligning by itself, so, we do |
108 | - BlockDriverState *base, | 321 | * alignment only if there is no ZERO flag. |
109 | - bool want_zero, | 322 | */ |
110 | - int64_t offset, | 323 | - ret = bdrv_pad_request(bs, &qiov, &qiov_offset, &offset, &bytes, &pad, |
111 | - int64_t bytes, | 324 | - &padded, &flags); |
112 | - int64_t *pnum, | 325 | + ret = bdrv_pad_request(bs, &qiov, &qiov_offset, &offset, &bytes, true, |
113 | - int64_t *map, | 326 | + &pad, &padded, &flags); |
114 | - BlockDriverState **file) | 327 | if (ret < 0) { |
115 | +static int coroutine_fn | 328 | return ret; |
116 | +bdrv_co_common_block_status_above(BlockDriverState *bs, | 329 | } |
117 | + BlockDriverState *base, | 330 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child, |
118 | + bool want_zero, | 331 | ret = bdrv_aligned_pwritev(child, &req, offset, bytes, align, |
119 | + int64_t offset, | 332 | qiov, qiov_offset, flags); |
120 | + int64_t bytes, | 333 | |
121 | + int64_t *pnum, | 334 | - bdrv_padding_destroy(&pad); |
122 | + int64_t *map, | 335 | + bdrv_padding_finalize(&pad); |
123 | + BlockDriverState **file) | 336 | |
124 | { | 337 | out: |
125 | BlockDriverState *p; | 338 | tracked_request_end(&req); |
126 | int ret = 0; | ||
127 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_block_status_above_co_entry(void *opaque) | ||
128 | { | ||
129 | BdrvCoBlockStatusData *data = opaque; | ||
130 | |||
131 | - return bdrv_co_block_status_above(data->bs, data->base, | ||
132 | - data->want_zero, | ||
133 | - data->offset, data->bytes, | ||
134 | - data->pnum, data->map, data->file); | ||
135 | + return bdrv_co_common_block_status_above(data->bs, data->base, | ||
136 | + data->want_zero, | ||
137 | + data->offset, data->bytes, | ||
138 | + data->pnum, data->map, data->file); | ||
139 | } | ||
140 | |||
141 | /* | ||
142 | -- | 339 | -- |
143 | 2.26.2 | 340 | 2.40.1 |
144 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | bdrv_pad_request() was the main user of qemu_iovec_init_extended(). |
---|---|---|---|
2 | HEAD^ has removed that use, so we can remove qemu_iovec_init_extended() | ||
3 | now. | ||
2 | 4 | ||
3 | Use code generation implemented in previous commit to generated | 5 | The only remaining user is qemu_iovec_init_slice(), which can easily |
4 | coroutine wrappers in block.c and block/io.c | 6 | inline the small part it really needs. |
5 | 7 | ||
6 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 8 | Note that qemu_iovec_init_extended() offered a memcpy() optimization to |
7 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 9 | initialize the new I/O vector. qemu_iovec_concat_iov(), which is used |
10 | to replace its functionality, does not, but calls qemu_iovec_add() for | ||
11 | every single element. If we decide this optimization was important, we | ||
12 | will need to re-implement it in qemu_iovec_concat_iov(), which might | ||
13 | also benefit its pre-existing users. | ||
14 | |||
8 | Reviewed-by: Eric Blake <eblake@redhat.com> | 15 | Reviewed-by: Eric Blake <eblake@redhat.com> |
9 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | 16 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> |
10 | Message-Id: <20200924185414.28642-6-vsementsov@virtuozzo.com> | 17 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> |
18 | Message-Id: <20230411173418.19549-4-hreitz@redhat.com> | ||
11 | --- | 19 | --- |
12 | block/coroutines.h | 6 +- | 20 | include/qemu/iov.h | 5 --- |
13 | include/block/block.h | 16 ++-- | 21 | util/iov.c | 79 +++++++--------------------------------------- |
14 | block.c | 73 --------------- | 22 | 2 files changed, 11 insertions(+), 73 deletions(-) |
15 | block/io.c | 212 ------------------------------------------ | ||
16 | 4 files changed, 13 insertions(+), 294 deletions(-) | ||
17 | 23 | ||
18 | diff --git a/block/coroutines.h b/block/coroutines.h | 24 | diff --git a/include/qemu/iov.h b/include/qemu/iov.h |
19 | index XXXXXXX..XXXXXXX 100644 | 25 | index XXXXXXX..XXXXXXX 100644 |
20 | --- a/block/coroutines.h | 26 | --- a/include/qemu/iov.h |
21 | +++ b/block/coroutines.h | 27 | +++ b/include/qemu/iov.h |
22 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp); | 28 | @@ -XXX,XX +XXX,XX @@ static inline void *qemu_iovec_buf(QEMUIOVector *qiov) |
23 | int coroutine_fn | 29 | |
24 | bdrv_co_prwv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov, | 30 | void qemu_iovec_init(QEMUIOVector *qiov, int alloc_hint); |
25 | bool is_write, BdrvRequestFlags flags); | 31 | void qemu_iovec_init_external(QEMUIOVector *qiov, struct iovec *iov, int niov); |
26 | -int | 32 | -int qemu_iovec_init_extended( |
27 | +int generated_co_wrapper | 33 | - QEMUIOVector *qiov, |
28 | bdrv_prwv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov, | 34 | - void *head_buf, size_t head_len, |
29 | bool is_write, BdrvRequestFlags flags); | 35 | - QEMUIOVector *mid_qiov, size_t mid_offset, size_t mid_len, |
30 | 36 | - void *tail_buf, size_t tail_len); | |
31 | @@ -XXX,XX +XXX,XX @@ bdrv_co_common_block_status_above(BlockDriverState *bs, | 37 | void qemu_iovec_init_slice(QEMUIOVector *qiov, QEMUIOVector *source, |
32 | int64_t *pnum, | 38 | size_t offset, size_t len); |
33 | int64_t *map, | 39 | struct iovec *qemu_iovec_slice(QEMUIOVector *qiov, |
34 | BlockDriverState **file); | 40 | diff --git a/util/iov.c b/util/iov.c |
35 | -int | ||
36 | +int generated_co_wrapper | ||
37 | bdrv_common_block_status_above(BlockDriverState *bs, | ||
38 | BlockDriverState *base, | ||
39 | bool want_zero, | ||
40 | @@ -XXX,XX +XXX,XX @@ bdrv_common_block_status_above(BlockDriverState *bs, | ||
41 | int coroutine_fn | ||
42 | bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, | ||
43 | bool is_read); | ||
44 | -int | ||
45 | +int generated_co_wrapper | ||
46 | bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, | ||
47 | bool is_read); | ||
48 | |||
49 | diff --git a/include/block/block.h b/include/block/block.h | ||
50 | index XXXXXXX..XXXXXXX 100644 | 41 | index XXXXXXX..XXXXXXX 100644 |
51 | --- a/include/block/block.h | 42 | --- a/util/iov.c |
52 | +++ b/include/block/block.h | 43 | +++ b/util/iov.c |
53 | @@ -XXX,XX +XXX,XX @@ void bdrv_refresh_filename(BlockDriverState *bs); | 44 | @@ -XXX,XX +XXX,XX @@ int qemu_iovec_subvec_niov(QEMUIOVector *qiov, size_t offset, size_t len) |
54 | int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact, | 45 | return niov; |
55 | PreallocMode prealloc, BdrvRequestFlags flags, | ||
56 | Error **errp); | ||
57 | -int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact, | ||
58 | - PreallocMode prealloc, BdrvRequestFlags flags, Error **errp); | ||
59 | +int generated_co_wrapper | ||
60 | +bdrv_truncate(BdrvChild *child, int64_t offset, bool exact, | ||
61 | + PreallocMode prealloc, BdrvRequestFlags flags, Error **errp); | ||
62 | |||
63 | int64_t bdrv_nb_sectors(BlockDriverState *bs); | ||
64 | int64_t bdrv_getlength(BlockDriverState *bs); | ||
65 | @@ -XXX,XX +XXX,XX @@ typedef enum { | ||
66 | BDRV_FIX_ERRORS = 2, | ||
67 | } BdrvCheckMode; | ||
68 | |||
69 | -int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix); | ||
70 | +int generated_co_wrapper bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, | ||
71 | + BdrvCheckMode fix); | ||
72 | |||
73 | /* The units of offset and total_work_size may be chosen arbitrarily by the | ||
74 | * block driver; total_work_size may change during the course of the amendment | ||
75 | @@ -XXX,XX +XXX,XX @@ void bdrv_aio_cancel_async(BlockAIOCB *acb); | ||
76 | int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf); | ||
77 | |||
78 | /* Invalidate any cached metadata used by image formats */ | ||
79 | -int bdrv_invalidate_cache(BlockDriverState *bs, Error **errp); | ||
80 | +int generated_co_wrapper bdrv_invalidate_cache(BlockDriverState *bs, | ||
81 | + Error **errp); | ||
82 | void bdrv_invalidate_cache_all(Error **errp); | ||
83 | int bdrv_inactivate_all(void); | ||
84 | |||
85 | /* Ensure contents are flushed to disk. */ | ||
86 | -int bdrv_flush(BlockDriverState *bs); | ||
87 | +int generated_co_wrapper bdrv_flush(BlockDriverState *bs); | ||
88 | int coroutine_fn bdrv_co_flush(BlockDriverState *bs); | ||
89 | int bdrv_flush_all(void); | ||
90 | void bdrv_close_all(void); | ||
91 | @@ -XXX,XX +XXX,XX @@ void bdrv_drain_all(void); | ||
92 | AIO_WAIT_WHILE(bdrv_get_aio_context(bs_), \ | ||
93 | cond); }) | ||
94 | |||
95 | -int bdrv_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes); | ||
96 | +int generated_co_wrapper bdrv_pdiscard(BdrvChild *child, int64_t offset, | ||
97 | + int64_t bytes); | ||
98 | int bdrv_co_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes); | ||
99 | int bdrv_has_zero_init_1(BlockDriverState *bs); | ||
100 | int bdrv_has_zero_init(BlockDriverState *bs); | ||
101 | diff --git a/block.c b/block.c | ||
102 | index XXXXXXX..XXXXXXX 100644 | ||
103 | --- a/block.c | ||
104 | +++ b/block.c | ||
105 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_check(BlockDriverState *bs, | ||
106 | return bs->drv->bdrv_co_check(bs, res, fix); | ||
107 | } | 46 | } |
108 | 47 | ||
109 | -typedef struct CheckCo { | 48 | -/* |
110 | - BlockDriverState *bs; | 49 | - * Compile new iovec, combining @head_buf buffer, sub-qiov of @mid_qiov, |
111 | - BdrvCheckResult *res; | 50 | - * and @tail_buf buffer into new qiov. |
112 | - BdrvCheckMode fix; | 51 | - */ |
113 | - int ret; | 52 | -int qemu_iovec_init_extended( |
114 | -} CheckCo; | 53 | - QEMUIOVector *qiov, |
54 | - void *head_buf, size_t head_len, | ||
55 | - QEMUIOVector *mid_qiov, size_t mid_offset, size_t mid_len, | ||
56 | - void *tail_buf, size_t tail_len) | ||
57 | -{ | ||
58 | - size_t mid_head, mid_tail; | ||
59 | - int total_niov, mid_niov = 0; | ||
60 | - struct iovec *p, *mid_iov = NULL; | ||
115 | - | 61 | - |
116 | -static void coroutine_fn bdrv_check_co_entry(void *opaque) | 62 | - assert(mid_qiov->niov <= IOV_MAX); |
117 | -{ | ||
118 | - CheckCo *cco = opaque; | ||
119 | - cco->ret = bdrv_co_check(cco->bs, cco->res, cco->fix); | ||
120 | - aio_wait_kick(); | ||
121 | -} | ||
122 | - | 63 | - |
123 | -int bdrv_check(BlockDriverState *bs, | 64 | - if (SIZE_MAX - head_len < mid_len || |
124 | - BdrvCheckResult *res, BdrvCheckMode fix) | 65 | - SIZE_MAX - head_len - mid_len < tail_len) |
125 | -{ | 66 | - { |
126 | - Coroutine *co; | 67 | - return -EINVAL; |
127 | - CheckCo cco = { | ||
128 | - .bs = bs, | ||
129 | - .res = res, | ||
130 | - .ret = -EINPROGRESS, | ||
131 | - .fix = fix, | ||
132 | - }; | ||
133 | - | ||
134 | - if (qemu_in_coroutine()) { | ||
135 | - /* Fast-path if already in coroutine context */ | ||
136 | - bdrv_check_co_entry(&cco); | ||
137 | - } else { | ||
138 | - co = qemu_coroutine_create(bdrv_check_co_entry, &cco); | ||
139 | - bdrv_coroutine_enter(bs, co); | ||
140 | - BDRV_POLL_WHILE(bs, cco.ret == -EINPROGRESS); | ||
141 | - } | 68 | - } |
142 | - | 69 | - |
143 | - return cco.ret; | 70 | - if (mid_len) { |
71 | - mid_iov = qemu_iovec_slice(mid_qiov, mid_offset, mid_len, | ||
72 | - &mid_head, &mid_tail, &mid_niov); | ||
73 | - } | ||
74 | - | ||
75 | - total_niov = !!head_len + mid_niov + !!tail_len; | ||
76 | - if (total_niov > IOV_MAX) { | ||
77 | - return -EINVAL; | ||
78 | - } | ||
79 | - | ||
80 | - if (total_niov == 1) { | ||
81 | - qemu_iovec_init_buf(qiov, NULL, 0); | ||
82 | - p = &qiov->local_iov; | ||
83 | - } else { | ||
84 | - qiov->niov = qiov->nalloc = total_niov; | ||
85 | - qiov->size = head_len + mid_len + tail_len; | ||
86 | - p = qiov->iov = g_new(struct iovec, qiov->niov); | ||
87 | - } | ||
88 | - | ||
89 | - if (head_len) { | ||
90 | - p->iov_base = head_buf; | ||
91 | - p->iov_len = head_len; | ||
92 | - p++; | ||
93 | - } | ||
94 | - | ||
95 | - assert(!mid_niov == !mid_len); | ||
96 | - if (mid_niov) { | ||
97 | - memcpy(p, mid_iov, mid_niov * sizeof(*p)); | ||
98 | - p[0].iov_base = (uint8_t *)p[0].iov_base + mid_head; | ||
99 | - p[0].iov_len -= mid_head; | ||
100 | - p[mid_niov - 1].iov_len -= mid_tail; | ||
101 | - p += mid_niov; | ||
102 | - } | ||
103 | - | ||
104 | - if (tail_len) { | ||
105 | - p->iov_base = tail_buf; | ||
106 | - p->iov_len = tail_len; | ||
107 | - } | ||
108 | - | ||
109 | - return 0; | ||
144 | -} | 110 | -} |
145 | - | 111 | - |
146 | /* | 112 | /* |
147 | * Return values: | 113 | * Check if the contents of subrange of qiov data is all zeroes. |
148 | * 0 - success | 114 | */ |
149 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp) | 115 | @@ -XXX,XX +XXX,XX @@ bool qemu_iovec_is_zero(QEMUIOVector *qiov, size_t offset, size_t bytes) |
150 | return 0; | 116 | void qemu_iovec_init_slice(QEMUIOVector *qiov, QEMUIOVector *source, |
117 | size_t offset, size_t len) | ||
118 | { | ||
119 | - int ret; | ||
120 | + struct iovec *slice_iov; | ||
121 | + int slice_niov; | ||
122 | + size_t slice_head, slice_tail; | ||
123 | |||
124 | assert(source->size >= len); | ||
125 | assert(source->size - len >= offset); | ||
126 | |||
127 | - /* We shrink the request, so we can't overflow neither size_t nor MAX_IOV */ | ||
128 | - ret = qemu_iovec_init_extended(qiov, NULL, 0, source, offset, len, NULL, 0); | ||
129 | - assert(ret == 0); | ||
130 | + slice_iov = qemu_iovec_slice(source, offset, len, | ||
131 | + &slice_head, &slice_tail, &slice_niov); | ||
132 | + if (slice_niov == 1) { | ||
133 | + qemu_iovec_init_buf(qiov, slice_iov[0].iov_base + slice_head, len); | ||
134 | + } else { | ||
135 | + qemu_iovec_init(qiov, slice_niov); | ||
136 | + qemu_iovec_concat_iov(qiov, slice_iov, slice_niov, slice_head, len); | ||
137 | + } | ||
151 | } | 138 | } |
152 | 139 | ||
153 | -typedef struct InvalidateCacheCo { | 140 | void qemu_iovec_destroy(QEMUIOVector *qiov) |
154 | - BlockDriverState *bs; | ||
155 | - Error **errp; | ||
156 | - bool done; | ||
157 | - int ret; | ||
158 | -} InvalidateCacheCo; | ||
159 | - | ||
160 | -static void coroutine_fn bdrv_invalidate_cache_co_entry(void *opaque) | ||
161 | -{ | ||
162 | - InvalidateCacheCo *ico = opaque; | ||
163 | - ico->ret = bdrv_co_invalidate_cache(ico->bs, ico->errp); | ||
164 | - ico->done = true; | ||
165 | - aio_wait_kick(); | ||
166 | -} | ||
167 | - | ||
168 | -int bdrv_invalidate_cache(BlockDriverState *bs, Error **errp) | ||
169 | -{ | ||
170 | - Coroutine *co; | ||
171 | - InvalidateCacheCo ico = { | ||
172 | - .bs = bs, | ||
173 | - .done = false, | ||
174 | - .errp = errp | ||
175 | - }; | ||
176 | - | ||
177 | - if (qemu_in_coroutine()) { | ||
178 | - /* Fast-path if already in coroutine context */ | ||
179 | - bdrv_invalidate_cache_co_entry(&ico); | ||
180 | - } else { | ||
181 | - co = qemu_coroutine_create(bdrv_invalidate_cache_co_entry, &ico); | ||
182 | - bdrv_coroutine_enter(bs, co); | ||
183 | - BDRV_POLL_WHILE(bs, !ico.done); | ||
184 | - } | ||
185 | - | ||
186 | - return ico.ret; | ||
187 | -} | ||
188 | - | ||
189 | void bdrv_invalidate_cache_all(Error **errp) | ||
190 | { | ||
191 | BlockDriverState *bs; | ||
192 | diff --git a/block/io.c b/block/io.c | ||
193 | index XXXXXXX..XXXXXXX 100644 | ||
194 | --- a/block/io.c | ||
195 | +++ b/block/io.c | ||
196 | @@ -XXX,XX +XXX,XX @@ static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset, | ||
197 | return 0; | ||
198 | } | ||
199 | |||
200 | -typedef int coroutine_fn BdrvRequestEntry(void *opaque); | ||
201 | -typedef struct BdrvRunCo { | ||
202 | - BdrvRequestEntry *entry; | ||
203 | - void *opaque; | ||
204 | - int ret; | ||
205 | - bool done; | ||
206 | - Coroutine *co; /* Coroutine, running bdrv_run_co_entry, for debugging */ | ||
207 | -} BdrvRunCo; | ||
208 | - | ||
209 | -static void coroutine_fn bdrv_run_co_entry(void *opaque) | ||
210 | -{ | ||
211 | - BdrvRunCo *arg = opaque; | ||
212 | - | ||
213 | - arg->ret = arg->entry(arg->opaque); | ||
214 | - arg->done = true; | ||
215 | - aio_wait_kick(); | ||
216 | -} | ||
217 | - | ||
218 | -static int bdrv_run_co(BlockDriverState *bs, BdrvRequestEntry *entry, | ||
219 | - void *opaque) | ||
220 | -{ | ||
221 | - if (qemu_in_coroutine()) { | ||
222 | - /* Fast-path if already in coroutine context */ | ||
223 | - return entry(opaque); | ||
224 | - } else { | ||
225 | - BdrvRunCo s = { .entry = entry, .opaque = opaque }; | ||
226 | - | ||
227 | - s.co = qemu_coroutine_create(bdrv_run_co_entry, &s); | ||
228 | - bdrv_coroutine_enter(bs, s.co); | ||
229 | - | ||
230 | - BDRV_POLL_WHILE(bs, !s.done); | ||
231 | - | ||
232 | - return s.ret; | ||
233 | - } | ||
234 | -} | ||
235 | - | ||
236 | -typedef struct RwCo { | ||
237 | - BdrvChild *child; | ||
238 | - int64_t offset; | ||
239 | - QEMUIOVector *qiov; | ||
240 | - bool is_write; | ||
241 | - BdrvRequestFlags flags; | ||
242 | -} RwCo; | ||
243 | - | ||
244 | int coroutine_fn bdrv_co_prwv(BdrvChild *child, int64_t offset, | ||
245 | QEMUIOVector *qiov, bool is_write, | ||
246 | BdrvRequestFlags flags) | ||
247 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_prwv(BdrvChild *child, int64_t offset, | ||
248 | } | ||
249 | } | ||
250 | |||
251 | -static int coroutine_fn bdrv_rw_co_entry(void *opaque) | ||
252 | -{ | ||
253 | - RwCo *rwco = opaque; | ||
254 | - | ||
255 | - return bdrv_co_prwv(rwco->child, rwco->offset, rwco->qiov, | ||
256 | - rwco->is_write, rwco->flags); | ||
257 | -} | ||
258 | - | ||
259 | -/* | ||
260 | - * Process a vectored synchronous request using coroutines | ||
261 | - */ | ||
262 | -int bdrv_prwv(BdrvChild *child, int64_t offset, | ||
263 | - QEMUIOVector *qiov, bool is_write, | ||
264 | - BdrvRequestFlags flags) | ||
265 | -{ | ||
266 | - RwCo rwco = { | ||
267 | - .child = child, | ||
268 | - .offset = offset, | ||
269 | - .qiov = qiov, | ||
270 | - .is_write = is_write, | ||
271 | - .flags = flags, | ||
272 | - }; | ||
273 | - | ||
274 | - return bdrv_run_co(child->bs, bdrv_rw_co_entry, &rwco); | ||
275 | -} | ||
276 | - | ||
277 | int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset, | ||
278 | int bytes, BdrvRequestFlags flags) | ||
279 | { | ||
280 | @@ -XXX,XX +XXX,XX @@ int bdrv_flush_all(void) | ||
281 | return result; | ||
282 | } | ||
283 | |||
284 | - | ||
285 | -typedef struct BdrvCoBlockStatusData { | ||
286 | - BlockDriverState *bs; | ||
287 | - BlockDriverState *base; | ||
288 | - bool want_zero; | ||
289 | - int64_t offset; | ||
290 | - int64_t bytes; | ||
291 | - int64_t *pnum; | ||
292 | - int64_t *map; | ||
293 | - BlockDriverState **file; | ||
294 | -} BdrvCoBlockStatusData; | ||
295 | - | ||
296 | /* | ||
297 | * Returns the allocation status of the specified sectors. | ||
298 | * Drivers not implementing the functionality are assumed to not support | ||
299 | @@ -XXX,XX +XXX,XX @@ bdrv_co_common_block_status_above(BlockDriverState *bs, | ||
300 | return ret; | ||
301 | } | ||
302 | |||
303 | -/* Coroutine wrapper for bdrv_block_status_above() */ | ||
304 | -static int coroutine_fn bdrv_block_status_above_co_entry(void *opaque) | ||
305 | -{ | ||
306 | - BdrvCoBlockStatusData *data = opaque; | ||
307 | - | ||
308 | - return bdrv_co_common_block_status_above(data->bs, data->base, | ||
309 | - data->want_zero, | ||
310 | - data->offset, data->bytes, | ||
311 | - data->pnum, data->map, data->file); | ||
312 | -} | ||
313 | - | ||
314 | -/* | ||
315 | - * Synchronous wrapper around bdrv_co_block_status_above(). | ||
316 | - * | ||
317 | - * See bdrv_co_block_status_above() for details. | ||
318 | - */ | ||
319 | -int bdrv_common_block_status_above(BlockDriverState *bs, | ||
320 | - BlockDriverState *base, | ||
321 | - bool want_zero, int64_t offset, | ||
322 | - int64_t bytes, int64_t *pnum, | ||
323 | - int64_t *map, | ||
324 | - BlockDriverState **file) | ||
325 | -{ | ||
326 | - BdrvCoBlockStatusData data = { | ||
327 | - .bs = bs, | ||
328 | - .base = base, | ||
329 | - .want_zero = want_zero, | ||
330 | - .offset = offset, | ||
331 | - .bytes = bytes, | ||
332 | - .pnum = pnum, | ||
333 | - .map = map, | ||
334 | - .file = file, | ||
335 | - }; | ||
336 | - | ||
337 | - return bdrv_run_co(bs, bdrv_block_status_above_co_entry, &data); | ||
338 | -} | ||
339 | - | ||
340 | int bdrv_block_status_above(BlockDriverState *bs, BlockDriverState *base, | ||
341 | int64_t offset, int64_t bytes, int64_t *pnum, | ||
342 | int64_t *map, BlockDriverState **file) | ||
343 | @@ -XXX,XX +XXX,XX @@ int bdrv_is_allocated_above(BlockDriverState *top, | ||
344 | return 0; | ||
345 | } | ||
346 | |||
347 | -typedef struct BdrvVmstateCo { | ||
348 | - BlockDriverState *bs; | ||
349 | - QEMUIOVector *qiov; | ||
350 | - int64_t pos; | ||
351 | - bool is_read; | ||
352 | -} BdrvVmstateCo; | ||
353 | - | ||
354 | int coroutine_fn | ||
355 | bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, | ||
356 | bool is_read) | ||
357 | @@ -XXX,XX +XXX,XX @@ bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, | ||
358 | return ret; | ||
359 | } | ||
360 | |||
361 | -static int coroutine_fn bdrv_co_rw_vmstate_entry(void *opaque) | ||
362 | -{ | ||
363 | - BdrvVmstateCo *co = opaque; | ||
364 | - | ||
365 | - return bdrv_co_rw_vmstate(co->bs, co->qiov, co->pos, co->is_read); | ||
366 | -} | ||
367 | - | ||
368 | -int bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, | ||
369 | - bool is_read) | ||
370 | -{ | ||
371 | - BdrvVmstateCo data = { | ||
372 | - .bs = bs, | ||
373 | - .qiov = qiov, | ||
374 | - .pos = pos, | ||
375 | - .is_read = is_read, | ||
376 | - }; | ||
377 | - | ||
378 | - return bdrv_run_co(bs, bdrv_co_rw_vmstate_entry, &data); | ||
379 | -} | ||
380 | - | ||
381 | int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf, | ||
382 | int64_t pos, int size) | ||
383 | { | ||
384 | @@ -XXX,XX +XXX,XX @@ void bdrv_aio_cancel_async(BlockAIOCB *acb) | ||
385 | /**************************************************************/ | ||
386 | /* Coroutine block device emulation */ | ||
387 | |||
388 | -static int coroutine_fn bdrv_flush_co_entry(void *opaque) | ||
389 | -{ | ||
390 | - return bdrv_co_flush(opaque); | ||
391 | -} | ||
392 | - | ||
393 | int coroutine_fn bdrv_co_flush(BlockDriverState *bs) | ||
394 | { | ||
395 | BdrvChild *primary_child = bdrv_primary_child(bs); | ||
396 | @@ -XXX,XX +XXX,XX @@ early_exit: | ||
397 | return ret; | ||
398 | } | ||
399 | |||
400 | -int bdrv_flush(BlockDriverState *bs) | ||
401 | -{ | ||
402 | - return bdrv_run_co(bs, bdrv_flush_co_entry, bs); | ||
403 | -} | ||
404 | - | ||
405 | -typedef struct DiscardCo { | ||
406 | - BdrvChild *child; | ||
407 | - int64_t offset; | ||
408 | - int64_t bytes; | ||
409 | -} DiscardCo; | ||
410 | - | ||
411 | -static int coroutine_fn bdrv_pdiscard_co_entry(void *opaque) | ||
412 | -{ | ||
413 | - DiscardCo *rwco = opaque; | ||
414 | - | ||
415 | - return bdrv_co_pdiscard(rwco->child, rwco->offset, rwco->bytes); | ||
416 | -} | ||
417 | - | ||
418 | int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset, | ||
419 | int64_t bytes) | ||
420 | { | ||
421 | @@ -XXX,XX +XXX,XX @@ out: | ||
422 | return ret; | ||
423 | } | ||
424 | |||
425 | -int bdrv_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes) | ||
426 | -{ | ||
427 | - DiscardCo rwco = { | ||
428 | - .child = child, | ||
429 | - .offset = offset, | ||
430 | - .bytes = bytes, | ||
431 | - }; | ||
432 | - | ||
433 | - return bdrv_run_co(child->bs, bdrv_pdiscard_co_entry, &rwco); | ||
434 | -} | ||
435 | - | ||
436 | int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf) | ||
437 | { | ||
438 | BlockDriver *drv = bs->drv; | ||
439 | @@ -XXX,XX +XXX,XX @@ out: | ||
440 | |||
441 | return ret; | ||
442 | } | ||
443 | - | ||
444 | -typedef struct TruncateCo { | ||
445 | - BdrvChild *child; | ||
446 | - int64_t offset; | ||
447 | - bool exact; | ||
448 | - PreallocMode prealloc; | ||
449 | - BdrvRequestFlags flags; | ||
450 | - Error **errp; | ||
451 | -} TruncateCo; | ||
452 | - | ||
453 | -static int coroutine_fn bdrv_truncate_co_entry(void *opaque) | ||
454 | -{ | ||
455 | - TruncateCo *tco = opaque; | ||
456 | - | ||
457 | - return bdrv_co_truncate(tco->child, tco->offset, tco->exact, | ||
458 | - tco->prealloc, tco->flags, tco->errp); | ||
459 | -} | ||
460 | - | ||
461 | -int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact, | ||
462 | - PreallocMode prealloc, BdrvRequestFlags flags, Error **errp) | ||
463 | -{ | ||
464 | - TruncateCo tco = { | ||
465 | - .child = child, | ||
466 | - .offset = offset, | ||
467 | - .exact = exact, | ||
468 | - .prealloc = prealloc, | ||
469 | - .flags = flags, | ||
470 | - .errp = errp, | ||
471 | - }; | ||
472 | - | ||
473 | - return bdrv_run_co(child->bs, bdrv_truncate_co_entry, &tco); | ||
474 | -} | ||
475 | -- | 141 | -- |
476 | 2.26.2 | 142 | 2.40.1 |
477 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | Test that even vectored IO requests with 1024 vector elements that are |
---|---|---|---|
2 | not aligned to the device's request alignment will succeed. | ||
2 | 3 | ||
3 | We have a very frequent pattern of creating a coroutine from a function | 4 | Reviewed-by: Eric Blake <eblake@redhat.com> |
4 | with several arguments: | 5 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> |
6 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> | ||
7 | Message-Id: <20230411173418.19549-5-hreitz@redhat.com> | ||
8 | --- | ||
9 | tests/qemu-iotests/tests/iov-padding | 85 ++++++++++++++++++++++++ | ||
10 | tests/qemu-iotests/tests/iov-padding.out | 59 ++++++++++++++++ | ||
11 | 2 files changed, 144 insertions(+) | ||
12 | create mode 100755 tests/qemu-iotests/tests/iov-padding | ||
13 | create mode 100644 tests/qemu-iotests/tests/iov-padding.out | ||
5 | 14 | ||
6 | - create a structure to pack parameters | 15 | diff --git a/tests/qemu-iotests/tests/iov-padding b/tests/qemu-iotests/tests/iov-padding |
7 | - create _entry function to call original function taking parameters | 16 | new file mode 100755 |
8 | from struct | 17 | index XXXXXXX..XXXXXXX |
9 | - do different magic to handle completion: set ret to NOT_DONE or | 18 | --- /dev/null |
10 | EINPROGRESS or use separate bool field | 19 | +++ b/tests/qemu-iotests/tests/iov-padding |
11 | - fill the struct and create coroutine from _entry function with this | 20 | @@ -XXX,XX +XXX,XX @@ |
12 | struct as a parameter | 21 | +#!/usr/bin/env bash |
13 | - do coroutine enter and BDRV_POLL_WHILE loop | 22 | +# group: rw quick |
14 | 23 | +# | |
15 | Let's reduce code duplication by generating coroutine wrappers. | 24 | +# Check the interaction of request padding (to fit alignment restrictions) with |
16 | 25 | +# vectored I/O from the guest | |
17 | This patch adds scripts/block-coroutine-wrapper.py together with some | 26 | +# |
18 | friends, which will generate functions with declared prototypes marked | 27 | +# Copyright Red Hat |
19 | by the 'generated_co_wrapper' specifier. | 28 | +# |
20 | 29 | +# This program is free software; you can redistribute it and/or modify | |
21 | The usage of new code generation is as follows: | 30 | +# it under the terms of the GNU General Public License as published by |
22 | 31 | +# the Free Software Foundation; either version 2 of the License, or | |
23 | 1. define the coroutine function somewhere | 32 | +# (at your option) any later version. |
24 | 33 | +# | |
25 | int coroutine_fn bdrv_co_NAME(...) {...} | 34 | +# This program is distributed in the hope that it will be useful, |
26 | 35 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of | |
27 | 2. declare in some header file | 36 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
28 | 37 | +# GNU General Public License for more details. | |
29 | int generated_co_wrapper bdrv_NAME(...); | 38 | +# |
30 | 39 | +# You should have received a copy of the GNU General Public License | |
31 | with same list of parameters (generated_co_wrapper is | 40 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
32 | defined in "include/block/block.h"). | 41 | +# |
33 | 42 | + | |
34 | 3. Make sure the block_gen_c declaration in block/meson.build | 43 | +seq=$(basename $0) |
35 | mentions the file with your marker function. | 44 | +echo "QA output created by $seq" |
36 | 45 | + | |
37 | Still, no function is now marked, this work is for the following | 46 | +status=1 # failure is the default! |
38 | commit. | 47 | + |
39 | 48 | +_cleanup() | |
40 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 49 | +{ |
41 | Reviewed-by: Eric Blake <eblake@redhat.com> | 50 | + _cleanup_test_img |
42 | Message-Id: <20200924185414.28642-5-vsementsov@virtuozzo.com> | 51 | +} |
43 | [Added encoding='utf-8' to open() calls as requested by Vladimir. Fixed | 52 | +trap "_cleanup; exit \$status" 0 1 2 3 15 |
44 | typo and grammar issues pointed out by Eric Blake. | 53 | + |
45 | --Stefan] | 54 | +# get standard environment, filters and checks |
46 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 55 | +cd .. |
47 | --- | 56 | +. ./common.rc |
48 | block/block-gen.h | 49 +++++++ | 57 | +. ./common.filter |
49 | include/block/block.h | 10 ++ | 58 | + |
50 | block/meson.build | 8 ++ | 59 | +_supported_fmt raw |
51 | docs/devel/block-coroutine-wrapper.rst | 54 +++++++ | 60 | +_supported_proto file |
52 | docs/devel/index.rst | 1 + | 61 | + |
53 | scripts/block-coroutine-wrapper.py | 188 +++++++++++++++++++++++++ | 62 | +_make_test_img 1M |
54 | 6 files changed, 310 insertions(+) | 63 | + |
55 | create mode 100644 block/block-gen.h | 64 | +IMGSPEC="driver=blkdebug,align=4096,image.driver=file,image.filename=$TEST_IMG" |
56 | create mode 100644 docs/devel/block-coroutine-wrapper.rst | 65 | + |
57 | create mode 100644 scripts/block-coroutine-wrapper.py | 66 | +# Four combinations: |
58 | 67 | +# - Offset 4096, length 1023 * 512 + 512: Fully aligned to 4k | |
59 | diff --git a/block/block-gen.h b/block/block-gen.h | 68 | +# - Offset 4096, length 1023 * 512 + 4096: Head is aligned, tail is not |
69 | +# - Offset 512, length 1023 * 512 + 512: Neither head nor tail are aligned | ||
70 | +# - Offset 512, length 1023 * 512 + 4096: Tail is aligned, head is not | ||
71 | +for start_offset in 4096 512; do | ||
72 | + for last_element_length in 512 4096; do | ||
73 | + length=$((1023 * 512 + $last_element_length)) | ||
74 | + | ||
75 | + echo | ||
76 | + echo "== performing 1024-element vectored requests to image (offset: $start_offset; length: $length) ==" | ||
77 | + | ||
78 | + # Fill with data for testing | ||
79 | + $QEMU_IO -c 'write -P 1 0 1M' "$TEST_IMG" | _filter_qemu_io | ||
80 | + | ||
81 | + # 1023 512-byte buffers, and then one with length $last_element_length | ||
82 | + cmd_params="-P 2 $start_offset $(yes 512 | head -n 1023 | tr '\n' ' ') $last_element_length" | ||
83 | + QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS_NO_FMT" $QEMU_IO \ | ||
84 | + -c "writev $cmd_params" \ | ||
85 | + --image-opts \ | ||
86 | + "$IMGSPEC" \ | ||
87 | + | _filter_qemu_io | ||
88 | + | ||
89 | + # Read all patterns -- read the part we just wrote with writev twice, | ||
90 | + # once "normally", and once with a readv, so we see that that works, too | ||
91 | + QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS_NO_FMT" $QEMU_IO \ | ||
92 | + -c "read -P 1 0 $start_offset" \ | ||
93 | + -c "read -P 2 $start_offset $length" \ | ||
94 | + -c "readv $cmd_params" \ | ||
95 | + -c "read -P 1 $((start_offset + length)) $((1024 * 1024 - length - start_offset))" \ | ||
96 | + --image-opts \ | ||
97 | + "$IMGSPEC" \ | ||
98 | + | _filter_qemu_io | ||
99 | + done | ||
100 | +done | ||
101 | + | ||
102 | +# success, all done | ||
103 | +echo "*** done" | ||
104 | +rm -f $seq.full | ||
105 | +status=0 | ||
106 | diff --git a/tests/qemu-iotests/tests/iov-padding.out b/tests/qemu-iotests/tests/iov-padding.out | ||
60 | new file mode 100644 | 107 | new file mode 100644 |
61 | index XXXXXXX..XXXXXXX | 108 | index XXXXXXX..XXXXXXX |
62 | --- /dev/null | 109 | --- /dev/null |
63 | +++ b/block/block-gen.h | 110 | +++ b/tests/qemu-iotests/tests/iov-padding.out |
64 | @@ -XXX,XX +XXX,XX @@ | 111 | @@ -XXX,XX +XXX,XX @@ |
65 | +/* | 112 | +QA output created by iov-padding |
66 | + * Block coroutine wrapping core, used by auto-generated block/block-gen.c | 113 | +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 |
67 | + * | ||
68 | + * Copyright (c) 2003 Fabrice Bellard | ||
69 | + * Copyright (c) 2020 Virtuozzo International GmbH | ||
70 | + * | ||
71 | + * Permission is hereby granted, free of charge, to any person obtaining a copy | ||
72 | + * of this software and associated documentation files (the "Software"), to deal | ||
73 | + * in the Software without restriction, including without limitation the rights | ||
74 | + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
75 | + * copies of the Software, and to permit persons to whom the Software is | ||
76 | + * furnished to do so, subject to the following conditions: | ||
77 | + * | ||
78 | + * The above copyright notice and this permission notice shall be included in | ||
79 | + * all copies or substantial portions of the Software. | ||
80 | + * | ||
81 | + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
82 | + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
83 | + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL | ||
84 | + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
85 | + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
86 | + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
87 | + * THE SOFTWARE. | ||
88 | + */ | ||
89 | + | 114 | + |
90 | +#ifndef BLOCK_BLOCK_GEN_H | 115 | +== performing 1024-element vectored requests to image (offset: 4096; length: 524288) == |
91 | +#define BLOCK_BLOCK_GEN_H | 116 | +wrote 1048576/1048576 bytes at offset 0 |
117 | +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
118 | +wrote 524288/524288 bytes at offset 4096 | ||
119 | +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
120 | +read 4096/4096 bytes at offset 0 | ||
121 | +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
122 | +read 524288/524288 bytes at offset 4096 | ||
123 | +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
124 | +read 524288/524288 bytes at offset 4096 | ||
125 | +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
126 | +read 520192/520192 bytes at offset 528384 | ||
127 | +508 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
92 | + | 128 | + |
93 | +#include "block/block_int.h" | 129 | +== performing 1024-element vectored requests to image (offset: 4096; length: 527872) == |
130 | +wrote 1048576/1048576 bytes at offset 0 | ||
131 | +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
132 | +wrote 527872/527872 bytes at offset 4096 | ||
133 | +515.500 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
134 | +read 4096/4096 bytes at offset 0 | ||
135 | +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
136 | +read 527872/527872 bytes at offset 4096 | ||
137 | +515.500 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
138 | +read 527872/527872 bytes at offset 4096 | ||
139 | +515.500 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
140 | +read 516608/516608 bytes at offset 531968 | ||
141 | +504.500 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
94 | + | 142 | + |
95 | +/* Base structure for argument packing structures */ | 143 | +== performing 1024-element vectored requests to image (offset: 512; length: 524288) == |
96 | +typedef struct BdrvPollCo { | 144 | +wrote 1048576/1048576 bytes at offset 0 |
97 | + BlockDriverState *bs; | 145 | +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) |
98 | + bool in_progress; | 146 | +wrote 524288/524288 bytes at offset 512 |
99 | + int ret; | 147 | +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) |
100 | + Coroutine *co; /* Keep pointer here for debugging */ | 148 | +read 512/512 bytes at offset 0 |
101 | +} BdrvPollCo; | 149 | +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) |
150 | +read 524288/524288 bytes at offset 512 | ||
151 | +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
152 | +read 524288/524288 bytes at offset 512 | ||
153 | +512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
154 | +read 523776/523776 bytes at offset 524800 | ||
155 | +511.500 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
102 | + | 156 | + |
103 | +static inline int bdrv_poll_co(BdrvPollCo *s) | 157 | +== performing 1024-element vectored requests to image (offset: 512; length: 527872) == |
104 | +{ | 158 | +wrote 1048576/1048576 bytes at offset 0 |
105 | + assert(!qemu_in_coroutine()); | 159 | +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) |
106 | + | 160 | +wrote 527872/527872 bytes at offset 512 |
107 | + bdrv_coroutine_enter(s->bs, s->co); | 161 | +515.500 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) |
108 | + BDRV_POLL_WHILE(s->bs, s->in_progress); | 162 | +read 512/512 bytes at offset 0 |
109 | + | 163 | +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) |
110 | + return s->ret; | 164 | +read 527872/527872 bytes at offset 512 |
111 | +} | 165 | +515.500 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) |
112 | + | 166 | +read 527872/527872 bytes at offset 512 |
113 | +#endif /* BLOCK_BLOCK_GEN_H */ | 167 | +515.500 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) |
114 | diff --git a/include/block/block.h b/include/block/block.h | 168 | +read 520192/520192 bytes at offset 528384 |
115 | index XXXXXXX..XXXXXXX 100644 | 169 | +508 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) |
116 | --- a/include/block/block.h | 170 | +*** done |
117 | +++ b/include/block/block.h | ||
118 | @@ -XXX,XX +XXX,XX @@ | ||
119 | #include "block/blockjob.h" | ||
120 | #include "qemu/hbitmap.h" | ||
121 | |||
122 | +/* | ||
123 | + * generated_co_wrapper | ||
124 | + * | ||
125 | + * Function specifier, which does nothing but mark functions to be | ||
126 | + * generated by scripts/block-coroutine-wrapper.py | ||
127 | + * | ||
128 | + * Read more in docs/devel/block-coroutine-wrapper.rst | ||
129 | + */ | ||
130 | +#define generated_co_wrapper | ||
131 | + | ||
132 | /* block.c */ | ||
133 | typedef struct BlockDriver BlockDriver; | ||
134 | typedef struct BdrvChild BdrvChild; | ||
135 | diff --git a/block/meson.build b/block/meson.build | ||
136 | index XXXXXXX..XXXXXXX 100644 | ||
137 | --- a/block/meson.build | ||
138 | +++ b/block/meson.build | ||
139 | @@ -XXX,XX +XXX,XX @@ module_block_h = custom_target('module_block.h', | ||
140 | command: [module_block_py, '@OUTPUT0@', modsrc]) | ||
141 | block_ss.add(module_block_h) | ||
142 | |||
143 | +wrapper_py = find_program('../scripts/block-coroutine-wrapper.py') | ||
144 | +block_gen_c = custom_target('block-gen.c', | ||
145 | + output: 'block-gen.c', | ||
146 | + input: files('../include/block/block.h', | ||
147 | + 'coroutines.h'), | ||
148 | + command: [wrapper_py, '@OUTPUT@', '@INPUT@']) | ||
149 | +block_ss.add(block_gen_c) | ||
150 | + | ||
151 | block_ss.add(files('stream.c')) | ||
152 | |||
153 | softmmu_ss.add(files('qapi-sysemu.c')) | ||
154 | diff --git a/docs/devel/block-coroutine-wrapper.rst b/docs/devel/block-coroutine-wrapper.rst | ||
155 | new file mode 100644 | ||
156 | index XXXXXXX..XXXXXXX | ||
157 | --- /dev/null | ||
158 | +++ b/docs/devel/block-coroutine-wrapper.rst | ||
159 | @@ -XXX,XX +XXX,XX @@ | ||
160 | +======================= | ||
161 | +block-coroutine-wrapper | ||
162 | +======================= | ||
163 | + | ||
164 | +A lot of functions in QEMU block layer (see ``block/*``) can only be | ||
165 | +called in coroutine context. Such functions are normally marked by the | ||
166 | +coroutine_fn specifier. Still, sometimes we need to call them from | ||
167 | +non-coroutine context; for this we need to start a coroutine, run the | ||
168 | +needed function from it and wait for the coroutine to finish in a | ||
169 | +BDRV_POLL_WHILE() loop. To run a coroutine we need a function with one | ||
170 | +void* argument. So for each coroutine_fn function which needs a | ||
171 | +non-coroutine interface, we should define a structure to pack the | ||
172 | +parameters, define a separate function to unpack the parameters and | ||
173 | +call the original function and finally define a new interface function | ||
174 | +with same list of arguments as original one, which will pack the | ||
175 | +parameters into a struct, create a coroutine, run it and wait in | ||
176 | +BDRV_POLL_WHILE() loop. It's boring to create such wrappers by hand, | ||
177 | +so we have a script to generate them. | ||
178 | + | ||
179 | +Usage | ||
180 | +===== | ||
181 | + | ||
182 | +Assume we have defined the ``coroutine_fn`` function | ||
183 | +``bdrv_co_foo(<some args>)`` and need a non-coroutine interface for it, | ||
184 | +called ``bdrv_foo(<same args>)``. In this case the script can help. To | ||
185 | +trigger the generation: | ||
186 | + | ||
187 | +1. You need ``bdrv_foo`` declaration somewhere (for example, in | ||
188 | + ``block/coroutines.h``) with the ``generated_co_wrapper`` mark, | ||
189 | + like this: | ||
190 | + | ||
191 | +.. code-block:: c | ||
192 | + | ||
193 | + int generated_co_wrapper bdrv_foo(<some args>); | ||
194 | + | ||
195 | +2. You need to feed this declaration to block-coroutine-wrapper script. | ||
196 | + For this, add the .h (or .c) file with the declaration to the | ||
197 | + ``input: files(...)`` list of ``block_gen_c`` target declaration in | ||
198 | + ``block/meson.build`` | ||
199 | + | ||
200 | +You are done. During the build, coroutine wrappers will be generated in | ||
201 | +``<BUILD_DIR>/block/block-gen.c``. | ||
202 | + | ||
203 | +Links | ||
204 | +===== | ||
205 | + | ||
206 | +1. The script location is ``scripts/block-coroutine-wrapper.py``. | ||
207 | + | ||
208 | +2. Generic place for private ``generated_co_wrapper`` declarations is | ||
209 | + ``block/coroutines.h``, for public declarations: | ||
210 | + ``include/block/block.h`` | ||
211 | + | ||
212 | +3. The core API of generated coroutine wrappers is placed in | ||
213 | + (not generated) ``block/block-gen.h`` | ||
214 | diff --git a/docs/devel/index.rst b/docs/devel/index.rst | ||
215 | index XXXXXXX..XXXXXXX 100644 | ||
216 | --- a/docs/devel/index.rst | ||
217 | +++ b/docs/devel/index.rst | ||
218 | @@ -XXX,XX +XXX,XX @@ Contents: | ||
219 | reset | ||
220 | s390-dasd-ipl | ||
221 | clocks | ||
222 | + block-coroutine-wrapper | ||
223 | diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py | ||
224 | new file mode 100644 | ||
225 | index XXXXXXX..XXXXXXX | ||
226 | --- /dev/null | ||
227 | +++ b/scripts/block-coroutine-wrapper.py | ||
228 | @@ -XXX,XX +XXX,XX @@ | ||
229 | +#! /usr/bin/env python3 | ||
230 | +"""Generate coroutine wrappers for block subsystem. | ||
231 | + | ||
232 | +The program parses one or several concatenated c files from stdin, | ||
233 | +searches for functions with the 'generated_co_wrapper' specifier | ||
234 | +and generates corresponding wrappers on stdout. | ||
235 | + | ||
236 | +Usage: block-coroutine-wrapper.py generated-file.c FILE.[ch]... | ||
237 | + | ||
238 | +Copyright (c) 2020 Virtuozzo International GmbH. | ||
239 | + | ||
240 | +This program is free software; you can redistribute it and/or modify | ||
241 | +it under the terms of the GNU General Public License as published by | ||
242 | +the Free Software Foundation; either version 2 of the License, or | ||
243 | +(at your option) any later version. | ||
244 | + | ||
245 | +This program is distributed in the hope that it will be useful, | ||
246 | +but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
247 | +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
248 | +GNU General Public License for more details. | ||
249 | + | ||
250 | +You should have received a copy of the GNU General Public License | ||
251 | +along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
252 | +""" | ||
253 | + | ||
254 | +import sys | ||
255 | +import re | ||
256 | +import subprocess | ||
257 | +import json | ||
258 | +from typing import Iterator | ||
259 | + | ||
260 | + | ||
261 | +def prettify(code: str) -> str: | ||
262 | + """Prettify code using clang-format if available""" | ||
263 | + | ||
264 | + try: | ||
265 | + style = json.dumps({ | ||
266 | + 'IndentWidth': 4, | ||
267 | + 'BraceWrapping': {'AfterFunction': True}, | ||
268 | + 'BreakBeforeBraces': 'Custom', | ||
269 | + 'SortIncludes': False, | ||
270 | + 'MaxEmptyLinesToKeep': 2, | ||
271 | + }) | ||
272 | + p = subprocess.run(['clang-format', f'-style={style}'], check=True, | ||
273 | + encoding='utf-8', input=code, | ||
274 | + stdout=subprocess.PIPE) | ||
275 | + return p.stdout | ||
276 | + except FileNotFoundError: | ||
277 | + return code | ||
278 | + | ||
279 | + | ||
280 | +def gen_header(): | ||
281 | + copyright = re.sub('^.*Copyright', 'Copyright', __doc__, flags=re.DOTALL) | ||
282 | + copyright = re.sub('^(?=.)', ' * ', copyright.strip(), flags=re.MULTILINE) | ||
283 | + copyright = re.sub('^$', ' *', copyright, flags=re.MULTILINE) | ||
284 | + return f"""\ | ||
285 | +/* | ||
286 | + * File is generated by scripts/block-coroutine-wrapper.py | ||
287 | + * | ||
288 | +{copyright} | ||
289 | + */ | ||
290 | + | ||
291 | +#include "qemu/osdep.h" | ||
292 | +#include "block/coroutines.h" | ||
293 | +#include "block/block-gen.h" | ||
294 | +#include "block/block_int.h"\ | ||
295 | +""" | ||
296 | + | ||
297 | + | ||
298 | +class ParamDecl: | ||
299 | + param_re = re.compile(r'(?P<decl>' | ||
300 | + r'(?P<type>.*[ *])' | ||
301 | + r'(?P<name>[a-z][a-z0-9_]*)' | ||
302 | + r')') | ||
303 | + | ||
304 | + def __init__(self, param_decl: str) -> None: | ||
305 | + m = self.param_re.match(param_decl.strip()) | ||
306 | + if m is None: | ||
307 | + raise ValueError(f'Wrong parameter declaration: "{param_decl}"') | ||
308 | + self.decl = m.group('decl') | ||
309 | + self.type = m.group('type') | ||
310 | + self.name = m.group('name') | ||
311 | + | ||
312 | + | ||
313 | +class FuncDecl: | ||
314 | + def __init__(self, return_type: str, name: str, args: str) -> None: | ||
315 | + self.return_type = return_type.strip() | ||
316 | + self.name = name.strip() | ||
317 | + self.args = [ParamDecl(arg.strip()) for arg in args.split(',')] | ||
318 | + | ||
319 | + def gen_list(self, format: str) -> str: | ||
320 | + return ', '.join(format.format_map(arg.__dict__) for arg in self.args) | ||
321 | + | ||
322 | + def gen_block(self, format: str) -> str: | ||
323 | + return '\n'.join(format.format_map(arg.__dict__) for arg in self.args) | ||
324 | + | ||
325 | + | ||
326 | +# Match wrappers declared with a generated_co_wrapper mark | ||
327 | +func_decl_re = re.compile(r'^int\s*generated_co_wrapper\s*' | ||
328 | + r'(?P<wrapper_name>[a-z][a-z0-9_]*)' | ||
329 | + r'\((?P<args>[^)]*)\);$', re.MULTILINE) | ||
330 | + | ||
331 | + | ||
332 | +def func_decl_iter(text: str) -> Iterator: | ||
333 | + for m in func_decl_re.finditer(text): | ||
334 | + yield FuncDecl(return_type='int', | ||
335 | + name=m.group('wrapper_name'), | ||
336 | + args=m.group('args')) | ||
337 | + | ||
338 | + | ||
339 | +def snake_to_camel(func_name: str) -> str: | ||
340 | + """ | ||
341 | + Convert underscore names like 'some_function_name' to camel-case like | ||
342 | + 'SomeFunctionName' | ||
343 | + """ | ||
344 | + words = func_name.split('_') | ||
345 | + words = [w[0].upper() + w[1:] for w in words] | ||
346 | + return ''.join(words) | ||
347 | + | ||
348 | + | ||
349 | +def gen_wrapper(func: FuncDecl) -> str: | ||
350 | + assert func.name.startswith('bdrv_') | ||
351 | + assert not func.name.startswith('bdrv_co_') | ||
352 | + assert func.return_type == 'int' | ||
353 | + assert func.args[0].type in ['BlockDriverState *', 'BdrvChild *'] | ||
354 | + | ||
355 | + name = 'bdrv_co_' + func.name[5:] | ||
356 | + bs = 'bs' if func.args[0].type == 'BlockDriverState *' else 'child->bs' | ||
357 | + struct_name = snake_to_camel(name) | ||
358 | + | ||
359 | + return f"""\ | ||
360 | +/* | ||
361 | + * Wrappers for {name} | ||
362 | + */ | ||
363 | + | ||
364 | +typedef struct {struct_name} {{ | ||
365 | + BdrvPollCo poll_state; | ||
366 | +{ func.gen_block(' {decl};') } | ||
367 | +}} {struct_name}; | ||
368 | + | ||
369 | +static void coroutine_fn {name}_entry(void *opaque) | ||
370 | +{{ | ||
371 | + {struct_name} *s = opaque; | ||
372 | + | ||
373 | + s->poll_state.ret = {name}({ func.gen_list('s->{name}') }); | ||
374 | + s->poll_state.in_progress = false; | ||
375 | + | ||
376 | + aio_wait_kick(); | ||
377 | +}} | ||
378 | + | ||
379 | +int {func.name}({ func.gen_list('{decl}') }) | ||
380 | +{{ | ||
381 | + if (qemu_in_coroutine()) {{ | ||
382 | + return {name}({ func.gen_list('{name}') }); | ||
383 | + }} else {{ | ||
384 | + {struct_name} s = {{ | ||
385 | + .poll_state.bs = {bs}, | ||
386 | + .poll_state.in_progress = true, | ||
387 | + | ||
388 | +{ func.gen_block(' .{name} = {name},') } | ||
389 | + }}; | ||
390 | + | ||
391 | + s.poll_state.co = qemu_coroutine_create({name}_entry, &s); | ||
392 | + | ||
393 | + return bdrv_poll_co(&s.poll_state); | ||
394 | + }} | ||
395 | +}}""" | ||
396 | + | ||
397 | + | ||
398 | +def gen_wrappers(input_code: str) -> str: | ||
399 | + res = '' | ||
400 | + for func in func_decl_iter(input_code): | ||
401 | + res += '\n\n\n' | ||
402 | + res += gen_wrapper(func) | ||
403 | + | ||
404 | + return prettify(res) # prettify to wrap long lines | ||
405 | + | ||
406 | + | ||
407 | +if __name__ == '__main__': | ||
408 | + if len(sys.argv) < 3: | ||
409 | + exit(f'Usage: {sys.argv[0]} OUT_FILE.c IN_FILE.[ch]...') | ||
410 | + | ||
411 | + with open(sys.argv[1], 'w', encoding='utf-8') as f_out: | ||
412 | + f_out.write(gen_header()) | ||
413 | + for fname in sys.argv[2:]: | ||
414 | + with open(fname, encoding='utf-8') as f_in: | ||
415 | + f_out.write(gen_wrappers(f_in.read())) | ||
416 | + f_out.write('\n') | ||
417 | -- | 171 | -- |
418 | 2.26.2 | 172 | 2.40.1 |
419 | diff view generated by jsdifflib |
1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> | 1 | From: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
---|---|---|---|
2 | 2 | ||
3 | Use the NVMe register definitions from "block/nvme.h" which | 3 | data_end field in BDRVParallelsState is set to the biggest offset present |
4 | ease a bit reviewing the code while matching the datasheet. | 4 | in BAT. If this offset is outside of the image, any further write will |
5 | create the cluster at this offset and/or the image will be truncated to | ||
6 | this offset on close. This is definitely not correct. | ||
5 | 7 | ||
6 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 8 | Raise an error in parallels_open() if data_end points outside the image |
7 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 9 | and it is not a check (let the check to repaire the image). Set data_end |
8 | Message-Id: <20200922083821.578519-6-philmd@redhat.com> | 10 | to the end of the cluster with the last correct offset. |
11 | |||
12 | Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> | ||
13 | Message-Id: <20230424093147.197643-2-alexander.ivanov@virtuozzo.com> | ||
14 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
15 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> | ||
9 | --- | 16 | --- |
10 | block/nvme.c | 21 +++++++++++---------- | 17 | block/parallels.c | 17 +++++++++++++++++ |
11 | 1 file changed, 11 insertions(+), 10 deletions(-) | 18 | 1 file changed, 17 insertions(+) |
12 | 19 | ||
13 | diff --git a/block/nvme.c b/block/nvme.c | 20 | diff --git a/block/parallels.c b/block/parallels.c |
14 | index XXXXXXX..XXXXXXX 100644 | 21 | index XXXXXXX..XXXXXXX 100644 |
15 | --- a/block/nvme.c | 22 | --- a/block/parallels.c |
16 | +++ b/block/nvme.c | 23 | +++ b/block/parallels.c |
17 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | 24 | @@ -XXX,XX +XXX,XX @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, |
18 | * Initialization". */ | 25 | BDRVParallelsState *s = bs->opaque; |
19 | 26 | ParallelsHeader ph; | |
20 | cap = le64_to_cpu(regs->cap); | 27 | int ret, size, i; |
21 | - if (!(cap & (1ULL << 37))) { | 28 | + int64_t file_nb_sectors; |
22 | + if (!NVME_CAP_CSS(cap)) { | 29 | QemuOpts *opts = NULL; |
23 | error_setg(errp, "Device doesn't support NVMe command set"); | 30 | Error *local_err = NULL; |
24 | ret = -EINVAL; | 31 | char *buf; |
25 | goto out; | 32 | @@ -XXX,XX +XXX,XX @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, |
33 | return ret; | ||
26 | } | 34 | } |
27 | 35 | ||
28 | - s->page_size = MAX(4096, 1 << (12 + ((cap >> 48) & 0xF))); | 36 | + file_nb_sectors = bdrv_nb_sectors(bs->file->bs); |
29 | - s->doorbell_scale = (4 << (((cap >> 32) & 0xF))) / sizeof(uint32_t); | 37 | + if (file_nb_sectors < 0) { |
30 | + s->page_size = MAX(4096, 1 << NVME_CAP_MPSMIN(cap)); | 38 | + return -EINVAL; |
31 | + s->doorbell_scale = (4 << NVME_CAP_DSTRD(cap)) / sizeof(uint32_t); | 39 | + } |
32 | bs->bl.opt_mem_alignment = s->page_size; | 40 | + |
33 | - timeout_ms = MIN(500 * ((cap >> 24) & 0xFF), 30000); | 41 | ret = bdrv_pread(bs->file, 0, sizeof(ph), &ph, 0); |
34 | + timeout_ms = MIN(500 * NVME_CAP_TO(cap), 30000); | 42 | if (ret < 0) { |
35 | 43 | goto fail; | |
36 | /* Reset device to get a clean state. */ | 44 | @@ -XXX,XX +XXX,XX @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, |
37 | regs->cc = cpu_to_le32(le32_to_cpu(regs->cc) & 0xFE); | 45 | |
38 | /* Wait for CSTS.RDY = 0. */ | 46 | for (i = 0; i < s->bat_size; i++) { |
39 | deadline = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + timeout_ms * SCALE_MS; | 47 | int64_t off = bat2sect(s, i); |
40 | - while (le32_to_cpu(regs->csts) & 0x1) { | 48 | + if (off >= file_nb_sectors) { |
41 | + while (NVME_CSTS_RDY(le32_to_cpu(regs->csts))) { | 49 | + if (flags & BDRV_O_CHECK) { |
42 | if (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) > deadline) { | 50 | + continue; |
43 | error_setg(errp, "Timeout while waiting for device to reset (%" | 51 | + } |
44 | PRId64 " ms)", | 52 | + error_setg(errp, "parallels: Offset %" PRIi64 " in BAT[%d] entry " |
45 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | 53 | + "is larger than file size (%" PRIi64 ")", |
46 | } | 54 | + off << BDRV_SECTOR_BITS, i, |
47 | s->nr_queues = 1; | 55 | + file_nb_sectors << BDRV_SECTOR_BITS); |
48 | QEMU_BUILD_BUG_ON(NVME_QUEUE_SIZE & 0xF000); | 56 | + ret = -EINVAL; |
49 | - regs->aqa = cpu_to_le32((NVME_QUEUE_SIZE << 16) | NVME_QUEUE_SIZE); | 57 | + goto fail; |
50 | + regs->aqa = cpu_to_le32((NVME_QUEUE_SIZE << AQA_ACQS_SHIFT) | | 58 | + } |
51 | + (NVME_QUEUE_SIZE << AQA_ASQS_SHIFT)); | 59 | if (off >= s->data_end) { |
52 | regs->asq = cpu_to_le64(s->queues[INDEX_ADMIN]->sq.iova); | 60 | s->data_end = off + s->tracks; |
53 | regs->acq = cpu_to_le64(s->queues[INDEX_ADMIN]->cq.iova); | 61 | } |
54 | |||
55 | /* After setting up all control registers we can enable device now. */ | ||
56 | - regs->cc = cpu_to_le32((ctz32(NVME_CQ_ENTRY_BYTES) << 20) | | ||
57 | - (ctz32(NVME_SQ_ENTRY_BYTES) << 16) | | ||
58 | - 0x1); | ||
59 | + regs->cc = cpu_to_le32((ctz32(NVME_CQ_ENTRY_BYTES) << CC_IOCQES_SHIFT) | | ||
60 | + (ctz32(NVME_SQ_ENTRY_BYTES) << CC_IOSQES_SHIFT) | | ||
61 | + CC_EN_MASK); | ||
62 | /* Wait for CSTS.RDY = 1. */ | ||
63 | now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); | ||
64 | deadline = now + timeout_ms * 1000000; | ||
65 | - while (!(le32_to_cpu(regs->csts) & 0x1)) { | ||
66 | + while (!NVME_CSTS_RDY(le32_to_cpu(regs->csts))) { | ||
67 | if (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) > deadline) { | ||
68 | error_setg(errp, "Timeout while waiting for device to start (%" | ||
69 | PRId64 " ms)", | ||
70 | -- | 62 | -- |
71 | 2.26.2 | 63 | 2.40.1 |
72 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | From: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
---|---|---|---|
2 | 2 | ||
3 | This is the only non-ascii character in the file and it doesn't really | 3 | Don't let high_off be more than the file size even if we don't fix the |
4 | needed here. Let's use normal "'" symbol for consistency with the rest | 4 | image. |
5 | 11 occurrences of "'" in the file. | ||
6 | 5 | ||
7 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 6 | Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
8 | Reviewed-by: Eric Blake <eblake@redhat.com> | 7 | Reviewed-by: Denis V. Lunev <den@openvz.org> |
9 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 8 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> |
9 | Message-Id: <20230424093147.197643-3-alexander.ivanov@virtuozzo.com> | ||
10 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
11 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> | ||
10 | --- | 12 | --- |
11 | include/block/block.h | 2 +- | 13 | block/parallels.c | 4 ++-- |
12 | 1 file changed, 1 insertion(+), 1 deletion(-) | 14 | 1 file changed, 2 insertions(+), 2 deletions(-) |
13 | 15 | ||
14 | diff --git a/include/block/block.h b/include/block/block.h | 16 | diff --git a/block/parallels.c b/block/parallels.c |
15 | index XXXXXXX..XXXXXXX 100644 | 17 | index XXXXXXX..XXXXXXX 100644 |
16 | --- a/include/block/block.h | 18 | --- a/block/parallels.c |
17 | +++ b/include/block/block.h | 19 | +++ b/block/parallels.c |
18 | @@ -XXX,XX +XXX,XX @@ enum BdrvChildRoleBits { | 20 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
19 | BDRV_CHILD_FILTERED = (1 << 2), | 21 | fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i); |
20 | 22 | res->corruptions++; | |
21 | /* | 23 | if (fix & BDRV_FIX_ERRORS) { |
22 | - * Child from which to read all data that isn’t allocated in the | 24 | - prev_off = 0; |
23 | + * Child from which to read all data that isn't allocated in the | 25 | s->bat_bitmap[i] = 0; |
24 | * parent (i.e., the backing child); such data is copied to the | 26 | res->corruptions_fixed++; |
25 | * parent through COW (and optionally COR). | 27 | flush_bat = true; |
26 | * This field is mutually exclusive with DATA, METADATA, and | 28 | - continue; |
29 | } | ||
30 | + prev_off = 0; | ||
31 | + continue; | ||
32 | } | ||
33 | |||
34 | res->bfi.allocated_clusters++; | ||
27 | -- | 35 | -- |
28 | 2.26.2 | 36 | 2.40.1 |
29 | diff view generated by jsdifflib |
1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> | 1 | From: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
---|---|---|---|
2 | 2 | ||
3 | Per the datasheet sections 3.1.13/3.1.14: | 3 | Set data_end to the end of the last cluster inside the image. In such a |
4 | "The host should not read the doorbell registers." | 4 | way we can be sure that corrupted offsets in the BAT can't affect on the |
5 | image size. If there are no allocated clusters set image_end_offset by | ||
6 | data_end. | ||
5 | 7 | ||
6 | As we don't need read access, map the doorbells with write-only | 8 | Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
7 | permission. We keep a reference to this mapped address in the | 9 | Reviewed-by: Denis V. Lunev <den@openvz.org> |
8 | BDRVNVMeState structure. | 10 | Message-Id: <20230424093147.197643-4-alexander.ivanov@virtuozzo.com> |
11 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
12 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> | ||
13 | --- | ||
14 | block/parallels.c | 8 +++++++- | ||
15 | 1 file changed, 7 insertions(+), 1 deletion(-) | ||
9 | 16 | ||
10 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 17 | diff --git a/block/parallels.c b/block/parallels.c |
11 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
12 | Message-Id: <20200922083821.578519-3-philmd@redhat.com> | ||
13 | --- | ||
14 | block/nvme.c | 29 +++++++++++++++++++---------- | ||
15 | 1 file changed, 19 insertions(+), 10 deletions(-) | ||
16 | |||
17 | diff --git a/block/nvme.c b/block/nvme.c | ||
18 | index XXXXXXX..XXXXXXX 100644 | 18 | index XXXXXXX..XXXXXXX 100644 |
19 | --- a/block/nvme.c | 19 | --- a/block/parallels.c |
20 | +++ b/block/nvme.c | 20 | +++ b/block/parallels.c |
21 | @@ -XXX,XX +XXX,XX @@ | 21 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
22 | #define NVME_SQ_ENTRY_BYTES 64 | ||
23 | #define NVME_CQ_ENTRY_BYTES 16 | ||
24 | #define NVME_QUEUE_SIZE 128 | ||
25 | -#define NVME_BAR_SIZE 8192 | ||
26 | +#define NVME_DOORBELL_SIZE 4096 | ||
27 | |||
28 | /* | ||
29 | * We have to leave one slot empty as that is the full queue case where | ||
30 | @@ -XXX,XX +XXX,XX @@ typedef struct { | ||
31 | /* Memory mapped registers */ | ||
32 | typedef volatile struct { | ||
33 | NvmeBar ctrl; | ||
34 | - struct { | ||
35 | - uint32_t sq_tail; | ||
36 | - uint32_t cq_head; | ||
37 | - } doorbells[]; | ||
38 | } NVMeRegs; | ||
39 | |||
40 | #define INDEX_ADMIN 0 | ||
41 | @@ -XXX,XX +XXX,XX @@ struct BDRVNVMeState { | ||
42 | AioContext *aio_context; | ||
43 | QEMUVFIOState *vfio; | ||
44 | NVMeRegs *regs; | ||
45 | + /* Memory mapped registers */ | ||
46 | + volatile struct { | ||
47 | + uint32_t sq_tail; | ||
48 | + uint32_t cq_head; | ||
49 | + } *doorbells; | ||
50 | /* The submission/completion queue pairs. | ||
51 | * [0]: admin queue. | ||
52 | * [1..]: io queues. | ||
53 | @@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s, | ||
54 | error_propagate(errp, local_err); | ||
55 | goto fail; | ||
56 | } | ||
57 | - q->sq.doorbell = &s->regs->doorbells[idx * s->doorbell_scale].sq_tail; | ||
58 | + q->sq.doorbell = &s->doorbells[idx * s->doorbell_scale].sq_tail; | ||
59 | |||
60 | nvme_init_queue(s, &q->cq, size, NVME_CQ_ENTRY_BYTES, &local_err); | ||
61 | if (local_err) { | ||
62 | error_propagate(errp, local_err); | ||
63 | goto fail; | ||
64 | } | ||
65 | - q->cq.doorbell = &s->regs->doorbells[idx * s->doorbell_scale].cq_head; | ||
66 | + q->cq.doorbell = &s->doorbells[idx * s->doorbell_scale].cq_head; | ||
67 | |||
68 | return q; | ||
69 | fail: | ||
70 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | ||
71 | goto out; | ||
72 | } | ||
73 | |||
74 | - s->regs = qemu_vfio_pci_map_bar(s->vfio, 0, 0, NVME_BAR_SIZE, | ||
75 | + s->regs = qemu_vfio_pci_map_bar(s->vfio, 0, 0, sizeof(NvmeBar), | ||
76 | PROT_READ | PROT_WRITE, errp); | ||
77 | if (!s->regs) { | ||
78 | ret = -EINVAL; | ||
79 | goto out; | ||
80 | } | ||
81 | - | ||
82 | /* Perform initialize sequence as described in NVMe spec "7.6.1 | ||
83 | * Initialization". */ | ||
84 | |||
85 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | ||
86 | } | 22 | } |
87 | } | 23 | } |
88 | 24 | ||
89 | + s->doorbells = qemu_vfio_pci_map_bar(s->vfio, 0, sizeof(NvmeBar), | 25 | - res->image_end_offset = high_off + s->cluster_size; |
90 | + NVME_DOORBELL_SIZE, PROT_WRITE, errp); | 26 | + if (high_off == 0) { |
91 | + if (!s->doorbells) { | 27 | + res->image_end_offset = s->data_end << BDRV_SECTOR_BITS; |
92 | + ret = -EINVAL; | 28 | + } else { |
93 | + goto out; | 29 | + res->image_end_offset = high_off + s->cluster_size; |
30 | + s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS; | ||
94 | + } | 31 | + } |
95 | + | 32 | + |
96 | /* Set up admin queue. */ | 33 | if (size > res->image_end_offset) { |
97 | s->queues = g_new(NVMeQueuePair *, 1); | 34 | int64_t count; |
98 | s->queues[INDEX_ADMIN] = nvme_create_queue_pair(s, aio_context, 0, | 35 | count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size); |
99 | @@ -XXX,XX +XXX,XX @@ static void nvme_close(BlockDriverState *bs) | ||
100 | &s->irq_notifier[MSIX_SHARED_IRQ_IDX], | ||
101 | false, NULL, NULL); | ||
102 | event_notifier_cleanup(&s->irq_notifier[MSIX_SHARED_IRQ_IDX]); | ||
103 | - qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->regs, 0, NVME_BAR_SIZE); | ||
104 | + qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->doorbells, | ||
105 | + sizeof(NvmeBar), NVME_DOORBELL_SIZE); | ||
106 | + qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->regs, 0, sizeof(NvmeBar)); | ||
107 | qemu_vfio_close(s->vfio); | ||
108 | |||
109 | g_free(s->device); | ||
110 | -- | 36 | -- |
111 | 2.26.2 | 37 | 2.40.1 |
112 | diff view generated by jsdifflib |
1 | From: Eric Auger <eric.auger@redhat.com> | 1 | From: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
---|---|---|---|
2 | 2 | ||
3 | The IOVA allocator currently ignores host reserved regions. | 3 | This helper will be reused in next patches during parallels_co_check |
4 | As a result some chosen IOVAs may collide with some of them, | 4 | rework to simplify its code. |
5 | resulting in VFIO MAP_DMA errors later on. This happens on ARM | ||
6 | where the MSI reserved window quickly is encountered: | ||
7 | [0x8000000, 0x8100000]. since 5.4 kernel, VFIO returns the usable | ||
8 | IOVA regions. So let's enumerate them in the prospect to avoid | ||
9 | them, later on. | ||
10 | 5 | ||
11 | Signed-off-by: Eric Auger <eric.auger@redhat.com> | 6 | Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
12 | Message-id: 20200929085550.30926-2-eric.auger@redhat.com | 7 | Reviewed-by: Denis V. Lunev <den@openvz.org> |
13 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 8 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> |
9 | Message-Id: <20230424093147.197643-5-alexander.ivanov@virtuozzo.com> | ||
10 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
11 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> | ||
14 | --- | 12 | --- |
15 | util/vfio-helpers.c | 72 +++++++++++++++++++++++++++++++++++++++++++-- | 13 | block/parallels.c | 11 ++++++++--- |
16 | 1 file changed, 70 insertions(+), 2 deletions(-) | 14 | 1 file changed, 8 insertions(+), 3 deletions(-) |
17 | 15 | ||
18 | diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c | 16 | diff --git a/block/parallels.c b/block/parallels.c |
19 | index XXXXXXX..XXXXXXX 100644 | 17 | index XXXXXXX..XXXXXXX 100644 |
20 | --- a/util/vfio-helpers.c | 18 | --- a/block/parallels.c |
21 | +++ b/util/vfio-helpers.c | 19 | +++ b/block/parallels.c |
22 | @@ -XXX,XX +XXX,XX @@ typedef struct { | 20 | @@ -XXX,XX +XXX,XX @@ static int64_t block_status(BDRVParallelsState *s, int64_t sector_num, |
23 | uint64_t iova; | 21 | return start_off; |
24 | } IOVAMapping; | ||
25 | |||
26 | +struct IOVARange { | ||
27 | + uint64_t start; | ||
28 | + uint64_t end; | ||
29 | +}; | ||
30 | + | ||
31 | struct QEMUVFIOState { | ||
32 | QemuMutex lock; | ||
33 | |||
34 | @@ -XXX,XX +XXX,XX @@ struct QEMUVFIOState { | ||
35 | int device; | ||
36 | RAMBlockNotifier ram_notifier; | ||
37 | struct vfio_region_info config_region_info, bar_region_info[6]; | ||
38 | + struct IOVARange *usable_iova_ranges; | ||
39 | + uint8_t nb_iova_ranges; | ||
40 | |||
41 | /* These fields are protected by @lock */ | ||
42 | /* VFIO's IO virtual address space is managed by splitting into a few | ||
43 | @@ -XXX,XX +XXX,XX @@ static int qemu_vfio_pci_write_config(QEMUVFIOState *s, void *buf, int size, int | ||
44 | return ret == size ? 0 : -errno; | ||
45 | } | 22 | } |
46 | 23 | ||
47 | +static void collect_usable_iova_ranges(QEMUVFIOState *s, void *buf) | 24 | +static void parallels_set_bat_entry(BDRVParallelsState *s, |
25 | + uint32_t index, uint32_t offset) | ||
48 | +{ | 26 | +{ |
49 | + struct vfio_iommu_type1_info *info = (struct vfio_iommu_type1_info *)buf; | 27 | + s->bat_bitmap[index] = cpu_to_le32(offset); |
50 | + struct vfio_info_cap_header *cap = (void *)buf + info->cap_offset; | 28 | + bitmap_set(s->bat_dirty_bmap, bat_entry_off(index) / s->bat_dirty_block, 1); |
51 | + struct vfio_iommu_type1_info_cap_iova_range *cap_iova_range; | ||
52 | + int i; | ||
53 | + | ||
54 | + while (cap->id != VFIO_IOMMU_TYPE1_INFO_CAP_IOVA_RANGE) { | ||
55 | + if (!cap->next) { | ||
56 | + return; | ||
57 | + } | ||
58 | + cap = (struct vfio_info_cap_header *)(buf + cap->next); | ||
59 | + } | ||
60 | + | ||
61 | + cap_iova_range = (struct vfio_iommu_type1_info_cap_iova_range *)cap; | ||
62 | + | ||
63 | + s->nb_iova_ranges = cap_iova_range->nr_iovas; | ||
64 | + if (s->nb_iova_ranges > 1) { | ||
65 | + s->usable_iova_ranges = | ||
66 | + g_realloc(s->usable_iova_ranges, | ||
67 | + s->nb_iova_ranges * sizeof(struct IOVARange)); | ||
68 | + } | ||
69 | + | ||
70 | + for (i = 0; i < s->nb_iova_ranges; i++) { | ||
71 | + s->usable_iova_ranges[i].start = cap_iova_range->iova_ranges[i].start; | ||
72 | + s->usable_iova_ranges[i].end = cap_iova_range->iova_ranges[i].end; | ||
73 | + } | ||
74 | +} | 29 | +} |
75 | + | 30 | + |
76 | static int qemu_vfio_init_pci(QEMUVFIOState *s, const char *device, | 31 | static int64_t coroutine_fn GRAPH_RDLOCK |
77 | Error **errp) | 32 | allocate_clusters(BlockDriverState *bs, int64_t sector_num, |
78 | { | 33 | int nb_sectors, int *pnum) |
79 | @@ -XXX,XX +XXX,XX @@ static int qemu_vfio_init_pci(QEMUVFIOState *s, const char *device, | 34 | @@ -XXX,XX +XXX,XX @@ allocate_clusters(BlockDriverState *bs, int64_t sector_num, |
80 | int i; | ||
81 | uint16_t pci_cmd; | ||
82 | struct vfio_group_status group_status = { .argsz = sizeof(group_status) }; | ||
83 | - struct vfio_iommu_type1_info iommu_info = { .argsz = sizeof(iommu_info) }; | ||
84 | + struct vfio_iommu_type1_info *iommu_info = NULL; | ||
85 | + size_t iommu_info_size = sizeof(*iommu_info); | ||
86 | struct vfio_device_info device_info = { .argsz = sizeof(device_info) }; | ||
87 | char *group_file = NULL; | ||
88 | |||
89 | + s->usable_iova_ranges = NULL; | ||
90 | + | ||
91 | /* Create a new container */ | ||
92 | s->container = open("/dev/vfio/vfio", O_RDWR); | ||
93 | |||
94 | @@ -XXX,XX +XXX,XX @@ static int qemu_vfio_init_pci(QEMUVFIOState *s, const char *device, | ||
95 | goto fail; | ||
96 | } | 35 | } |
97 | 36 | ||
98 | + iommu_info = g_malloc0(iommu_info_size); | 37 | for (i = 0; i < to_allocate; i++) { |
99 | + iommu_info->argsz = iommu_info_size; | 38 | - s->bat_bitmap[idx + i] = cpu_to_le32(s->data_end / s->off_multiplier); |
100 | + | 39 | + parallels_set_bat_entry(s, idx + i, s->data_end / s->off_multiplier); |
101 | /* Get additional IOMMU info */ | 40 | s->data_end += s->tracks; |
102 | - if (ioctl(s->container, VFIO_IOMMU_GET_INFO, &iommu_info)) { | 41 | - bitmap_set(s->bat_dirty_bmap, |
103 | + if (ioctl(s->container, VFIO_IOMMU_GET_INFO, iommu_info)) { | 42 | - bat_entry_off(idx + i) / s->bat_dirty_block, 1); |
104 | error_setg_errno(errp, errno, "Failed to get IOMMU info"); | ||
105 | ret = -errno; | ||
106 | goto fail; | ||
107 | } | 43 | } |
108 | 44 | ||
109 | + /* | 45 | return bat2sect(s, idx) + sector_num % s->tracks; |
110 | + * if the kernel does not report usable IOVA regions, choose | ||
111 | + * the legacy [QEMU_VFIO_IOVA_MIN, QEMU_VFIO_IOVA_MAX -1] region | ||
112 | + */ | ||
113 | + s->nb_iova_ranges = 1; | ||
114 | + s->usable_iova_ranges = g_new0(struct IOVARange, 1); | ||
115 | + s->usable_iova_ranges[0].start = QEMU_VFIO_IOVA_MIN; | ||
116 | + s->usable_iova_ranges[0].end = QEMU_VFIO_IOVA_MAX - 1; | ||
117 | + | ||
118 | + if (iommu_info->argsz > iommu_info_size) { | ||
119 | + iommu_info_size = iommu_info->argsz; | ||
120 | + iommu_info = g_realloc(iommu_info, iommu_info_size); | ||
121 | + if (ioctl(s->container, VFIO_IOMMU_GET_INFO, iommu_info)) { | ||
122 | + ret = -errno; | ||
123 | + goto fail; | ||
124 | + } | ||
125 | + collect_usable_iova_ranges(s, iommu_info); | ||
126 | + } | ||
127 | + | ||
128 | s->device = ioctl(s->group, VFIO_GROUP_GET_DEVICE_FD, device); | ||
129 | |||
130 | if (s->device < 0) { | ||
131 | @@ -XXX,XX +XXX,XX @@ static int qemu_vfio_init_pci(QEMUVFIOState *s, const char *device, | ||
132 | if (ret) { | ||
133 | goto fail; | ||
134 | } | ||
135 | + g_free(iommu_info); | ||
136 | return 0; | ||
137 | fail: | ||
138 | + g_free(s->usable_iova_ranges); | ||
139 | + s->usable_iova_ranges = NULL; | ||
140 | + s->nb_iova_ranges = 0; | ||
141 | + g_free(iommu_info); | ||
142 | close(s->group); | ||
143 | fail_container: | ||
144 | close(s->container); | ||
145 | @@ -XXX,XX +XXX,XX @@ void qemu_vfio_close(QEMUVFIOState *s) | ||
146 | qemu_vfio_undo_mapping(s, &s->mappings[i], NULL); | ||
147 | } | ||
148 | ram_block_notifier_remove(&s->ram_notifier); | ||
149 | + g_free(s->usable_iova_ranges); | ||
150 | + s->nb_iova_ranges = 0; | ||
151 | qemu_vfio_reset(s); | ||
152 | close(s->device); | ||
153 | close(s->group); | ||
154 | -- | 46 | -- |
155 | 2.26.2 | 47 | 2.40.1 |
156 | diff view generated by jsdifflib |
1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> | 1 | From: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
---|---|---|---|
2 | 2 | ||
3 | We only access the I/O register in nvme_init(). | 3 | BAT is written in the context of conventional operations over the image |
4 | Remove the reference in BDRVNVMeState and reduce its scope. | 4 | inside bdrv_co_flush() when it calls parallels_co_flush_to_os() callback. |
5 | Thus we should not modify BAT array directly, but call | ||
6 | parallels_set_bat_entry() helper and bdrv_co_flush() further on. After | ||
7 | that there is no need to manually write BAT and track its modification. | ||
5 | 8 | ||
6 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 9 | This makes code more generic and allows to split parallels_set_bat_entry() |
7 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 10 | for independent pieces. |
8 | Message-Id: <20200922083821.578519-4-philmd@redhat.com> | 11 | |
12 | Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> | ||
13 | Reviewed-by: Denis V. Lunev <den@openvz.org> | ||
14 | Message-Id: <20230424093147.197643-6-alexander.ivanov@virtuozzo.com> | ||
15 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
16 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> | ||
9 | --- | 17 | --- |
10 | block/nvme.c | 29 ++++++++++++++++------------- | 18 | block/parallels.c | 23 ++++++++++------------- |
11 | 1 file changed, 16 insertions(+), 13 deletions(-) | 19 | 1 file changed, 10 insertions(+), 13 deletions(-) |
12 | 20 | ||
13 | diff --git a/block/nvme.c b/block/nvme.c | 21 | diff --git a/block/parallels.c b/block/parallels.c |
14 | index XXXXXXX..XXXXXXX 100644 | 22 | index XXXXXXX..XXXXXXX 100644 |
15 | --- a/block/nvme.c | 23 | --- a/block/parallels.c |
16 | +++ b/block/nvme.c | 24 | +++ b/block/parallels.c |
17 | @@ -XXX,XX +XXX,XX @@ enum { | 25 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
18 | struct BDRVNVMeState { | 26 | { |
19 | AioContext *aio_context; | 27 | BDRVParallelsState *s = bs->opaque; |
20 | QEMUVFIOState *vfio; | 28 | int64_t size, prev_off, high_off; |
21 | - NVMeRegs *regs; | 29 | - int ret; |
22 | /* Memory mapped registers */ | 30 | + int ret = 0; |
23 | volatile struct { | 31 | uint32_t i; |
24 | uint32_t sq_tail; | 32 | - bool flush_bat = false; |
25 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | 33 | |
26 | uint64_t timeout_ms; | 34 | size = bdrv_getlength(bs->file->bs); |
27 | uint64_t deadline, now; | 35 | if (size < 0) { |
28 | Error *local_err = NULL; | 36 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
29 | + NVMeRegs *regs; | 37 | fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i); |
30 | 38 | res->corruptions++; | |
31 | qemu_co_mutex_init(&s->dma_map_lock); | 39 | if (fix & BDRV_FIX_ERRORS) { |
32 | qemu_co_queue_init(&s->dma_flush_queue); | 40 | - s->bat_bitmap[i] = 0; |
33 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | 41 | + parallels_set_bat_entry(s, i, 0); |
34 | goto out; | 42 | res->corruptions_fixed++; |
43 | - flush_bat = true; | ||
44 | } | ||
45 | prev_off = 0; | ||
46 | continue; | ||
47 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, | ||
48 | prev_off = off; | ||
35 | } | 49 | } |
36 | 50 | ||
37 | - s->regs = qemu_vfio_pci_map_bar(s->vfio, 0, 0, sizeof(NvmeBar), | 51 | - ret = 0; |
38 | - PROT_READ | PROT_WRITE, errp); | 52 | - if (flush_bat) { |
39 | - if (!s->regs) { | 53 | - ret = bdrv_co_pwrite_sync(bs->file, 0, s->header_size, s->header, 0); |
40 | + regs = qemu_vfio_pci_map_bar(s->vfio, 0, 0, sizeof(NvmeBar), | 54 | - if (ret < 0) { |
41 | + PROT_READ | PROT_WRITE, errp); | 55 | - res->check_errors++; |
42 | + if (!regs) { | 56 | - goto out; |
43 | ret = -EINVAL; | 57 | - } |
44 | goto out; | 58 | - } |
45 | } | 59 | - |
46 | /* Perform initialize sequence as described in NVMe spec "7.6.1 | 60 | if (high_off == 0) { |
47 | * Initialization". */ | 61 | res->image_end_offset = s->data_end << BDRV_SECTOR_BITS; |
48 | 62 | } else { | |
49 | - cap = le64_to_cpu(s->regs->ctrl.cap); | 63 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
50 | + cap = le64_to_cpu(regs->ctrl.cap); | 64 | |
51 | if (!(cap & (1ULL << 37))) { | ||
52 | error_setg(errp, "Device doesn't support NVMe command set"); | ||
53 | ret = -EINVAL; | ||
54 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | ||
55 | timeout_ms = MIN(500 * ((cap >> 24) & 0xFF), 30000); | ||
56 | |||
57 | /* Reset device to get a clean state. */ | ||
58 | - s->regs->ctrl.cc = cpu_to_le32(le32_to_cpu(s->regs->ctrl.cc) & 0xFE); | ||
59 | + regs->ctrl.cc = cpu_to_le32(le32_to_cpu(regs->ctrl.cc) & 0xFE); | ||
60 | /* Wait for CSTS.RDY = 0. */ | ||
61 | deadline = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + timeout_ms * SCALE_MS; | ||
62 | - while (le32_to_cpu(s->regs->ctrl.csts) & 0x1) { | ||
63 | + while (le32_to_cpu(regs->ctrl.csts) & 0x1) { | ||
64 | if (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) > deadline) { | ||
65 | error_setg(errp, "Timeout while waiting for device to reset (%" | ||
66 | PRId64 " ms)", | ||
67 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | ||
68 | } | ||
69 | s->nr_queues = 1; | ||
70 | QEMU_BUILD_BUG_ON(NVME_QUEUE_SIZE & 0xF000); | ||
71 | - s->regs->ctrl.aqa = cpu_to_le32((NVME_QUEUE_SIZE << 16) | NVME_QUEUE_SIZE); | ||
72 | - s->regs->ctrl.asq = cpu_to_le64(s->queues[INDEX_ADMIN]->sq.iova); | ||
73 | - s->regs->ctrl.acq = cpu_to_le64(s->queues[INDEX_ADMIN]->cq.iova); | ||
74 | + regs->ctrl.aqa = cpu_to_le32((NVME_QUEUE_SIZE << 16) | NVME_QUEUE_SIZE); | ||
75 | + regs->ctrl.asq = cpu_to_le64(s->queues[INDEX_ADMIN]->sq.iova); | ||
76 | + regs->ctrl.acq = cpu_to_le64(s->queues[INDEX_ADMIN]->cq.iova); | ||
77 | |||
78 | /* After setting up all control registers we can enable device now. */ | ||
79 | - s->regs->ctrl.cc = cpu_to_le32((ctz32(NVME_CQ_ENTRY_BYTES) << 20) | | ||
80 | + regs->ctrl.cc = cpu_to_le32((ctz32(NVME_CQ_ENTRY_BYTES) << 20) | | ||
81 | (ctz32(NVME_SQ_ENTRY_BYTES) << 16) | | ||
82 | 0x1); | ||
83 | /* Wait for CSTS.RDY = 1. */ | ||
84 | now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); | ||
85 | deadline = now + timeout_ms * 1000000; | ||
86 | - while (!(le32_to_cpu(s->regs->ctrl.csts) & 0x1)) { | ||
87 | + while (!(le32_to_cpu(regs->ctrl.csts) & 0x1)) { | ||
88 | if (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) > deadline) { | ||
89 | error_setg(errp, "Timeout while waiting for device to start (%" | ||
90 | PRId64 " ms)", | ||
91 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | ||
92 | ret = -EIO; | ||
93 | } | ||
94 | out: | 65 | out: |
95 | + if (regs) { | 66 | qemu_co_mutex_unlock(&s->lock); |
96 | + qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)regs, 0, sizeof(NvmeBar)); | 67 | + |
68 | + if (ret == 0) { | ||
69 | + ret = bdrv_co_flush(bs); | ||
70 | + if (ret < 0) { | ||
71 | + res->check_errors++; | ||
72 | + } | ||
97 | + } | 73 | + } |
98 | + | 74 | + |
99 | /* Cleaning up is done in nvme_file_open() upon error. */ | ||
100 | return ret; | 75 | return ret; |
101 | } | 76 | } |
102 | @@ -XXX,XX +XXX,XX @@ static void nvme_close(BlockDriverState *bs) | 77 | |
103 | event_notifier_cleanup(&s->irq_notifier[MSIX_SHARED_IRQ_IDX]); | ||
104 | qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->doorbells, | ||
105 | sizeof(NvmeBar), NVME_DOORBELL_SIZE); | ||
106 | - qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->regs, 0, sizeof(NvmeBar)); | ||
107 | qemu_vfio_close(s->vfio); | ||
108 | |||
109 | g_free(s->device); | ||
110 | -- | 78 | -- |
111 | 2.26.2 | 79 | 2.40.1 |
112 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | From: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
---|---|---|---|
2 | 2 | ||
3 | We are going to keep coroutine-wrappers code (structure-packing | 3 | We will add more and more checks so we need a better code structure |
4 | parameters, BDRV_POLL wrapper functions) in separate auto-generated | 4 | in parallels_co_check. Let each check performs in a separate loop |
5 | files. So, we'll need a header with declaration of original _co_ | 5 | in a separate helper. |
6 | functions, for those which are static now. As well, we'll need | ||
7 | declarations for wrapper functions. Do these declarations now, as a | ||
8 | preparation step. | ||
9 | 6 | ||
10 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 7 | Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
11 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 8 | Reviewed-by: Denis V. Lunev <den@openvz.org> |
12 | Reviewed-by: Eric Blake <eblake@redhat.com> | 9 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> |
13 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 10 | Message-Id: <20230424093147.197643-7-alexander.ivanov@virtuozzo.com> |
14 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | 11 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> |
15 | Message-Id: <20200924185414.28642-4-vsementsov@virtuozzo.com> | 12 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> |
16 | --- | 13 | --- |
17 | block/coroutines.h | 67 ++++++++++++++++++++++++++++++++++++++++++++++ | 14 | block/parallels.c | 31 +++++++++++++++++++++---------- |
18 | block.c | 8 +++--- | 15 | 1 file changed, 21 insertions(+), 10 deletions(-) |
19 | block/io.c | 34 +++++++++++------------ | ||
20 | 3 files changed, 88 insertions(+), 21 deletions(-) | ||
21 | create mode 100644 block/coroutines.h | ||
22 | 16 | ||
23 | diff --git a/block/coroutines.h b/block/coroutines.h | 17 | diff --git a/block/parallels.c b/block/parallels.c |
24 | new file mode 100644 | ||
25 | index XXXXXXX..XXXXXXX | ||
26 | --- /dev/null | ||
27 | +++ b/block/coroutines.h | ||
28 | @@ -XXX,XX +XXX,XX @@ | ||
29 | +/* | ||
30 | + * Block layer I/O functions | ||
31 | + * | ||
32 | + * Copyright (c) 2003 Fabrice Bellard | ||
33 | + * | ||
34 | + * Permission is hereby granted, free of charge, to any person obtaining a copy | ||
35 | + * of this software and associated documentation files (the "Software"), to deal | ||
36 | + * in the Software without restriction, including without limitation the rights | ||
37 | + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
38 | + * copies of the Software, and to permit persons to whom the Software is | ||
39 | + * furnished to do so, subject to the following conditions: | ||
40 | + * | ||
41 | + * The above copyright notice and this permission notice shall be included in | ||
42 | + * all copies or substantial portions of the Software. | ||
43 | + * | ||
44 | + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
45 | + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
46 | + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL | ||
47 | + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
48 | + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
49 | + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
50 | + * THE SOFTWARE. | ||
51 | + */ | ||
52 | + | ||
53 | +#ifndef BLOCK_COROUTINES_INT_H | ||
54 | +#define BLOCK_COROUTINES_INT_H | ||
55 | + | ||
56 | +#include "block/block_int.h" | ||
57 | + | ||
58 | +int coroutine_fn bdrv_co_check(BlockDriverState *bs, | ||
59 | + BdrvCheckResult *res, BdrvCheckMode fix); | ||
60 | +int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp); | ||
61 | + | ||
62 | +int coroutine_fn | ||
63 | +bdrv_co_prwv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov, | ||
64 | + bool is_write, BdrvRequestFlags flags); | ||
65 | +int | ||
66 | +bdrv_prwv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov, | ||
67 | + bool is_write, BdrvRequestFlags flags); | ||
68 | + | ||
69 | +int coroutine_fn | ||
70 | +bdrv_co_common_block_status_above(BlockDriverState *bs, | ||
71 | + BlockDriverState *base, | ||
72 | + bool want_zero, | ||
73 | + int64_t offset, | ||
74 | + int64_t bytes, | ||
75 | + int64_t *pnum, | ||
76 | + int64_t *map, | ||
77 | + BlockDriverState **file); | ||
78 | +int | ||
79 | +bdrv_common_block_status_above(BlockDriverState *bs, | ||
80 | + BlockDriverState *base, | ||
81 | + bool want_zero, | ||
82 | + int64_t offset, | ||
83 | + int64_t bytes, | ||
84 | + int64_t *pnum, | ||
85 | + int64_t *map, | ||
86 | + BlockDriverState **file); | ||
87 | + | ||
88 | +int coroutine_fn | ||
89 | +bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, | ||
90 | + bool is_read); | ||
91 | +int | ||
92 | +bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, | ||
93 | + bool is_read); | ||
94 | + | ||
95 | +#endif /* BLOCK_COROUTINES_INT_H */ | ||
96 | diff --git a/block.c b/block.c | ||
97 | index XXXXXXX..XXXXXXX 100644 | 18 | index XXXXXXX..XXXXXXX 100644 |
98 | --- a/block.c | 19 | --- a/block/parallels.c |
99 | +++ b/block.c | 20 | +++ b/block/parallels.c |
100 | @@ -XXX,XX +XXX,XX @@ | 21 | @@ -XXX,XX +XXX,XX @@ parallels_co_readv(BlockDriverState *bs, int64_t sector_num, int nb_sectors, |
101 | #include "qemu/timer.h" | ||
102 | #include "qemu/cutils.h" | ||
103 | #include "qemu/id.h" | ||
104 | +#include "block/coroutines.h" | ||
105 | |||
106 | #ifdef CONFIG_BSD | ||
107 | #include <sys/ioctl.h> | ||
108 | @@ -XXX,XX +XXX,XX @@ static void bdrv_delete(BlockDriverState *bs) | ||
109 | * free of errors) or -errno when an internal error occurred. The results of the | ||
110 | * check are stored in res. | ||
111 | */ | ||
112 | -static int coroutine_fn bdrv_co_check(BlockDriverState *bs, | ||
113 | - BdrvCheckResult *res, BdrvCheckMode fix) | ||
114 | +int coroutine_fn bdrv_co_check(BlockDriverState *bs, | ||
115 | + BdrvCheckResult *res, BdrvCheckMode fix) | ||
116 | { | ||
117 | if (bs->drv == NULL) { | ||
118 | return -ENOMEDIUM; | ||
119 | @@ -XXX,XX +XXX,XX @@ void bdrv_init_with_whitelist(void) | ||
120 | bdrv_init(); | ||
121 | } | ||
122 | |||
123 | -static int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, | ||
124 | - Error **errp) | ||
125 | +int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp) | ||
126 | { | ||
127 | BdrvChild *child, *parent; | ||
128 | uint64_t perm, shared_perm; | ||
129 | diff --git a/block/io.c b/block/io.c | ||
130 | index XXXXXXX..XXXXXXX 100644 | ||
131 | --- a/block/io.c | ||
132 | +++ b/block/io.c | ||
133 | @@ -XXX,XX +XXX,XX @@ | ||
134 | #include "block/blockjob.h" | ||
135 | #include "block/blockjob_int.h" | ||
136 | #include "block/block_int.h" | ||
137 | +#include "block/coroutines.h" | ||
138 | #include "qemu/cutils.h" | ||
139 | #include "qapi/error.h" | ||
140 | #include "qemu/error-report.h" | ||
141 | @@ -XXX,XX +XXX,XX @@ typedef struct RwCo { | ||
142 | BdrvRequestFlags flags; | ||
143 | } RwCo; | ||
144 | |||
145 | -static int coroutine_fn bdrv_co_prwv(BdrvChild *child, int64_t offset, | ||
146 | - QEMUIOVector *qiov, bool is_write, | ||
147 | - BdrvRequestFlags flags) | ||
148 | +int coroutine_fn bdrv_co_prwv(BdrvChild *child, int64_t offset, | ||
149 | + QEMUIOVector *qiov, bool is_write, | ||
150 | + BdrvRequestFlags flags) | ||
151 | { | ||
152 | if (is_write) { | ||
153 | return bdrv_co_pwritev(child, offset, qiov->size, qiov, flags); | ||
154 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_rw_co_entry(void *opaque) | ||
155 | /* | ||
156 | * Process a vectored synchronous request using coroutines | ||
157 | */ | ||
158 | -static int bdrv_prwv(BdrvChild *child, int64_t offset, | ||
159 | - QEMUIOVector *qiov, bool is_write, | ||
160 | - BdrvRequestFlags flags) | ||
161 | +int bdrv_prwv(BdrvChild *child, int64_t offset, | ||
162 | + QEMUIOVector *qiov, bool is_write, | ||
163 | + BdrvRequestFlags flags) | ||
164 | { | ||
165 | RwCo rwco = { | ||
166 | .child = child, | ||
167 | @@ -XXX,XX +XXX,XX @@ early_out: | ||
168 | return ret; | 22 | return ret; |
169 | } | 23 | } |
170 | 24 | ||
171 | -static int coroutine_fn | 25 | +static void parallels_check_unclean(BlockDriverState *bs, |
172 | +int coroutine_fn | 26 | + BdrvCheckResult *res, |
173 | bdrv_co_common_block_status_above(BlockDriverState *bs, | 27 | + BdrvCheckMode fix) |
174 | BlockDriverState *base, | 28 | +{ |
175 | bool want_zero, | 29 | + BDRVParallelsState *s = bs->opaque; |
176 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_block_status_above_co_entry(void *opaque) | 30 | + |
177 | * | 31 | + if (!s->header_unclean) { |
178 | * See bdrv_co_block_status_above() for details. | 32 | + return; |
179 | */ | 33 | + } |
180 | -static int bdrv_common_block_status_above(BlockDriverState *bs, | 34 | + |
181 | - BlockDriverState *base, | 35 | + fprintf(stderr, "%s image was not closed correctly\n", |
182 | - bool want_zero, int64_t offset, | 36 | + fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR"); |
183 | - int64_t bytes, int64_t *pnum, | 37 | + res->corruptions++; |
184 | - int64_t *map, | 38 | + if (fix & BDRV_FIX_ERRORS) { |
185 | - BlockDriverState **file) | 39 | + /* parallels_close will do the job right */ |
186 | +int bdrv_common_block_status_above(BlockDriverState *bs, | 40 | + res->corruptions_fixed++; |
187 | + BlockDriverState *base, | 41 | + s->header_unclean = false; |
188 | + bool want_zero, int64_t offset, | 42 | + } |
189 | + int64_t bytes, int64_t *pnum, | 43 | +} |
190 | + int64_t *map, | 44 | |
191 | + BlockDriverState **file) | 45 | static int coroutine_fn GRAPH_RDLOCK |
192 | { | 46 | parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
193 | BdrvCoBlockStatusData data = { | 47 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
194 | .bs = bs, | 48 | } |
195 | @@ -XXX,XX +XXX,XX @@ typedef struct BdrvVmstateCo { | 49 | |
196 | bool is_read; | 50 | qemu_co_mutex_lock(&s->lock); |
197 | } BdrvVmstateCo; | 51 | - if (s->header_unclean) { |
198 | 52 | - fprintf(stderr, "%s image was not closed correctly\n", | |
199 | -static int coroutine_fn | 53 | - fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR"); |
200 | +int coroutine_fn | 54 | - res->corruptions++; |
201 | bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, | 55 | - if (fix & BDRV_FIX_ERRORS) { |
202 | bool is_read) | 56 | - /* parallels_close will do the job right */ |
203 | { | 57 | - res->corruptions_fixed++; |
204 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_rw_vmstate_entry(void *opaque) | 58 | - s->header_unclean = false; |
205 | return bdrv_co_rw_vmstate(co->bs, co->qiov, co->pos, co->is_read); | 59 | - } |
206 | } | 60 | - } |
207 | 61 | + | |
208 | -static inline int | 62 | + parallels_check_unclean(bs, res, fix); |
209 | -bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, | 63 | |
210 | - bool is_read) | 64 | res->bfi.total_clusters = s->bat_size; |
211 | +int bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, | 65 | res->bfi.compressed_clusters = 0; /* compression is not supported */ |
212 | + bool is_read) | ||
213 | { | ||
214 | BdrvVmstateCo data = { | ||
215 | .bs = bs, | ||
216 | -- | 66 | -- |
217 | 2.26.2 | 67 | 2.40.1 |
218 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | From: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
---|---|---|---|
2 | 2 | ||
3 | Like for read/write in a previous commit, drop extra indirection layer, | 3 | We will add more and more checks so we need a better code structure in |
4 | generate directly bdrv_readv_vmstate() and bdrv_writev_vmstate(). | 4 | parallels_co_check. Let each check performs in a separate loop in a |
5 | separate helper. | ||
5 | 6 | ||
6 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 7 | Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
7 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 8 | Reviewed-by: Denis V. Lunev <den@openvz.org> |
8 | Reviewed-by: Eric Blake <eblake@redhat.com> | 9 | Message-Id: <20230424093147.197643-8-alexander.ivanov@virtuozzo.com> |
9 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | 10 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> |
10 | Message-Id: <20200924185414.28642-8-vsementsov@virtuozzo.com> | 11 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> |
11 | --- | 12 | --- |
12 | block/coroutines.h | 10 +++---- | 13 | block/parallels.c | 75 +++++++++++++++++++++++++++++++---------------- |
13 | include/block/block.h | 6 ++-- | 14 | 1 file changed, 49 insertions(+), 26 deletions(-) |
14 | block/io.c | 70 ++++++++++++++++++++++--------------------- | ||
15 | 3 files changed, 44 insertions(+), 42 deletions(-) | ||
16 | 15 | ||
17 | diff --git a/block/coroutines.h b/block/coroutines.h | 16 | diff --git a/block/parallels.c b/block/parallels.c |
18 | index XXXXXXX..XXXXXXX 100644 | 17 | index XXXXXXX..XXXXXXX 100644 |
19 | --- a/block/coroutines.h | 18 | --- a/block/parallels.c |
20 | +++ b/block/coroutines.h | 19 | +++ b/block/parallels.c |
21 | @@ -XXX,XX +XXX,XX @@ bdrv_common_block_status_above(BlockDriverState *bs, | 20 | @@ -XXX,XX +XXX,XX @@ static void parallels_check_unclean(BlockDriverState *bs, |
22 | int64_t *map, | 21 | } |
23 | BlockDriverState **file); | ||
24 | |||
25 | -int coroutine_fn | ||
26 | -bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, | ||
27 | - bool is_read); | ||
28 | -int generated_co_wrapper | ||
29 | -bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, | ||
30 | - bool is_read); | ||
31 | +int coroutine_fn bdrv_co_readv_vmstate(BlockDriverState *bs, | ||
32 | + QEMUIOVector *qiov, int64_t pos); | ||
33 | +int coroutine_fn bdrv_co_writev_vmstate(BlockDriverState *bs, | ||
34 | + QEMUIOVector *qiov, int64_t pos); | ||
35 | |||
36 | #endif /* BLOCK_COROUTINES_INT_H */ | ||
37 | diff --git a/include/block/block.h b/include/block/block.h | ||
38 | index XXXXXXX..XXXXXXX 100644 | ||
39 | --- a/include/block/block.h | ||
40 | +++ b/include/block/block.h | ||
41 | @@ -XXX,XX +XXX,XX @@ int path_has_protocol(const char *path); | ||
42 | int path_is_absolute(const char *path); | ||
43 | char *path_combine(const char *base_path, const char *filename); | ||
44 | |||
45 | -int bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos); | ||
46 | -int bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos); | ||
47 | +int generated_co_wrapper | ||
48 | +bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos); | ||
49 | +int generated_co_wrapper | ||
50 | +bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos); | ||
51 | int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf, | ||
52 | int64_t pos, int size); | ||
53 | |||
54 | diff --git a/block/io.c b/block/io.c | ||
55 | index XXXXXXX..XXXXXXX 100644 | ||
56 | --- a/block/io.c | ||
57 | +++ b/block/io.c | ||
58 | @@ -XXX,XX +XXX,XX @@ int bdrv_is_allocated_above(BlockDriverState *top, | ||
59 | } | 22 | } |
60 | 23 | ||
61 | int coroutine_fn | 24 | +static int coroutine_fn GRAPH_RDLOCK |
62 | -bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, | 25 | +parallels_check_outside_image(BlockDriverState *bs, BdrvCheckResult *res, |
63 | - bool is_read) | 26 | + BdrvCheckMode fix) |
64 | +bdrv_co_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos) | 27 | +{ |
65 | { | 28 | + BDRVParallelsState *s = bs->opaque; |
66 | BlockDriver *drv = bs->drv; | 29 | + uint32_t i; |
67 | BlockDriverState *child_bs = bdrv_primary_bs(bs); | 30 | + int64_t off, high_off, size; |
68 | int ret = -ENOTSUP; | 31 | + |
69 | 32 | + size = bdrv_getlength(bs->file->bs); | |
70 | + if (!drv) { | 33 | + if (size < 0) { |
71 | + return -ENOMEDIUM; | 34 | + res->check_errors++; |
35 | + return size; | ||
72 | + } | 36 | + } |
73 | + | 37 | + |
74 | bdrv_inc_in_flight(bs); | 38 | + high_off = 0; |
75 | 39 | + for (i = 0; i < s->bat_size; i++) { | |
76 | + if (drv->bdrv_load_vmstate) { | 40 | + off = bat2sect(s, i) << BDRV_SECTOR_BITS; |
77 | + ret = drv->bdrv_load_vmstate(bs, qiov, pos); | 41 | + if (off > size) { |
78 | + } else if (child_bs) { | 42 | + fprintf(stderr, "%s cluster %u is outside image\n", |
79 | + ret = bdrv_co_readv_vmstate(child_bs, qiov, pos); | 43 | + fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i); |
44 | + res->corruptions++; | ||
45 | + if (fix & BDRV_FIX_ERRORS) { | ||
46 | + parallels_set_bat_entry(s, i, 0); | ||
47 | + res->corruptions_fixed++; | ||
48 | + } | ||
49 | + continue; | ||
50 | + } | ||
51 | + if (high_off < off) { | ||
52 | + high_off = off; | ||
53 | + } | ||
80 | + } | 54 | + } |
81 | + | 55 | + |
82 | + bdrv_dec_in_flight(bs); | 56 | + if (high_off == 0) { |
57 | + res->image_end_offset = s->data_end << BDRV_SECTOR_BITS; | ||
58 | + } else { | ||
59 | + res->image_end_offset = high_off + s->cluster_size; | ||
60 | + s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS; | ||
61 | + } | ||
83 | + | 62 | + |
84 | + return ret; | 63 | + return 0; |
85 | +} | 64 | +} |
86 | + | 65 | + |
87 | +int coroutine_fn | 66 | static int coroutine_fn GRAPH_RDLOCK |
88 | +bdrv_co_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos) | 67 | parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
89 | +{ | 68 | BdrvCheckMode fix) |
90 | + BlockDriver *drv = bs->drv; | 69 | { |
91 | + BlockDriverState *child_bs = bdrv_primary_bs(bs); | 70 | BDRVParallelsState *s = bs->opaque; |
92 | + int ret = -ENOTSUP; | 71 | - int64_t size, prev_off, high_off; |
93 | + | 72 | - int ret = 0; |
94 | if (!drv) { | 73 | + int64_t size, prev_off; |
95 | - ret = -ENOMEDIUM; | 74 | + int ret; |
96 | - } else if (drv->bdrv_load_vmstate) { | 75 | uint32_t i; |
97 | - if (is_read) { | 76 | |
98 | - ret = drv->bdrv_load_vmstate(bs, qiov, pos); | 77 | size = bdrv_getlength(bs->file->bs); |
99 | - } else { | 78 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
100 | - ret = drv->bdrv_save_vmstate(bs, qiov, pos); | 79 | |
101 | - } | 80 | parallels_check_unclean(bs, res, fix); |
102 | + return -ENOMEDIUM; | 81 | |
82 | + ret = parallels_check_outside_image(bs, res, fix); | ||
83 | + if (ret < 0) { | ||
84 | + goto out; | ||
103 | + } | 85 | + } |
104 | + | 86 | + |
105 | + bdrv_inc_in_flight(bs); | 87 | res->bfi.total_clusters = s->bat_size; |
106 | + | 88 | res->bfi.compressed_clusters = 0; /* compression is not supported */ |
107 | + if (drv->bdrv_save_vmstate) { | 89 | |
108 | + ret = drv->bdrv_save_vmstate(bs, qiov, pos); | 90 | - high_off = 0; |
109 | } else if (child_bs) { | 91 | prev_off = 0; |
110 | - ret = bdrv_co_rw_vmstate(child_bs, qiov, pos, is_read); | 92 | for (i = 0; i < s->bat_size; i++) { |
111 | + ret = bdrv_co_writev_vmstate(child_bs, qiov, pos); | 93 | int64_t off = bat2sect(s, i) << BDRV_SECTOR_BITS; |
94 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, | ||
95 | continue; | ||
96 | } | ||
97 | |||
98 | - /* cluster outside the image */ | ||
99 | - if (off > size) { | ||
100 | - fprintf(stderr, "%s cluster %u is outside image\n", | ||
101 | - fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i); | ||
102 | - res->corruptions++; | ||
103 | - if (fix & BDRV_FIX_ERRORS) { | ||
104 | - parallels_set_bat_entry(s, i, 0); | ||
105 | - res->corruptions_fixed++; | ||
106 | - } | ||
107 | - prev_off = 0; | ||
108 | - continue; | ||
109 | - } | ||
110 | - | ||
111 | res->bfi.allocated_clusters++; | ||
112 | - if (off > high_off) { | ||
113 | - high_off = off; | ||
114 | - } | ||
115 | |||
116 | if (prev_off != 0 && (prev_off + s->cluster_size) != off) { | ||
117 | res->bfi.fragmented_clusters++; | ||
118 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, | ||
119 | prev_off = off; | ||
112 | } | 120 | } |
113 | 121 | ||
114 | bdrv_dec_in_flight(bs); | 122 | - if (high_off == 0) { |
115 | + | 123 | - res->image_end_offset = s->data_end << BDRV_SECTOR_BITS; |
116 | return ret; | 124 | - } else { |
117 | } | 125 | - res->image_end_offset = high_off + s->cluster_size; |
118 | 126 | - s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS; | |
119 | @@ -XXX,XX +XXX,XX @@ int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf, | ||
120 | int64_t pos, int size) | ||
121 | { | ||
122 | QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, size); | ||
123 | - int ret; | ||
124 | + int ret = bdrv_writev_vmstate(bs, &qiov, pos); | ||
125 | |||
126 | - ret = bdrv_writev_vmstate(bs, &qiov, pos); | ||
127 | - if (ret < 0) { | ||
128 | - return ret; | ||
129 | - } | 127 | - } |
130 | - | 128 | - |
131 | - return size; | 129 | if (size > res->image_end_offset) { |
132 | -} | 130 | int64_t count; |
133 | - | 131 | count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size); |
134 | -int bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos) | ||
135 | -{ | ||
136 | - return bdrv_rw_vmstate(bs, qiov, pos, false); | ||
137 | + return ret < 0 ? ret : size; | ||
138 | } | ||
139 | |||
140 | int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf, | ||
141 | int64_t pos, int size) | ||
142 | { | ||
143 | QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, size); | ||
144 | - int ret; | ||
145 | + int ret = bdrv_readv_vmstate(bs, &qiov, pos); | ||
146 | |||
147 | - ret = bdrv_readv_vmstate(bs, &qiov, pos); | ||
148 | - if (ret < 0) { | ||
149 | - return ret; | ||
150 | - } | ||
151 | - | ||
152 | - return size; | ||
153 | -} | ||
154 | - | ||
155 | -int bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos) | ||
156 | -{ | ||
157 | - return bdrv_rw_vmstate(bs, qiov, pos, true); | ||
158 | + return ret < 0 ? ret : size; | ||
159 | } | ||
160 | |||
161 | /**************************************************************/ | ||
162 | -- | 132 | -- |
163 | 2.26.2 | 133 | 2.40.1 |
164 | diff view generated by jsdifflib |
1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> | 1 | From: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
---|---|---|---|
2 | 2 | ||
3 | NVMeRegs only contains NvmeBar. Simplify the code by using NvmeBar | 3 | Exclude out-of-image clusters from allocated and fragmented clusters |
4 | directly. | 4 | calculation. |
5 | 5 | ||
6 | This triggers a checkpatch.pl error: | 6 | Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
7 | Message-Id: <20230424093147.197643-9-alexander.ivanov@virtuozzo.com> | ||
8 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
9 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> | ||
10 | --- | ||
11 | block/parallels.c | 6 +++++- | ||
12 | 1 file changed, 5 insertions(+), 1 deletion(-) | ||
7 | 13 | ||
8 | ERROR: Use of volatile is usually wrong, please add a comment | 14 | diff --git a/block/parallels.c b/block/parallels.c |
9 | #30: FILE: block/nvme.c:691: | ||
10 | + volatile NvmeBar *regs; | ||
11 | |||
12 | This is a false positive as in our case we are using I/O registers, | ||
13 | so the 'volatile' use is justified. | ||
14 | |||
15 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
17 | Message-Id: <20200922083821.578519-5-philmd@redhat.com> | ||
18 | --- | ||
19 | block/nvme.c | 23 +++++++++-------------- | ||
20 | 1 file changed, 9 insertions(+), 14 deletions(-) | ||
21 | |||
22 | diff --git a/block/nvme.c b/block/nvme.c | ||
23 | index XXXXXXX..XXXXXXX 100644 | 15 | index XXXXXXX..XXXXXXX 100644 |
24 | --- a/block/nvme.c | 16 | --- a/block/parallels.c |
25 | +++ b/block/nvme.c | 17 | +++ b/block/parallels.c |
26 | @@ -XXX,XX +XXX,XX @@ typedef struct { | 18 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
27 | QEMUBH *completion_bh; | 19 | prev_off = 0; |
28 | } NVMeQueuePair; | 20 | for (i = 0; i < s->bat_size; i++) { |
29 | 21 | int64_t off = bat2sect(s, i) << BDRV_SECTOR_BITS; | |
30 | -/* Memory mapped registers */ | 22 | - if (off == 0) { |
31 | -typedef volatile struct { | 23 | + /* |
32 | - NvmeBar ctrl; | 24 | + * If BDRV_FIX_ERRORS is not set, out-of-image BAT entries were not |
33 | -} NVMeRegs; | 25 | + * fixed. Skip not allocated and out-of-image BAT entries. |
34 | - | 26 | + */ |
35 | #define INDEX_ADMIN 0 | 27 | + if (off == 0 || off + s->cluster_size > res->image_end_offset) { |
36 | #define INDEX_IO(n) (1 + n) | 28 | prev_off = 0; |
37 | 29 | continue; | |
38 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | 30 | } |
39 | uint64_t timeout_ms; | ||
40 | uint64_t deadline, now; | ||
41 | Error *local_err = NULL; | ||
42 | - NVMeRegs *regs; | ||
43 | + volatile NvmeBar *regs = NULL; | ||
44 | |||
45 | qemu_co_mutex_init(&s->dma_map_lock); | ||
46 | qemu_co_queue_init(&s->dma_flush_queue); | ||
47 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | ||
48 | /* Perform initialize sequence as described in NVMe spec "7.6.1 | ||
49 | * Initialization". */ | ||
50 | |||
51 | - cap = le64_to_cpu(regs->ctrl.cap); | ||
52 | + cap = le64_to_cpu(regs->cap); | ||
53 | if (!(cap & (1ULL << 37))) { | ||
54 | error_setg(errp, "Device doesn't support NVMe command set"); | ||
55 | ret = -EINVAL; | ||
56 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | ||
57 | timeout_ms = MIN(500 * ((cap >> 24) & 0xFF), 30000); | ||
58 | |||
59 | /* Reset device to get a clean state. */ | ||
60 | - regs->ctrl.cc = cpu_to_le32(le32_to_cpu(regs->ctrl.cc) & 0xFE); | ||
61 | + regs->cc = cpu_to_le32(le32_to_cpu(regs->cc) & 0xFE); | ||
62 | /* Wait for CSTS.RDY = 0. */ | ||
63 | deadline = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + timeout_ms * SCALE_MS; | ||
64 | - while (le32_to_cpu(regs->ctrl.csts) & 0x1) { | ||
65 | + while (le32_to_cpu(regs->csts) & 0x1) { | ||
66 | if (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) > deadline) { | ||
67 | error_setg(errp, "Timeout while waiting for device to reset (%" | ||
68 | PRId64 " ms)", | ||
69 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | ||
70 | } | ||
71 | s->nr_queues = 1; | ||
72 | QEMU_BUILD_BUG_ON(NVME_QUEUE_SIZE & 0xF000); | ||
73 | - regs->ctrl.aqa = cpu_to_le32((NVME_QUEUE_SIZE << 16) | NVME_QUEUE_SIZE); | ||
74 | - regs->ctrl.asq = cpu_to_le64(s->queues[INDEX_ADMIN]->sq.iova); | ||
75 | - regs->ctrl.acq = cpu_to_le64(s->queues[INDEX_ADMIN]->cq.iova); | ||
76 | + regs->aqa = cpu_to_le32((NVME_QUEUE_SIZE << 16) | NVME_QUEUE_SIZE); | ||
77 | + regs->asq = cpu_to_le64(s->queues[INDEX_ADMIN]->sq.iova); | ||
78 | + regs->acq = cpu_to_le64(s->queues[INDEX_ADMIN]->cq.iova); | ||
79 | |||
80 | /* After setting up all control registers we can enable device now. */ | ||
81 | - regs->ctrl.cc = cpu_to_le32((ctz32(NVME_CQ_ENTRY_BYTES) << 20) | | ||
82 | + regs->cc = cpu_to_le32((ctz32(NVME_CQ_ENTRY_BYTES) << 20) | | ||
83 | (ctz32(NVME_SQ_ENTRY_BYTES) << 16) | | ||
84 | 0x1); | ||
85 | /* Wait for CSTS.RDY = 1. */ | ||
86 | now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); | ||
87 | deadline = now + timeout_ms * 1000000; | ||
88 | - while (!(le32_to_cpu(regs->ctrl.csts) & 0x1)) { | ||
89 | + while (!(le32_to_cpu(regs->csts) & 0x1)) { | ||
90 | if (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) > deadline) { | ||
91 | error_setg(errp, "Timeout while waiting for device to start (%" | ||
92 | PRId64 " ms)", | ||
93 | -- | 31 | -- |
94 | 2.26.2 | 32 | 2.40.1 |
95 | diff view generated by jsdifflib |
1 | From: Eric Auger <eric.auger@redhat.com> | 1 | From: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
---|---|---|---|
2 | 2 | ||
3 | Introduce the qemu_vfio_find_fixed/temp_iova helpers which | 3 | We will add more and more checks so we need a better code structure |
4 | respectively allocate IOVAs from the bottom/top parts of the | 4 | in parallels_co_check. Let each check performs in a separate loop |
5 | usable IOVA range, without picking within host IOVA reserved | 5 | in a separate helper. |
6 | windows. The allocation remains basic: if the size is too big | ||
7 | for the remaining of the current usable IOVA range, we jump | ||
8 | to the next one, leaving a hole in the address map. | ||
9 | 6 | ||
10 | Signed-off-by: Eric Auger <eric.auger@redhat.com> | 7 | Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
11 | Message-id: 20200929085550.30926-3-eric.auger@redhat.com | 8 | Message-Id: <20230424093147.197643-10-alexander.ivanov@virtuozzo.com> |
12 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 9 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> |
10 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> | ||
13 | --- | 11 | --- |
14 | util/vfio-helpers.c | 57 +++++++++++++++++++++++++++++++++++++++++---- | 12 | block/parallels.c | 74 ++++++++++++++++++++++++++++------------------- |
15 | 1 file changed, 53 insertions(+), 4 deletions(-) | 13 | 1 file changed, 45 insertions(+), 29 deletions(-) |
16 | 14 | ||
17 | diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c | 15 | diff --git a/block/parallels.c b/block/parallels.c |
18 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100644 |
19 | --- a/util/vfio-helpers.c | 17 | --- a/block/parallels.c |
20 | +++ b/util/vfio-helpers.c | 18 | +++ b/block/parallels.c |
21 | @@ -XXX,XX +XXX,XX @@ static bool qemu_vfio_verify_mappings(QEMUVFIOState *s) | 19 | @@ -XXX,XX +XXX,XX @@ parallels_check_outside_image(BlockDriverState *bs, BdrvCheckResult *res, |
22 | return true; | ||
23 | } | 20 | } |
24 | 21 | ||
25 | +static int | 22 | static int coroutine_fn GRAPH_RDLOCK |
26 | +qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova) | 23 | -parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
27 | +{ | 24 | - BdrvCheckMode fix) |
28 | + int i; | 25 | +parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res, |
26 | + BdrvCheckMode fix) | ||
27 | { | ||
28 | BDRVParallelsState *s = bs->opaque; | ||
29 | - int64_t size, prev_off; | ||
30 | + int64_t size; | ||
31 | int ret; | ||
32 | - uint32_t i; | ||
33 | |||
34 | size = bdrv_getlength(bs->file->bs); | ||
35 | if (size < 0) { | ||
36 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, | ||
37 | return size; | ||
38 | } | ||
39 | |||
40 | + if (size > res->image_end_offset) { | ||
41 | + int64_t count; | ||
42 | + count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size); | ||
43 | + fprintf(stderr, "%s space leaked at the end of the image %" PRId64 "\n", | ||
44 | + fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR", | ||
45 | + size - res->image_end_offset); | ||
46 | + res->leaks += count; | ||
47 | + if (fix & BDRV_FIX_LEAKS) { | ||
48 | + Error *local_err = NULL; | ||
29 | + | 49 | + |
30 | + for (i = 0; i < s->nb_iova_ranges; i++) { | 50 | + /* |
31 | + if (s->usable_iova_ranges[i].end < s->low_water_mark) { | 51 | + * In order to really repair the image, we must shrink it. |
32 | + continue; | 52 | + * That means we have to pass exact=true. |
33 | + } | 53 | + */ |
34 | + s->low_water_mark = | 54 | + ret = bdrv_co_truncate(bs->file, res->image_end_offset, true, |
35 | + MAX(s->low_water_mark, s->usable_iova_ranges[i].start); | 55 | + PREALLOC_MODE_OFF, 0, &local_err); |
36 | + | 56 | + if (ret < 0) { |
37 | + if (s->usable_iova_ranges[i].end - s->low_water_mark + 1 >= size || | 57 | + error_report_err(local_err); |
38 | + s->usable_iova_ranges[i].end - s->low_water_mark + 1 == 0) { | 58 | + res->check_errors++; |
39 | + *iova = s->low_water_mark; | 59 | + return ret; |
40 | + s->low_water_mark += size; | 60 | + } |
41 | + return 0; | 61 | + res->leaks_fixed += count; |
42 | + } | 62 | + } |
43 | + } | 63 | + } |
44 | + return -ENOMEM; | 64 | + |
65 | + return 0; | ||
45 | +} | 66 | +} |
46 | + | 67 | + |
47 | +static int | 68 | +static int coroutine_fn GRAPH_RDLOCK |
48 | +qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova) | 69 | +parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
70 | + BdrvCheckMode fix) | ||
49 | +{ | 71 | +{ |
50 | + int i; | 72 | + BDRVParallelsState *s = bs->opaque; |
73 | + int64_t prev_off; | ||
74 | + int ret; | ||
75 | + uint32_t i; | ||
51 | + | 76 | + |
52 | + for (i = s->nb_iova_ranges - 1; i >= 0; i--) { | 77 | qemu_co_mutex_lock(&s->lock); |
53 | + if (s->usable_iova_ranges[i].start > s->high_water_mark) { | 78 | |
54 | + continue; | 79 | parallels_check_unclean(bs, res, fix); |
55 | + } | 80 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
56 | + s->high_water_mark = | 81 | goto out; |
57 | + MIN(s->high_water_mark, s->usable_iova_ranges[i].end + 1); | 82 | } |
83 | |||
84 | + ret = parallels_check_leak(bs, res, fix); | ||
85 | + if (ret < 0) { | ||
86 | + goto out; | ||
87 | + } | ||
58 | + | 88 | + |
59 | + if (s->high_water_mark - s->usable_iova_ranges[i].start + 1 >= size || | 89 | res->bfi.total_clusters = s->bat_size; |
60 | + s->high_water_mark - s->usable_iova_ranges[i].start + 1 == 0) { | 90 | res->bfi.compressed_clusters = 0; /* compression is not supported */ |
61 | + *iova = s->high_water_mark - size; | 91 | |
62 | + s->high_water_mark = *iova; | 92 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
63 | + return 0; | 93 | prev_off = off; |
64 | + } | ||
65 | + } | ||
66 | + return -ENOMEM; | ||
67 | +} | ||
68 | + | ||
69 | /* Map [host, host + size) area into a contiguous IOVA address space, and store | ||
70 | * the result in @iova if not NULL. The caller need to make sure the area is | ||
71 | * aligned to page size, and mustn't overlap with existing mapping areas (split | ||
72 | @@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size, | ||
73 | goto out; | ||
74 | } | ||
75 | if (!temporary) { | ||
76 | - iova0 = s->low_water_mark; | ||
77 | + if (qemu_vfio_find_fixed_iova(s, size, &iova0)) { | ||
78 | + ret = -ENOMEM; | ||
79 | + goto out; | ||
80 | + } | ||
81 | + | ||
82 | mapping = qemu_vfio_add_mapping(s, host, size, index + 1, iova0); | ||
83 | if (!mapping) { | ||
84 | ret = -ENOMEM; | ||
85 | @@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size, | ||
86 | qemu_vfio_undo_mapping(s, mapping, NULL); | ||
87 | goto out; | ||
88 | } | ||
89 | - s->low_water_mark += size; | ||
90 | qemu_vfio_dump_mappings(s); | ||
91 | } else { | ||
92 | - iova0 = s->high_water_mark - size; | ||
93 | + if (qemu_vfio_find_temp_iova(s, size, &iova0)) { | ||
94 | + ret = -ENOMEM; | ||
95 | + goto out; | ||
96 | + } | ||
97 | ret = qemu_vfio_do_mapping(s, host, size, iova0); | ||
98 | if (ret) { | ||
99 | goto out; | ||
100 | } | ||
101 | - s->high_water_mark -= size; | ||
102 | } | ||
103 | } | 94 | } |
104 | if (iova) { | 95 | |
96 | - if (size > res->image_end_offset) { | ||
97 | - int64_t count; | ||
98 | - count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size); | ||
99 | - fprintf(stderr, "%s space leaked at the end of the image %" PRId64 "\n", | ||
100 | - fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR", | ||
101 | - size - res->image_end_offset); | ||
102 | - res->leaks += count; | ||
103 | - if (fix & BDRV_FIX_LEAKS) { | ||
104 | - Error *local_err = NULL; | ||
105 | - | ||
106 | - /* | ||
107 | - * In order to really repair the image, we must shrink it. | ||
108 | - * That means we have to pass exact=true. | ||
109 | - */ | ||
110 | - ret = bdrv_co_truncate(bs->file, res->image_end_offset, true, | ||
111 | - PREALLOC_MODE_OFF, 0, &local_err); | ||
112 | - if (ret < 0) { | ||
113 | - error_report_err(local_err); | ||
114 | - res->check_errors++; | ||
115 | - goto out; | ||
116 | - } | ||
117 | - res->leaks_fixed += count; | ||
118 | - } | ||
119 | - } | ||
120 | - | ||
121 | out: | ||
122 | qemu_co_mutex_unlock(&s->lock); | ||
123 | |||
105 | -- | 124 | -- |
106 | 2.26.2 | 125 | 2.40.1 |
107 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | From: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
---|---|---|---|
2 | 2 | ||
3 | Now that we are not maintaining boilerplate code for coroutine | 3 | We will add more and more checks so we need a better code structure |
4 | wrappers, there is no more sense in keeping the extra indirection layer | 4 | in parallels_co_check. Let each check performs in a separate loop |
5 | of bdrv_prwv(). Let's drop it and instead generate pure bdrv_preadv() | 5 | in a separate helper. |
6 | and bdrv_pwritev(). | ||
7 | 6 | ||
8 | Currently, bdrv_pwritev() and bdrv_preadv() are returning bytes on | 7 | Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
9 | success, auto generated functions will instead return zero, as their | 8 | Reviewed-by: Denis V. Lunev <den@openvz.org> |
10 | _co_ prototype. Still, it's simple to make the conversion safe: the | 9 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> |
11 | only external user of bdrv_pwritev() is test-bdrv-drain, and it is | 10 | Message-Id: <20230424093147.197643-11-alexander.ivanov@virtuozzo.com> |
12 | comfortable enough with bdrv_co_pwritev() instead. So prototypes are | 11 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> |
13 | moved to local block/coroutines.h. Next, the only internal use is | 12 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> |
14 | bdrv_pread() and bdrv_pwrite(), which are modified to return bytes on | 13 | --- |
15 | success. | 14 | block/parallels.c | 52 +++++++++++++++++++++++++++-------------------- |
15 | 1 file changed, 30 insertions(+), 22 deletions(-) | ||
16 | 16 | ||
17 | Of course, it would be great to convert bdrv_pread() and bdrv_pwrite() | 17 | diff --git a/block/parallels.c b/block/parallels.c |
18 | to return 0 on success. But this requires audit (and probably | ||
19 | conversion) of all their users, let's leave it for another day | ||
20 | refactoring. | ||
21 | |||
22 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
23 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
24 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
25 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
26 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
27 | Message-Id: <20200924185414.28642-7-vsementsov@virtuozzo.com> | ||
28 | --- | ||
29 | block/coroutines.h | 10 ++++----- | ||
30 | include/block/block.h | 2 -- | ||
31 | block/io.c | 49 ++++++++--------------------------------- | ||
32 | tests/test-bdrv-drain.c | 2 +- | ||
33 | 4 files changed, 15 insertions(+), 48 deletions(-) | ||
34 | |||
35 | diff --git a/block/coroutines.h b/block/coroutines.h | ||
36 | index XXXXXXX..XXXXXXX 100644 | 18 | index XXXXXXX..XXXXXXX 100644 |
37 | --- a/block/coroutines.h | 19 | --- a/block/parallels.c |
38 | +++ b/block/coroutines.h | 20 | +++ b/block/parallels.c |
39 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_check(BlockDriverState *bs, | 21 | @@ -XXX,XX +XXX,XX @@ parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res, |
40 | BdrvCheckResult *res, BdrvCheckMode fix); | ||
41 | int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp); | ||
42 | |||
43 | -int coroutine_fn | ||
44 | -bdrv_co_prwv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov, | ||
45 | - bool is_write, BdrvRequestFlags flags); | ||
46 | int generated_co_wrapper | ||
47 | -bdrv_prwv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov, | ||
48 | - bool is_write, BdrvRequestFlags flags); | ||
49 | +bdrv_preadv(BdrvChild *child, int64_t offset, unsigned int bytes, | ||
50 | + QEMUIOVector *qiov, BdrvRequestFlags flags); | ||
51 | +int generated_co_wrapper | ||
52 | +bdrv_pwritev(BdrvChild *child, int64_t offset, unsigned int bytes, | ||
53 | + QEMUIOVector *qiov, BdrvRequestFlags flags); | ||
54 | |||
55 | int coroutine_fn | ||
56 | bdrv_co_common_block_status_above(BlockDriverState *bs, | ||
57 | diff --git a/include/block/block.h b/include/block/block.h | ||
58 | index XXXXXXX..XXXXXXX 100644 | ||
59 | --- a/include/block/block.h | ||
60 | +++ b/include/block/block.h | ||
61 | @@ -XXX,XX +XXX,XX @@ int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset, | ||
62 | int bytes, BdrvRequestFlags flags); | ||
63 | int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags); | ||
64 | int bdrv_pread(BdrvChild *child, int64_t offset, void *buf, int bytes); | ||
65 | -int bdrv_preadv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov); | ||
66 | int bdrv_pwrite(BdrvChild *child, int64_t offset, const void *buf, int bytes); | ||
67 | -int bdrv_pwritev(BdrvChild *child, int64_t offset, QEMUIOVector *qiov); | ||
68 | int bdrv_pwrite_sync(BdrvChild *child, int64_t offset, | ||
69 | const void *buf, int count); | ||
70 | /* | ||
71 | diff --git a/block/io.c b/block/io.c | ||
72 | index XXXXXXX..XXXXXXX 100644 | ||
73 | --- a/block/io.c | ||
74 | +++ b/block/io.c | ||
75 | @@ -XXX,XX +XXX,XX @@ static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset, | ||
76 | return 0; | 22 | return 0; |
77 | } | 23 | } |
78 | 24 | ||
79 | -int coroutine_fn bdrv_co_prwv(BdrvChild *child, int64_t offset, | 25 | -static int coroutine_fn GRAPH_RDLOCK |
80 | - QEMUIOVector *qiov, bool is_write, | 26 | -parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
81 | - BdrvRequestFlags flags) | 27 | - BdrvCheckMode fix) |
82 | -{ | 28 | +static void parallels_collect_statistics(BlockDriverState *bs, |
83 | - if (is_write) { | 29 | + BdrvCheckResult *res, |
84 | - return bdrv_co_pwritev(child, offset, qiov->size, qiov, flags); | 30 | + BdrvCheckMode fix) |
85 | - } else { | 31 | { |
86 | - return bdrv_co_preadv(child, offset, qiov->size, qiov, flags); | 32 | BDRVParallelsState *s = bs->opaque; |
87 | - } | 33 | - int64_t prev_off; |
88 | -} | 34 | - int ret; |
35 | + int64_t off, prev_off; | ||
36 | uint32_t i; | ||
37 | |||
38 | - qemu_co_mutex_lock(&s->lock); | ||
89 | - | 39 | - |
90 | int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset, | 40 | - parallels_check_unclean(bs, res, fix); |
91 | int bytes, BdrvRequestFlags flags) | ||
92 | { | ||
93 | - QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, bytes); | ||
94 | - | 41 | - |
95 | - return bdrv_prwv(child, offset, &qiov, true, BDRV_REQ_ZERO_WRITE | flags); | 42 | - ret = parallels_check_outside_image(bs, res, fix); |
96 | + return bdrv_pwritev(child, offset, bytes, NULL, | ||
97 | + BDRV_REQ_ZERO_WRITE | flags); | ||
98 | } | ||
99 | |||
100 | /* | ||
101 | @@ -XXX,XX +XXX,XX @@ int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags) | ||
102 | } | ||
103 | } | ||
104 | |||
105 | -/* return < 0 if error. See bdrv_pwrite() for the return codes */ | ||
106 | -int bdrv_preadv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov) | ||
107 | -{ | ||
108 | - int ret; | ||
109 | - | ||
110 | - ret = bdrv_prwv(child, offset, qiov, false, 0); | ||
111 | - if (ret < 0) { | 43 | - if (ret < 0) { |
112 | - return ret; | 44 | - goto out; |
113 | - } | 45 | - } |
114 | - | 46 | - |
115 | - return qiov->size; | 47 | - ret = parallels_check_leak(bs, res, fix); |
116 | -} | ||
117 | - | ||
118 | /* See bdrv_pwrite() for the return codes */ | ||
119 | int bdrv_pread(BdrvChild *child, int64_t offset, void *buf, int bytes) | ||
120 | { | ||
121 | + int ret; | ||
122 | QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes); | ||
123 | |||
124 | if (bytes < 0) { | ||
125 | return -EINVAL; | ||
126 | } | ||
127 | |||
128 | - return bdrv_preadv(child, offset, &qiov); | ||
129 | -} | ||
130 | + ret = bdrv_preadv(child, offset, bytes, &qiov, 0); | ||
131 | |||
132 | -int bdrv_pwritev(BdrvChild *child, int64_t offset, QEMUIOVector *qiov) | ||
133 | -{ | ||
134 | - int ret; | ||
135 | - | ||
136 | - ret = bdrv_prwv(child, offset, qiov, true, 0); | ||
137 | - if (ret < 0) { | 48 | - if (ret < 0) { |
138 | - return ret; | 49 | - goto out; |
139 | - } | 50 | - } |
140 | - | 51 | - |
141 | - return qiov->size; | 52 | res->bfi.total_clusters = s->bat_size; |
142 | + return ret < 0 ? ret : bytes; | 53 | res->bfi.compressed_clusters = 0; /* compression is not supported */ |
143 | } | 54 | |
144 | 55 | prev_off = 0; | |
145 | /* Return no. of bytes on success or < 0 on error. Important errors are: | 56 | for (i = 0; i < s->bat_size; i++) { |
146 | @@ -XXX,XX +XXX,XX @@ int bdrv_pwritev(BdrvChild *child, int64_t offset, QEMUIOVector *qiov) | 57 | - int64_t off = bat2sect(s, i) << BDRV_SECTOR_BITS; |
147 | */ | 58 | + off = bat2sect(s, i) << BDRV_SECTOR_BITS; |
148 | int bdrv_pwrite(BdrvChild *child, int64_t offset, const void *buf, int bytes) | 59 | /* |
149 | { | 60 | * If BDRV_FIX_ERRORS is not set, out-of-image BAT entries were not |
61 | * fixed. Skip not allocated and out-of-image BAT entries. | ||
62 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, | ||
63 | continue; | ||
64 | } | ||
65 | |||
66 | - res->bfi.allocated_clusters++; | ||
67 | - | ||
68 | if (prev_off != 0 && (prev_off + s->cluster_size) != off) { | ||
69 | res->bfi.fragmented_clusters++; | ||
70 | } | ||
71 | prev_off = off; | ||
72 | + res->bfi.allocated_clusters++; | ||
73 | } | ||
74 | +} | ||
75 | + | ||
76 | +static int coroutine_fn GRAPH_RDLOCK | ||
77 | +parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, | ||
78 | + BdrvCheckMode fix) | ||
79 | +{ | ||
80 | + BDRVParallelsState *s = bs->opaque; | ||
150 | + int ret; | 81 | + int ret; |
151 | QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes); | ||
152 | |||
153 | if (bytes < 0) { | ||
154 | return -EINVAL; | ||
155 | } | ||
156 | |||
157 | - return bdrv_pwritev(child, offset, &qiov); | ||
158 | + ret = bdrv_pwritev(child, offset, bytes, &qiov, 0); | ||
159 | + | 82 | + |
160 | + return ret < 0 ? ret : bytes; | 83 | + qemu_co_mutex_lock(&s->lock); |
161 | } | 84 | + |
162 | 85 | + parallels_check_unclean(bs, res, fix); | |
163 | /* | 86 | + |
164 | diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c | 87 | + ret = parallels_check_outside_image(bs, res, fix); |
165 | index XXXXXXX..XXXXXXX 100644 | 88 | + if (ret < 0) { |
166 | --- a/tests/test-bdrv-drain.c | 89 | + goto out; |
167 | +++ b/tests/test-bdrv-drain.c | 90 | + } |
168 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_replace_test_co_preadv(BlockDriverState *bs, | 91 | + |
169 | } | 92 | + ret = parallels_check_leak(bs, res, fix); |
170 | s->io_co = NULL; | 93 | + if (ret < 0) { |
171 | 94 | + goto out; | |
172 | - ret = bdrv_preadv(bs->backing, offset, qiov); | 95 | + } |
173 | + ret = bdrv_co_preadv(bs->backing, offset, bytes, qiov, 0); | 96 | + |
174 | s->has_read = true; | 97 | + parallels_collect_statistics(bs, res, fix); |
175 | 98 | ||
176 | /* Wake up drain_co if it runs */ | 99 | out: |
100 | qemu_co_mutex_unlock(&s->lock); | ||
177 | -- | 101 | -- |
178 | 2.26.2 | 102 | 2.40.1 |
179 | diff view generated by jsdifflib |
1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> | 1 | From: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
---|---|---|---|
2 | 2 | ||
3 | Pages are currently mapped READ/WRITE. To be able to use different | 3 | Replace the way we use mutex in parallels_co_check() for simplier |
4 | protections, add a new argument to qemu_vfio_pci_map_bar(). | 4 | and less error prone code. |
5 | 5 | ||
6 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 6 | Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
7 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 7 | Reviewed-by: Denis V. Lunev <den@openvz.org> |
8 | Message-Id: <20200922083821.578519-2-philmd@redhat.com> | 8 | Message-Id: <20230424093147.197643-12-alexander.ivanov@virtuozzo.com> |
9 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
10 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> | ||
9 | --- | 11 | --- |
10 | include/qemu/vfio-helpers.h | 2 +- | 12 | block/parallels.c | 33 ++++++++++++++------------------- |
11 | block/nvme.c | 3 ++- | 13 | 1 file changed, 14 insertions(+), 19 deletions(-) |
12 | util/vfio-helpers.c | 4 ++-- | ||
13 | 3 files changed, 5 insertions(+), 4 deletions(-) | ||
14 | 14 | ||
15 | diff --git a/include/qemu/vfio-helpers.h b/include/qemu/vfio-helpers.h | 15 | diff --git a/block/parallels.c b/block/parallels.c |
16 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100644 |
17 | --- a/include/qemu/vfio-helpers.h | 17 | --- a/block/parallels.c |
18 | +++ b/include/qemu/vfio-helpers.h | 18 | +++ b/block/parallels.c |
19 | @@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size, | 19 | @@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res, |
20 | int qemu_vfio_dma_reset_temporary(QEMUVFIOState *s); | 20 | BDRVParallelsState *s = bs->opaque; |
21 | void qemu_vfio_dma_unmap(QEMUVFIOState *s, void *host); | 21 | int ret; |
22 | void *qemu_vfio_pci_map_bar(QEMUVFIOState *s, int index, | 22 | |
23 | - uint64_t offset, uint64_t size, | 23 | - qemu_co_mutex_lock(&s->lock); |
24 | + uint64_t offset, uint64_t size, int prot, | 24 | + WITH_QEMU_LOCK_GUARD(&s->lock) { |
25 | Error **errp); | 25 | + parallels_check_unclean(bs, res, fix); |
26 | void qemu_vfio_pci_unmap_bar(QEMUVFIOState *s, int index, void *bar, | 26 | |
27 | uint64_t offset, uint64_t size); | 27 | - parallels_check_unclean(bs, res, fix); |
28 | diff --git a/block/nvme.c b/block/nvme.c | 28 | + ret = parallels_check_outside_image(bs, res, fix); |
29 | index XXXXXXX..XXXXXXX 100644 | 29 | + if (ret < 0) { |
30 | --- a/block/nvme.c | 30 | + return ret; |
31 | +++ b/block/nvme.c | 31 | + } |
32 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | 32 | |
33 | goto out; | 33 | - ret = parallels_check_outside_image(bs, res, fix); |
34 | - if (ret < 0) { | ||
35 | - goto out; | ||
36 | - } | ||
37 | + ret = parallels_check_leak(bs, res, fix); | ||
38 | + if (ret < 0) { | ||
39 | + return ret; | ||
40 | + } | ||
41 | |||
42 | - ret = parallels_check_leak(bs, res, fix); | ||
43 | - if (ret < 0) { | ||
44 | - goto out; | ||
45 | + parallels_collect_statistics(bs, res, fix); | ||
34 | } | 46 | } |
35 | 47 | ||
36 | - s->regs = qemu_vfio_pci_map_bar(s->vfio, 0, 0, NVME_BAR_SIZE, errp); | 48 | - parallels_collect_statistics(bs, res, fix); |
37 | + s->regs = qemu_vfio_pci_map_bar(s->vfio, 0, 0, NVME_BAR_SIZE, | 49 | - |
38 | + PROT_READ | PROT_WRITE, errp); | 50 | -out: |
39 | if (!s->regs) { | 51 | - qemu_co_mutex_unlock(&s->lock); |
40 | ret = -EINVAL; | 52 | - |
41 | goto out; | 53 | - if (ret == 0) { |
42 | diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c | 54 | - ret = bdrv_co_flush(bs); |
43 | index XXXXXXX..XXXXXXX 100644 | 55 | - if (ret < 0) { |
44 | --- a/util/vfio-helpers.c | 56 | - res->check_errors++; |
45 | +++ b/util/vfio-helpers.c | 57 | - } |
46 | @@ -XXX,XX +XXX,XX @@ static int qemu_vfio_pci_init_bar(QEMUVFIOState *s, int index, Error **errp) | 58 | + ret = bdrv_co_flush(bs); |
47 | * Map a PCI bar area. | 59 | + if (ret < 0) { |
48 | */ | 60 | + res->check_errors++; |
49 | void *qemu_vfio_pci_map_bar(QEMUVFIOState *s, int index, | 61 | } |
50 | - uint64_t offset, uint64_t size, | 62 | |
51 | + uint64_t offset, uint64_t size, int prot, | 63 | return ret; |
52 | Error **errp) | ||
53 | { | ||
54 | void *p; | ||
55 | assert_bar_index_valid(s, index); | ||
56 | p = mmap(NULL, MIN(size, s->bar_region_info[index].size - offset), | ||
57 | - PROT_READ | PROT_WRITE, MAP_SHARED, | ||
58 | + prot, MAP_SHARED, | ||
59 | s->device, s->bar_region_info[index].offset + offset); | ||
60 | if (p == MAP_FAILED) { | ||
61 | error_setg_errno(errp, errno, "Failed to map BAR region"); | ||
62 | -- | 64 | -- |
63 | 2.26.2 | 65 | 2.40.1 |
64 | diff view generated by jsdifflib |
1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> | 1 | From: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
---|---|---|---|
2 | 2 | ||
3 | Use self-explicit SCALE_MS definition instead of magic value | 3 | All the offsets in the BAT must be lower than the file size. |
4 | (missed in similar commit e4f310fe7f5). | 4 | Fix the check condition for correct check. |
5 | 5 | ||
6 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 6 | Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> |
7 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 7 | Reviewed-by: Denis V. Lunev <den@openvz.org> |
8 | Message-Id: <20200922083821.578519-7-philmd@redhat.com> | 8 | Message-Id: <20230424093147.197643-13-alexander.ivanov@virtuozzo.com> |
9 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
10 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> | ||
9 | --- | 11 | --- |
10 | block/nvme.c | 2 +- | 12 | block/parallels.c | 2 +- |
11 | 1 file changed, 1 insertion(+), 1 deletion(-) | 13 | 1 file changed, 1 insertion(+), 1 deletion(-) |
12 | 14 | ||
13 | diff --git a/block/nvme.c b/block/nvme.c | 15 | diff --git a/block/parallels.c b/block/parallels.c |
14 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100644 |
15 | --- a/block/nvme.c | 17 | --- a/block/parallels.c |
16 | +++ b/block/nvme.c | 18 | +++ b/block/parallels.c |
17 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, | 19 | @@ -XXX,XX +XXX,XX @@ parallels_check_outside_image(BlockDriverState *bs, BdrvCheckResult *res, |
18 | CC_EN_MASK); | 20 | high_off = 0; |
19 | /* Wait for CSTS.RDY = 1. */ | 21 | for (i = 0; i < s->bat_size; i++) { |
20 | now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); | 22 | off = bat2sect(s, i) << BDRV_SECTOR_BITS; |
21 | - deadline = now + timeout_ms * 1000000; | 23 | - if (off > size) { |
22 | + deadline = now + timeout_ms * SCALE_MS; | 24 | + if (off + s->cluster_size > size) { |
23 | while (!NVME_CSTS_RDY(le32_to_cpu(regs->csts))) { | 25 | fprintf(stderr, "%s cluster %u is outside image\n", |
24 | if (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) > deadline) { | 26 | fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i); |
25 | error_setg(errp, "Timeout while waiting for device to start (%" | 27 | res->corruptions++; |
26 | -- | 28 | -- |
27 | 2.26.2 | 29 | 2.40.1 |
28 | diff view generated by jsdifflib |
1 | From: Stefano Garzarella <sgarzare@redhat.com> | 1 | From: Jean-Louis Dupond <jean-louis@dupond.be> |
---|---|---|---|
2 | 2 | ||
3 | When we added io_uring AIO engine, we forgot to update qemu-options.hx, | 3 | When we for example have a sparse qcow2 image and discard: unmap is enabled, |
4 | so qemu(1) man page and qemu help were outdated. | 4 | there can be a lot of fragmentation in the image after some time. Especially on VM's |
5 | 5 | that do a lot of writes/deletes. | |
6 | Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> | 6 | This causes the qcow2 image to grow even over 110% of its virtual size, |
7 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 7 | because the free gaps in the image get too small to allocate new |
8 | Reviewed-by: Julia Suvorova <jusual@redhat.com> | 8 | continuous clusters. So it allocates new space at the end of the image. |
9 | Reviewed-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com> | 9 | |
10 | Message-Id: <20200924151511.131471-1-sgarzare@redhat.com> | 10 | Disabling discard is not an option, as discard is needed to keep the |
11 | incremental backup size as low as possible. Without discard, the | ||
12 | incremental backups would become large, as qemu thinks it's just dirty | ||
13 | blocks but it doesn't know the blocks are unneeded. | ||
14 | So we need to avoid fragmentation but also 'empty' the unneeded blocks in | ||
15 | the image to have a small incremental backup. | ||
16 | |||
17 | In addition, we also want to send the discards further down the stack, so | ||
18 | the underlying blocks are still discarded. | ||
19 | |||
20 | Therefor we introduce a new qcow2 option "discard-no-unref". | ||
21 | When setting this option to true, discards will no longer have the qcow2 | ||
22 | driver relinquish cluster allocations. Other than that, the request is | ||
23 | handled as normal: All clusters in range are marked as zero, and, if | ||
24 | pass-discard-request is true, it is passed further down the stack. | ||
25 | The only difference is that the now-zero clusters are preallocated | ||
26 | instead of being unallocated. | ||
27 | This will avoid fragmentation on the qcow2 image. | ||
28 | |||
29 | Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1621 | ||
30 | Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be> | ||
31 | Message-Id: <20230605084523.34134-2-jean-louis@dupond.be> | ||
32 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
33 | Signed-off-by: Hanna Czenczek <hreitz@redhat.com> | ||
11 | --- | 34 | --- |
12 | qemu-options.hx | 10 ++++++---- | 35 | qapi/block-core.json | 12 ++++++++++++ |
13 | 1 file changed, 6 insertions(+), 4 deletions(-) | 36 | block/qcow2.h | 3 +++ |
14 | 37 | block/qcow2-cluster.c | 32 ++++++++++++++++++++++++++++---- | |
38 | block/qcow2.c | 18 ++++++++++++++++++ | ||
39 | qemu-options.hx | 12 ++++++++++++ | ||
40 | 5 files changed, 73 insertions(+), 4 deletions(-) | ||
41 | |||
42 | diff --git a/qapi/block-core.json b/qapi/block-core.json | ||
43 | index XXXXXXX..XXXXXXX 100644 | ||
44 | --- a/qapi/block-core.json | ||
45 | +++ b/qapi/block-core.json | ||
46 | @@ -XXX,XX +XXX,XX @@ | ||
47 | # @pass-discard-other: whether discard requests for the data source | ||
48 | # should be issued on other occasions where a cluster gets freed | ||
49 | # | ||
50 | +# @discard-no-unref: when enabled, discards from the guest will not cause | ||
51 | +# cluster allocations to be relinquished. This prevents qcow2 fragmentation | ||
52 | +# that would be caused by such discards. Besides potential | ||
53 | +# performance degradation, such fragmentation can lead to increased | ||
54 | +# allocation of clusters past the end of the image file, | ||
55 | +# resulting in image files whose file length can grow much larger | ||
56 | +# than their guest disk size would suggest. | ||
57 | +# If image file length is of concern (e.g. when storing qcow2 | ||
58 | +# images directly on block devices), you should consider enabling | ||
59 | +# this option. (since 8.1) | ||
60 | +# | ||
61 | # @overlap-check: which overlap checks to perform for writes to the | ||
62 | # image, defaults to 'cached' (since 2.2) | ||
63 | # | ||
64 | @@ -XXX,XX +XXX,XX @@ | ||
65 | '*pass-discard-request': 'bool', | ||
66 | '*pass-discard-snapshot': 'bool', | ||
67 | '*pass-discard-other': 'bool', | ||
68 | + '*discard-no-unref': 'bool', | ||
69 | '*overlap-check': 'Qcow2OverlapChecks', | ||
70 | '*cache-size': 'int', | ||
71 | '*l2-cache-size': 'int', | ||
72 | diff --git a/block/qcow2.h b/block/qcow2.h | ||
73 | index XXXXXXX..XXXXXXX 100644 | ||
74 | --- a/block/qcow2.h | ||
75 | +++ b/block/qcow2.h | ||
76 | @@ -XXX,XX +XXX,XX @@ | ||
77 | #define QCOW2_OPT_DISCARD_REQUEST "pass-discard-request" | ||
78 | #define QCOW2_OPT_DISCARD_SNAPSHOT "pass-discard-snapshot" | ||
79 | #define QCOW2_OPT_DISCARD_OTHER "pass-discard-other" | ||
80 | +#define QCOW2_OPT_DISCARD_NO_UNREF "discard-no-unref" | ||
81 | #define QCOW2_OPT_OVERLAP "overlap-check" | ||
82 | #define QCOW2_OPT_OVERLAP_TEMPLATE "overlap-check.template" | ||
83 | #define QCOW2_OPT_OVERLAP_MAIN_HEADER "overlap-check.main-header" | ||
84 | @@ -XXX,XX +XXX,XX @@ typedef struct BDRVQcow2State { | ||
85 | |||
86 | bool discard_passthrough[QCOW2_DISCARD_MAX]; | ||
87 | |||
88 | + bool discard_no_unref; | ||
89 | + | ||
90 | int overlap_check; /* bitmask of Qcow2MetadataOverlap values */ | ||
91 | bool signaled_corruption; | ||
92 | |||
93 | diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c | ||
94 | index XXXXXXX..XXXXXXX 100644 | ||
95 | --- a/block/qcow2-cluster.c | ||
96 | +++ b/block/qcow2-cluster.c | ||
97 | @@ -XXX,XX +XXX,XX @@ static int discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, | ||
98 | uint64_t new_l2_bitmap = old_l2_bitmap; | ||
99 | QCow2ClusterType cluster_type = | ||
100 | qcow2_get_cluster_type(bs, old_l2_entry); | ||
101 | + bool keep_reference = (cluster_type != QCOW2_CLUSTER_COMPRESSED) && | ||
102 | + !full_discard && | ||
103 | + (s->discard_no_unref && | ||
104 | + type == QCOW2_DISCARD_REQUEST); | ||
105 | |||
106 | /* | ||
107 | * If full_discard is true, the cluster should not read back as zeroes, | ||
108 | @@ -XXX,XX +XXX,XX @@ static int discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, | ||
109 | new_l2_entry = new_l2_bitmap = 0; | ||
110 | } else if (bs->backing || qcow2_cluster_is_allocated(cluster_type)) { | ||
111 | if (has_subclusters(s)) { | ||
112 | - new_l2_entry = 0; | ||
113 | + if (keep_reference) { | ||
114 | + new_l2_entry = old_l2_entry; | ||
115 | + } else { | ||
116 | + new_l2_entry = 0; | ||
117 | + } | ||
118 | new_l2_bitmap = QCOW_L2_BITMAP_ALL_ZEROES; | ||
119 | } else { | ||
120 | - new_l2_entry = s->qcow_version >= 3 ? QCOW_OFLAG_ZERO : 0; | ||
121 | + if (s->qcow_version >= 3) { | ||
122 | + if (keep_reference) { | ||
123 | + new_l2_entry |= QCOW_OFLAG_ZERO; | ||
124 | + } else { | ||
125 | + new_l2_entry = QCOW_OFLAG_ZERO; | ||
126 | + } | ||
127 | + } else { | ||
128 | + new_l2_entry = 0; | ||
129 | + } | ||
130 | } | ||
131 | } | ||
132 | |||
133 | @@ -XXX,XX +XXX,XX @@ static int discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, | ||
134 | if (has_subclusters(s)) { | ||
135 | set_l2_bitmap(s, l2_slice, l2_index + i, new_l2_bitmap); | ||
136 | } | ||
137 | - /* Then decrease the refcount */ | ||
138 | - qcow2_free_any_cluster(bs, old_l2_entry, type); | ||
139 | + if (!keep_reference) { | ||
140 | + /* Then decrease the refcount */ | ||
141 | + qcow2_free_any_cluster(bs, old_l2_entry, type); | ||
142 | + } else if (s->discard_passthrough[type] && | ||
143 | + (cluster_type == QCOW2_CLUSTER_NORMAL || | ||
144 | + cluster_type == QCOW2_CLUSTER_ZERO_ALLOC)) { | ||
145 | + /* If we keep the reference, pass on the discard still */ | ||
146 | + bdrv_pdiscard(s->data_file, old_l2_entry & L2E_OFFSET_MASK, | ||
147 | + s->cluster_size); | ||
148 | + } | ||
149 | } | ||
150 | |||
151 | qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice); | ||
152 | diff --git a/block/qcow2.c b/block/qcow2.c | ||
153 | index XXXXXXX..XXXXXXX 100644 | ||
154 | --- a/block/qcow2.c | ||
155 | +++ b/block/qcow2.c | ||
156 | @@ -XXX,XX +XXX,XX @@ static const char *const mutable_opts[] = { | ||
157 | QCOW2_OPT_DISCARD_REQUEST, | ||
158 | QCOW2_OPT_DISCARD_SNAPSHOT, | ||
159 | QCOW2_OPT_DISCARD_OTHER, | ||
160 | + QCOW2_OPT_DISCARD_NO_UNREF, | ||
161 | QCOW2_OPT_OVERLAP, | ||
162 | QCOW2_OPT_OVERLAP_TEMPLATE, | ||
163 | QCOW2_OPT_OVERLAP_MAIN_HEADER, | ||
164 | @@ -XXX,XX +XXX,XX @@ static QemuOptsList qcow2_runtime_opts = { | ||
165 | .type = QEMU_OPT_BOOL, | ||
166 | .help = "Generate discard requests when other clusters are freed", | ||
167 | }, | ||
168 | + { | ||
169 | + .name = QCOW2_OPT_DISCARD_NO_UNREF, | ||
170 | + .type = QEMU_OPT_BOOL, | ||
171 | + .help = "Do not unreference discarded clusters", | ||
172 | + }, | ||
173 | { | ||
174 | .name = QCOW2_OPT_OVERLAP, | ||
175 | .type = QEMU_OPT_STRING, | ||
176 | @@ -XXX,XX +XXX,XX @@ typedef struct Qcow2ReopenState { | ||
177 | bool use_lazy_refcounts; | ||
178 | int overlap_check; | ||
179 | bool discard_passthrough[QCOW2_DISCARD_MAX]; | ||
180 | + bool discard_no_unref; | ||
181 | uint64_t cache_clean_interval; | ||
182 | QCryptoBlockOpenOptions *crypto_opts; /* Disk encryption runtime options */ | ||
183 | } Qcow2ReopenState; | ||
184 | @@ -XXX,XX +XXX,XX @@ static int qcow2_update_options_prepare(BlockDriverState *bs, | ||
185 | r->discard_passthrough[QCOW2_DISCARD_OTHER] = | ||
186 | qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false); | ||
187 | |||
188 | + r->discard_no_unref = qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_NO_UNREF, | ||
189 | + false); | ||
190 | + if (r->discard_no_unref && s->qcow_version < 3) { | ||
191 | + error_setg(errp, | ||
192 | + "discard-no-unref is only supported since qcow2 version 3"); | ||
193 | + ret = -EINVAL; | ||
194 | + goto fail; | ||
195 | + } | ||
196 | + | ||
197 | switch (s->crypt_method_header) { | ||
198 | case QCOW_CRYPT_NONE: | ||
199 | if (encryptfmt) { | ||
200 | @@ -XXX,XX +XXX,XX @@ static void qcow2_update_options_commit(BlockDriverState *bs, | ||
201 | s->discard_passthrough[i] = r->discard_passthrough[i]; | ||
202 | } | ||
203 | |||
204 | + s->discard_no_unref = r->discard_no_unref; | ||
205 | + | ||
206 | if (s->cache_clean_interval != r->cache_clean_interval) { | ||
207 | cache_clean_timer_del(bs); | ||
208 | s->cache_clean_interval = r->cache_clean_interval; | ||
15 | diff --git a/qemu-options.hx b/qemu-options.hx | 209 | diff --git a/qemu-options.hx b/qemu-options.hx |
16 | index XXXXXXX..XXXXXXX 100644 | 210 | index XXXXXXX..XXXXXXX 100644 |
17 | --- a/qemu-options.hx | 211 | --- a/qemu-options.hx |
18 | +++ b/qemu-options.hx | 212 | +++ b/qemu-options.hx |
19 | @@ -XXX,XX +XXX,XX @@ SRST | 213 | @@ -XXX,XX +XXX,XX @@ SRST |
20 | The path to the image file in the local filesystem | 214 | issued on other occasions where a cluster gets freed |
21 | 215 | (on/off; default: off) | |
22 | ``aio`` | 216 | |
23 | - Specifies the AIO backend (threads/native, default: threads) | 217 | + ``discard-no-unref`` |
24 | + Specifies the AIO backend (threads/native/io_uring, | 218 | + When enabled, discards from the guest will not cause cluster |
25 | + default: threads) | 219 | + allocations to be relinquished. This prevents qcow2 fragmentation |
26 | 220 | + that would be caused by such discards. Besides potential | |
27 | ``locking`` | 221 | + performance degradation, such fragmentation can lead to increased |
28 | Specifies whether the image file is protected with Linux OFD | 222 | + allocation of clusters past the end of the image file, |
29 | @@ -XXX,XX +XXX,XX @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive, | 223 | + resulting in image files whose file length can grow much larger |
30 | "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" | 224 | + than their guest disk size would suggest. |
31 | " [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n" | 225 | + If image file length is of concern (e.g. when storing qcow2 |
32 | " [,snapshot=on|off][,rerror=ignore|stop|report]\n" | 226 | + images directly on block devices), you should consider enabling |
33 | - " [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n" | 227 | + this option. |
34 | + " [,werror=ignore|stop|report|enospc][,id=name]\n" | 228 | + |
35 | + " [,aio=threads|native|io_uring]\n" | 229 | ``overlap-check`` |
36 | " [,readonly=on|off][,copy-on-read=on|off]\n" | 230 | Which overlap checks to perform for writes to the image |
37 | " [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n" | 231 | (none/constant/cached/all; default: cached). For details or |
38 | " [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n" | ||
39 | @@ -XXX,XX +XXX,XX @@ SRST | ||
40 | The default mode is ``cache=writeback``. | ||
41 | |||
42 | ``aio=aio`` | ||
43 | - aio is "threads", or "native" and selects between pthread based | ||
44 | - disk I/O and native Linux AIO. | ||
45 | + aio is "threads", "native", or "io_uring" and selects between pthread | ||
46 | + based disk I/O, native Linux AIO, or Linux io_uring API. | ||
47 | |||
48 | ``format=format`` | ||
49 | Specify which disk format will be used rather than detecting the | ||
50 | -- | 232 | -- |
51 | 2.26.2 | 233 | 2.40.1 |
52 | diff view generated by jsdifflib |