1 | The following changes since commit d01beac177d44491d7db8747b79d94e1b53d173b: | 1 | The following changes since commit ca61fa4b803e5d0abaf6f1ceb690f23bb78a4def: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/kraxel/tags/vga-20180507-pull-request' into staging (2018-05-08 14:23:02 +0100) | 3 | Merge remote-tracking branch 'remotes/quic/tags/pull-hex-20211006' into staging (2021-10-06 12:11:14 -0700) |
4 | 4 | ||
5 | are available in the git repository at: | 5 | are available in the Git repository at: |
6 | 6 | ||
7 | git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request | 7 | https://gitlab.com/stefanha/qemu.git tags/block-pull-request |
8 | 8 | ||
9 | for you to fetch changes up to a2cb9239b7610ffb00f9ced5cd7640d40b0e1ccf: | 9 | for you to fetch changes up to 1cc7eada97914f090125e588497986f6f7900514: |
10 | 10 | ||
11 | sheepdog: Fix sd_co_create_opts() memory leaks (2018-05-08 10:47:27 -0400) | 11 | iothread: use IOThreadParamInfo in iothread_[set|get]_param() (2021-10-07 15:29:50 +0100) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Block patches | 14 | Pull request |
15 | |||
15 | ---------------------------------------------------------------- | 16 | ---------------------------------------------------------------- |
16 | 17 | ||
17 | Kevin Wolf (1): | 18 | Stefano Garzarella (2): |
18 | sheepdog: Fix sd_co_create_opts() memory leaks | 19 | iothread: rename PollParamInfo to IOThreadParamInfo |
20 | iothread: use IOThreadParamInfo in iothread_[set|get]_param() | ||
19 | 21 | ||
20 | Max Reitz (2): | 22 | iothread.c | 28 +++++++++++++++------------- |
21 | block/mirror: Make cancel always cancel pre-READY | 23 | 1 file changed, 15 insertions(+), 13 deletions(-) |
22 | iotests: Add test for cancelling a mirror job | ||
23 | |||
24 | Stefan Hajnoczi (1): | ||
25 | block/mirror: honor ratelimit again | ||
26 | |||
27 | block/mirror.c | 14 +++-- | ||
28 | block/sheepdog.c | 4 +- | ||
29 | tests/qemu-iotests/185.out | 4 +- | ||
30 | tests/qemu-iotests/218 | 138 +++++++++++++++++++++++++++++++++++++++++++++ | ||
31 | tests/qemu-iotests/218.out | 30 ++++++++++ | ||
32 | tests/qemu-iotests/group | 1 + | ||
33 | 6 files changed, 183 insertions(+), 8 deletions(-) | ||
34 | create mode 100644 tests/qemu-iotests/218 | ||
35 | create mode 100644 tests/qemu-iotests/218.out | ||
36 | 24 | ||
37 | -- | 25 | -- |
38 | 2.13.6 | 26 | 2.31.1 |
39 | 27 | ||
40 | 28 | ||
29 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Stefan Hajnoczi <stefanha@redhat.com> | ||
2 | 1 | ||
3 | Commit b76e4458b1eb3c32e9824fe6aa51f67d2b251748 ("block/mirror: change | ||
4 | the semantic of 'force' of block-job-cancel") accidentally removed the | ||
5 | ratelimit in the mirror job. | ||
6 | |||
7 | Reintroduce the ratelimit but keep the block-job-cancel force=true | ||
8 | behavior that was added in commit | ||
9 | b76e4458b1eb3c32e9824fe6aa51f67d2b251748. | ||
10 | |||
11 | Note that block_job_sleep_ns() returns immediately when the job is | ||
12 | cancelled. Therefore it's safe to unconditionally call | ||
13 | block_job_sleep_ns() - a cancelled job does not sleep. | ||
14 | |||
15 | This commit fixes the non-deterministic qemu-iotests 185 output. The | ||
16 | test relies on the ratelimit to make the job sleep until the 'quit' | ||
17 | command is processed. Previously the job could complete before the | ||
18 | 'quit' command was received since there was no ratelimit. | ||
19 | |||
20 | Cc: Liang Li <liliang.opensource@gmail.com> | ||
21 | Cc: Jeff Cody <jcody@redhat.com> | ||
22 | Cc: Kevin Wolf <kwolf@redhat.com> | ||
23 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
24 | Message-id: 20180424123527.19168-1-stefanha@redhat.com | ||
25 | Signed-off-by: Jeff Cody <jcody@redhat.com> | ||
26 | --- | ||
27 | block/mirror.c | 8 +++++--- | ||
28 | tests/qemu-iotests/185.out | 4 ++-- | ||
29 | 2 files changed, 7 insertions(+), 5 deletions(-) | ||
30 | |||
31 | diff --git a/block/mirror.c b/block/mirror.c | ||
32 | index XXXXXXX..XXXXXXX 100644 | ||
33 | --- a/block/mirror.c | ||
34 | +++ b/block/mirror.c | ||
35 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn mirror_run(void *opaque) | ||
36 | } | ||
37 | |||
38 | ret = 0; | ||
39 | + | ||
40 | + if (s->synced && !should_complete) { | ||
41 | + delay_ns = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 0); | ||
42 | + } | ||
43 | trace_mirror_before_sleep(s, cnt, s->synced, delay_ns); | ||
44 | + block_job_sleep_ns(&s->common, delay_ns); | ||
45 | if (block_job_is_cancelled(&s->common) && s->common.force) { | ||
46 | break; | ||
47 | - } else if (!should_complete) { | ||
48 | - delay_ns = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 0); | ||
49 | - block_job_sleep_ns(&s->common, delay_ns); | ||
50 | } | ||
51 | s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); | ||
52 | } | ||
53 | diff --git a/tests/qemu-iotests/185.out b/tests/qemu-iotests/185.out | ||
54 | index XXXXXXX..XXXXXXX 100644 | ||
55 | --- a/tests/qemu-iotests/185.out | ||
56 | +++ b/tests/qemu-iotests/185.out | ||
57 | @@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 backing_file=TEST_DIR/t.q | ||
58 | {"return": {}} | ||
59 | Formatting 'TEST_DIR/t.qcow2.copy', fmt=qcow2 size=67108864 cluster_size=65536 lazy_refcounts=off refcount_bits=16 | ||
60 | {"return": {}} | ||
61 | +{"return": {}} | ||
62 | +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} | ||
63 | {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "mirror"}} | ||
64 | -{"return": {}} | ||
65 | -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} | ||
66 | {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "mirror"}} | ||
67 | |||
68 | === Start backup job and exit qemu === | ||
69 | -- | ||
70 | 2.13.6 | ||
71 | |||
72 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Max Reitz <mreitz@redhat.com> | ||
2 | 1 | ||
3 | Commit b76e4458b1eb3c32e9824fe6aa51f67d2b251748 made the mirror block | ||
4 | job respect block-job-cancel's @force flag: With that flag set, it would | ||
5 | now always really cancel, even post-READY. | ||
6 | |||
7 | Unfortunately, it had a side effect: Without that flag set, it would now | ||
8 | never cancel, not even before READY. Considering that is an | ||
9 | incompatible change and not noted anywhere in the commit or the | ||
10 | description of block-job-cancel's @force parameter, this seems | ||
11 | unintentional and we should revert to the previous behavior, which is to | ||
12 | immediately cancel the job when block-job-cancel is called before source | ||
13 | and target are in sync (i.e. before the READY event). | ||
14 | |||
15 | Cc: qemu-stable@nongnu.org | ||
16 | Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1572856 | ||
17 | Reported-by: Yanan Fu <yfu@redhat.com> | ||
18 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
19 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
20 | Message-id: 20180501220509.14152-2-mreitz@redhat.com | ||
21 | Reviewed-by: Jeff Cody <jcody@redhat.com> | ||
22 | Signed-off-by: Jeff Cody <jcody@redhat.com> | ||
23 | --- | ||
24 | block/mirror.c | 4 +++- | ||
25 | 1 file changed, 3 insertions(+), 1 deletion(-) | ||
26 | |||
27 | diff --git a/block/mirror.c b/block/mirror.c | ||
28 | index XXXXXXX..XXXXXXX 100644 | ||
29 | --- a/block/mirror.c | ||
30 | +++ b/block/mirror.c | ||
31 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn mirror_run(void *opaque) | ||
32 | } | ||
33 | trace_mirror_before_sleep(s, cnt, s->synced, delay_ns); | ||
34 | block_job_sleep_ns(&s->common, delay_ns); | ||
35 | - if (block_job_is_cancelled(&s->common) && s->common.force) { | ||
36 | + if (block_job_is_cancelled(&s->common) && | ||
37 | + (!s->synced || s->common.force)) | ||
38 | + { | ||
39 | break; | ||
40 | } | ||
41 | s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); | ||
42 | -- | ||
43 | 2.13.6 | ||
44 | |||
45 | diff view generated by jsdifflib |
1 | From: Max Reitz <mreitz@redhat.com> | 1 | From: Stefano Garzarella <sgarzare@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | We already have an extensive mirror test (041) which does cover | 3 | Commit 1793ad0247 ("iothread: add aio-max-batch parameter") added |
4 | cancelling a mirror job, especially after it has emitted the READY | 4 | a new parameter (aio-max-batch) to IOThread and used PollParamInfo |
5 | event. However, it does not check what exact events are emitted after | 5 | structure to handle it. |
6 | block-job-cancel is executed. More importantly, it does not use | ||
7 | throttling to ensure that it covers the case of block-job-cancel before | ||
8 | READY. | ||
9 | 6 | ||
10 | It would be possible to add this case to 041, but considering it is | 7 | Since it is not a parameter of the polling mechanism, we rename the |
11 | already our largest test file, it makes sense to create a new file for | 8 | structure to a more generic IOThreadParamInfo. |
12 | these cases. | ||
13 | 9 | ||
14 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 10 | Suggested-by: Kevin Wolf <kwolf@redhat.com> |
15 | Message-id: 20180501220509.14152-3-mreitz@redhat.com | 11 | Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> |
16 | Signed-off-by: Jeff Cody <jcody@redhat.com> | 12 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> |
13 | Message-id: 20210727145936.147032-2-sgarzare@redhat.com | ||
14 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
17 | --- | 15 | --- |
18 | tests/qemu-iotests/218 | 138 +++++++++++++++++++++++++++++++++++++++++++++ | 16 | iothread.c | 14 +++++++------- |
19 | tests/qemu-iotests/218.out | 30 ++++++++++ | 17 | 1 file changed, 7 insertions(+), 7 deletions(-) |
20 | tests/qemu-iotests/group | 1 + | ||
21 | 3 files changed, 169 insertions(+) | ||
22 | create mode 100644 tests/qemu-iotests/218 | ||
23 | create mode 100644 tests/qemu-iotests/218.out | ||
24 | 18 | ||
25 | diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218 | 19 | diff --git a/iothread.c b/iothread.c |
26 | new file mode 100644 | ||
27 | index XXXXXXX..XXXXXXX | ||
28 | --- /dev/null | ||
29 | +++ b/tests/qemu-iotests/218 | ||
30 | @@ -XXX,XX +XXX,XX @@ | ||
31 | +#!/usr/bin/env python | ||
32 | +# | ||
33 | +# This test covers what happens when a mirror block job is cancelled | ||
34 | +# in various phases of its existence. | ||
35 | +# | ||
36 | +# Note that this test only checks the emitted events (i.e. | ||
37 | +# BLOCK_JOB_COMPLETED vs. BLOCK_JOB_CANCELLED), it does not compare | ||
38 | +# whether the target is in sync with the source when the | ||
39 | +# BLOCK_JOB_COMPLETED event occurs. This is covered by other tests | ||
40 | +# (such as 041). | ||
41 | +# | ||
42 | +# Copyright (C) 2018 Red Hat, Inc. | ||
43 | +# | ||
44 | +# This program is free software; you can redistribute it and/or modify | ||
45 | +# it under the terms of the GNU General Public License as published by | ||
46 | +# the Free Software Foundation; either version 2 of the License, or | ||
47 | +# (at your option) any later version. | ||
48 | +# | ||
49 | +# This program is distributed in the hope that it will be useful, | ||
50 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
51 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
52 | +# GNU General Public License for more details. | ||
53 | +# | ||
54 | +# You should have received a copy of the GNU General Public License | ||
55 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
56 | +# | ||
57 | +# Creator/Owner: Max Reitz <mreitz@redhat.com> | ||
58 | + | ||
59 | +import iotests | ||
60 | +from iotests import log | ||
61 | + | ||
62 | +iotests.verify_platform(['linux']) | ||
63 | + | ||
64 | + | ||
65 | +# Launches the VM, adds two null-co nodes (source and target), and | ||
66 | +# starts a blockdev-mirror job on them. | ||
67 | +# | ||
68 | +# Either both or none of speed and buf_size must be given. | ||
69 | + | ||
70 | +def start_mirror(vm, speed=None, buf_size=None): | ||
71 | + vm.launch() | ||
72 | + | ||
73 | + ret = vm.qmp('blockdev-add', | ||
74 | + node_name='source', | ||
75 | + driver='null-co', | ||
76 | + size=1048576) | ||
77 | + assert ret['return'] == {} | ||
78 | + | ||
79 | + ret = vm.qmp('blockdev-add', | ||
80 | + node_name='target', | ||
81 | + driver='null-co', | ||
82 | + size=1048576) | ||
83 | + assert ret['return'] == {} | ||
84 | + | ||
85 | + if speed is not None: | ||
86 | + ret = vm.qmp('blockdev-mirror', | ||
87 | + job_id='mirror', | ||
88 | + device='source', | ||
89 | + target='target', | ||
90 | + sync='full', | ||
91 | + speed=speed, | ||
92 | + buf_size=buf_size) | ||
93 | + else: | ||
94 | + ret = vm.qmp('blockdev-mirror', | ||
95 | + job_id='mirror', | ||
96 | + device='source', | ||
97 | + target='target', | ||
98 | + sync='full') | ||
99 | + | ||
100 | + assert ret['return'] == {} | ||
101 | + | ||
102 | + | ||
103 | +log('') | ||
104 | +log('=== Cancel mirror job before convergence ===') | ||
105 | +log('') | ||
106 | + | ||
107 | +log('--- force=false ---') | ||
108 | +log('') | ||
109 | + | ||
110 | +with iotests.VM() as vm: | ||
111 | + # Low speed so it does not converge | ||
112 | + start_mirror(vm, 65536, 65536) | ||
113 | + | ||
114 | + log('Cancelling job') | ||
115 | + log(vm.qmp('block-job-cancel', device='mirror', force=False)) | ||
116 | + | ||
117 | + log(vm.event_wait('BLOCK_JOB_CANCELLED'), | ||
118 | + filters=[iotests.filter_qmp_event]) | ||
119 | + | ||
120 | +log('') | ||
121 | +log('--- force=true ---') | ||
122 | +log('') | ||
123 | + | ||
124 | +with iotests.VM() as vm: | ||
125 | + # Low speed so it does not converge | ||
126 | + start_mirror(vm, 65536, 65536) | ||
127 | + | ||
128 | + log('Cancelling job') | ||
129 | + log(vm.qmp('block-job-cancel', device='mirror', force=True)) | ||
130 | + | ||
131 | + log(vm.event_wait('BLOCK_JOB_CANCELLED'), | ||
132 | + filters=[iotests.filter_qmp_event]) | ||
133 | + | ||
134 | + | ||
135 | +log('') | ||
136 | +log('=== Cancel mirror job after convergence ===') | ||
137 | +log('') | ||
138 | + | ||
139 | +log('--- force=false ---') | ||
140 | +log('') | ||
141 | + | ||
142 | +with iotests.VM() as vm: | ||
143 | + start_mirror(vm) | ||
144 | + | ||
145 | + log(vm.event_wait('BLOCK_JOB_READY'), | ||
146 | + filters=[iotests.filter_qmp_event]) | ||
147 | + | ||
148 | + log('Cancelling job') | ||
149 | + log(vm.qmp('block-job-cancel', device='mirror', force=False)) | ||
150 | + | ||
151 | + log(vm.event_wait('BLOCK_JOB_COMPLETED'), | ||
152 | + filters=[iotests.filter_qmp_event]) | ||
153 | + | ||
154 | +log('') | ||
155 | +log('--- force=true ---') | ||
156 | +log('') | ||
157 | + | ||
158 | +with iotests.VM() as vm: | ||
159 | + start_mirror(vm) | ||
160 | + | ||
161 | + log(vm.event_wait('BLOCK_JOB_READY'), | ||
162 | + filters=[iotests.filter_qmp_event]) | ||
163 | + | ||
164 | + log('Cancelling job') | ||
165 | + log(vm.qmp('block-job-cancel', device='mirror', force=True)) | ||
166 | + | ||
167 | + log(vm.event_wait('BLOCK_JOB_CANCELLED'), | ||
168 | + filters=[iotests.filter_qmp_event]) | ||
169 | diff --git a/tests/qemu-iotests/218.out b/tests/qemu-iotests/218.out | ||
170 | new file mode 100644 | ||
171 | index XXXXXXX..XXXXXXX | ||
172 | --- /dev/null | ||
173 | +++ b/tests/qemu-iotests/218.out | ||
174 | @@ -XXX,XX +XXX,XX @@ | ||
175 | + | ||
176 | +=== Cancel mirror job before convergence === | ||
177 | + | ||
178 | +--- force=false --- | ||
179 | + | ||
180 | +Cancelling job | ||
181 | +{u'return': {}} | ||
182 | +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 65536, u'len': 1048576, u'offset': 65536}, u'event': u'BLOCK_JOB_CANCELLED'} | ||
183 | + | ||
184 | +--- force=true --- | ||
185 | + | ||
186 | +Cancelling job | ||
187 | +{u'return': {}} | ||
188 | +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 65536, u'len': 1048576, u'offset': 65536}, u'event': u'BLOCK_JOB_CANCELLED'} | ||
189 | + | ||
190 | +=== Cancel mirror job after convergence === | ||
191 | + | ||
192 | +--- force=false --- | ||
193 | + | ||
194 | +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_READY'} | ||
195 | +Cancelling job | ||
196 | +{u'return': {}} | ||
197 | +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_COMPLETED'} | ||
198 | + | ||
199 | +--- force=true --- | ||
200 | + | ||
201 | +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_READY'} | ||
202 | +Cancelling job | ||
203 | +{u'return': {}} | ||
204 | +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_CANCELLED'} | ||
205 | diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group | ||
206 | index XXXXXXX..XXXXXXX 100644 | 20 | index XXXXXXX..XXXXXXX 100644 |
207 | --- a/tests/qemu-iotests/group | 21 | --- a/iothread.c |
208 | +++ b/tests/qemu-iotests/group | 22 | +++ b/iothread.c |
209 | @@ -XXX,XX +XXX,XX @@ | 23 | @@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp) |
210 | 211 rw auto quick | 24 | typedef struct { |
211 | 212 rw auto quick | 25 | const char *name; |
212 | 213 rw auto quick | 26 | ptrdiff_t offset; /* field's byte offset in IOThread struct */ |
213 | +218 rw auto quick | 27 | -} PollParamInfo; |
28 | +} IOThreadParamInfo; | ||
29 | |||
30 | -static PollParamInfo poll_max_ns_info = { | ||
31 | +static IOThreadParamInfo poll_max_ns_info = { | ||
32 | "poll-max-ns", offsetof(IOThread, poll_max_ns), | ||
33 | }; | ||
34 | -static PollParamInfo poll_grow_info = { | ||
35 | +static IOThreadParamInfo poll_grow_info = { | ||
36 | "poll-grow", offsetof(IOThread, poll_grow), | ||
37 | }; | ||
38 | -static PollParamInfo poll_shrink_info = { | ||
39 | +static IOThreadParamInfo poll_shrink_info = { | ||
40 | "poll-shrink", offsetof(IOThread, poll_shrink), | ||
41 | }; | ||
42 | -static PollParamInfo aio_max_batch_info = { | ||
43 | +static IOThreadParamInfo aio_max_batch_info = { | ||
44 | "aio-max-batch", offsetof(IOThread, aio_max_batch), | ||
45 | }; | ||
46 | |||
47 | @@ -XXX,XX +XXX,XX @@ static void iothread_get_param(Object *obj, Visitor *v, | ||
48 | const char *name, void *opaque, Error **errp) | ||
49 | { | ||
50 | IOThread *iothread = IOTHREAD(obj); | ||
51 | - PollParamInfo *info = opaque; | ||
52 | + IOThreadParamInfo *info = opaque; | ||
53 | int64_t *field = (void *)iothread + info->offset; | ||
54 | |||
55 | visit_type_int64(v, name, field, errp); | ||
56 | @@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v, | ||
57 | const char *name, void *opaque, Error **errp) | ||
58 | { | ||
59 | IOThread *iothread = IOTHREAD(obj); | ||
60 | - PollParamInfo *info = opaque; | ||
61 | + IOThreadParamInfo *info = opaque; | ||
62 | int64_t *field = (void *)iothread + info->offset; | ||
63 | int64_t value; | ||
64 | |||
214 | -- | 65 | -- |
215 | 2.13.6 | 66 | 2.31.1 |
216 | 67 | ||
217 | 68 | diff view generated by jsdifflib |
1 | From: Kevin Wolf <kwolf@redhat.com> | 1 | From: Stefano Garzarella <sgarzare@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Both the option string for the 'redundancy' option and the | 3 | Commit 0445409d74 ("iothread: generalize |
4 | SheepdogRedundancy object that is created accordingly could be leaked in | 4 | iothread_set_param/iothread_get_param") moved common code to set and |
5 | error paths. This fixes the memory leaks. | 5 | get IOThread parameters in two new functions. |
6 | 6 | ||
7 | Reported by Coverity (CID 1390614 and 1390641). | 7 | These functions are called inside callbacks, so we don't need to use an |
8 | opaque pointer. Let's replace `void *opaque` parameter with | ||
9 | `IOThreadParamInfo *info`. | ||
8 | 10 | ||
9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 11 | Suggested-by: Kevin Wolf <kwolf@redhat.com> |
10 | Message-id: 20180503153509.22223-1-kwolf@redhat.com | 12 | Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> |
11 | Reviewed-by: Jeff Cody <jcody@redhat.com> | 13 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> |
12 | Signed-off-by: Jeff Cody <jcody@redhat.com> | 14 | Message-id: 20210727145936.147032-3-sgarzare@redhat.com |
15 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
13 | --- | 16 | --- |
14 | block/sheepdog.c | 4 +++- | 17 | iothread.c | 18 ++++++++++-------- |
15 | 1 file changed, 3 insertions(+), 1 deletion(-) | 18 | 1 file changed, 10 insertions(+), 8 deletions(-) |
16 | 19 | ||
17 | diff --git a/block/sheepdog.c b/block/sheepdog.c | 20 | diff --git a/iothread.c b/iothread.c |
18 | index XXXXXXX..XXXXXXX 100644 | 21 | index XXXXXXX..XXXXXXX 100644 |
19 | --- a/block/sheepdog.c | 22 | --- a/iothread.c |
20 | +++ b/block/sheepdog.c | 23 | +++ b/iothread.c |
21 | @@ -XXX,XX +XXX,XX @@ static SheepdogRedundancy *parse_redundancy_str(const char *opt) | 24 | @@ -XXX,XX +XXX,XX @@ static IOThreadParamInfo aio_max_batch_info = { |
22 | } else { | 25 | }; |
23 | ret = qemu_strtol(n2, NULL, 10, &parity); | 26 | |
24 | if (ret < 0) { | 27 | static void iothread_get_param(Object *obj, Visitor *v, |
25 | + g_free(redundancy); | 28 | - const char *name, void *opaque, Error **errp) |
26 | return NULL; | 29 | + const char *name, IOThreadParamInfo *info, Error **errp) |
27 | } | 30 | { |
28 | 31 | IOThread *iothread = IOTHREAD(obj); | |
29 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_create_opts(const char *filename, QemuOpts *opts, | 32 | - IOThreadParamInfo *info = opaque; |
30 | QDict *qdict, *location_qdict; | 33 | int64_t *field = (void *)iothread + info->offset; |
31 | QObject *crumpled; | 34 | |
32 | Visitor *v; | 35 | visit_type_int64(v, name, field, errp); |
33 | - const char *redundancy; | ||
34 | + char *redundancy; | ||
35 | Error *local_err = NULL; | ||
36 | int ret; | ||
37 | |||
38 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_create_opts(const char *filename, QemuOpts *opts, | ||
39 | fail: | ||
40 | qapi_free_BlockdevCreateOptions(create_options); | ||
41 | qobject_unref(qdict); | ||
42 | + g_free(redundancy); | ||
43 | return ret; | ||
44 | } | 36 | } |
45 | 37 | ||
38 | static bool iothread_set_param(Object *obj, Visitor *v, | ||
39 | - const char *name, void *opaque, Error **errp) | ||
40 | + const char *name, IOThreadParamInfo *info, Error **errp) | ||
41 | { | ||
42 | IOThread *iothread = IOTHREAD(obj); | ||
43 | - IOThreadParamInfo *info = opaque; | ||
44 | int64_t *field = (void *)iothread + info->offset; | ||
45 | int64_t value; | ||
46 | |||
47 | @@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v, | ||
48 | static void iothread_get_poll_param(Object *obj, Visitor *v, | ||
49 | const char *name, void *opaque, Error **errp) | ||
50 | { | ||
51 | + IOThreadParamInfo *info = opaque; | ||
52 | |||
53 | - iothread_get_param(obj, v, name, opaque, errp); | ||
54 | + iothread_get_param(obj, v, name, info, errp); | ||
55 | } | ||
56 | |||
57 | static void iothread_set_poll_param(Object *obj, Visitor *v, | ||
58 | const char *name, void *opaque, Error **errp) | ||
59 | { | ||
60 | IOThread *iothread = IOTHREAD(obj); | ||
61 | + IOThreadParamInfo *info = opaque; | ||
62 | |||
63 | - if (!iothread_set_param(obj, v, name, opaque, errp)) { | ||
64 | + if (!iothread_set_param(obj, v, name, info, errp)) { | ||
65 | return; | ||
66 | } | ||
67 | |||
68 | @@ -XXX,XX +XXX,XX @@ static void iothread_set_poll_param(Object *obj, Visitor *v, | ||
69 | static void iothread_get_aio_param(Object *obj, Visitor *v, | ||
70 | const char *name, void *opaque, Error **errp) | ||
71 | { | ||
72 | + IOThreadParamInfo *info = opaque; | ||
73 | |||
74 | - iothread_get_param(obj, v, name, opaque, errp); | ||
75 | + iothread_get_param(obj, v, name, info, errp); | ||
76 | } | ||
77 | |||
78 | static void iothread_set_aio_param(Object *obj, Visitor *v, | ||
79 | const char *name, void *opaque, Error **errp) | ||
80 | { | ||
81 | IOThread *iothread = IOTHREAD(obj); | ||
82 | + IOThreadParamInfo *info = opaque; | ||
83 | |||
84 | - if (!iothread_set_param(obj, v, name, opaque, errp)) { | ||
85 | + if (!iothread_set_param(obj, v, name, info, errp)) { | ||
86 | return; | ||
87 | } | ||
88 | |||
46 | -- | 89 | -- |
47 | 2.13.6 | 90 | 2.31.1 |
48 | 91 | ||
49 | 92 | diff view generated by jsdifflib |