Merge ~paelzer/ubuntu/+source/qemu:fix-1848556-qemu-img-curl-hang-1848497-virt-balloon-migrate-EOAN into ubuntu/+source/qemu:ubuntu/eoan-devel

Proposed by Christian Ehrhardt 
Status: Merged
Approved by: Christian Ehrhardt 
Approved revision: 0ba9755b24716b71d94acae605a255299cb48a32
Merge reported by: Christian Ehrhardt 
Merged at revision: 0ba9755b24716b71d94acae605a255299cb48a32
Proposed branch: ~paelzer/ubuntu/+source/qemu:fix-1848556-qemu-img-curl-hang-1848497-virt-balloon-migrate-EOAN
Merge into: ubuntu/+source/qemu:ubuntu/eoan-devel
Diff against target: 329 lines (+301/-0)
4 files modified
debian/changelog (+10/-0)
debian/patches/series (+2/-0)
debian/patches/ubuntu/lp-1848497-virtio-balloon-fix-QEMU-4.0-config-size-migration-in.patch (+137/-0)
debian/patches/ubuntu/lp-1848556-curl-Handle-success-in-multi_check_completion.patch (+152/-0)
Reviewer Review Type Date Requested Status
Rafael David Tinoco (community) Approve
Canonical Server packageset reviewers Pending
Ubuntu Server Dev import team Pending
Review via email: mp+374771@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

I have reviewed the same change for Focal and I'm +1 for this change, including the backport changes for virtio balloon that Christian made regarding hw_compat.

review: Approve
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Tagged and uploaded

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Migrated, setting merged

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 430c149..79ff220 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,13 @@
6+qemu (1:4.0+dfsg-0ubuntu9.1) eoan; urgency=medium
7+
8+ * d/p/ubuntu/lp-1848556-curl-Handle-success-in-multi_check_completion.patch:
9+ fix a potential hang when qemu or qemu-img where accessing http backed
10+ disks via libcurl (LP: #1848556)
11+ * d/p/u/lp-1848497-virtio-balloon-fix-QEMU-4.0-config-size-migration-in.patch:
12+ fix migration issue from qemu <4.0 when using virtio-balloon (LP: #1848497)
13+
14+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Mon, 21 Oct 2019 14:51:45 +0200
15+
16 qemu (1:4.0+dfsg-0ubuntu9) eoan; urgency=medium
17
18 * d/p/lp-1842774-s390x-cpumodel-Add-the-z15-name-to-the-description-o.patch:
19diff --git a/debian/patches/series b/debian/patches/series
20index 72b9e84..4f2418d 100644
21--- a/debian/patches/series
22+++ b/debian/patches/series
23@@ -31,3 +31,5 @@ ubuntu/lp-1836154-s390x-cpumodel-also-change-name-of-vxbeh.patch
24 ubuntu/lp-1841066-i386-x86_cpu_list_feature_names-function.patch
25 ubuntu/lp-1841066-i386-unavailable-features-QOM-property.patch
26 lp-1842774-s390x-cpumodel-Add-the-z15-name-to-the-description-o.patch
27+ubuntu/lp-1848556-curl-Handle-success-in-multi_check_completion.patch
28+ubuntu/lp-1848497-virtio-balloon-fix-QEMU-4.0-config-size-migration-in.patch
29diff --git a/debian/patches/ubuntu/lp-1848497-virtio-balloon-fix-QEMU-4.0-config-size-migration-in.patch b/debian/patches/ubuntu/lp-1848497-virtio-balloon-fix-QEMU-4.0-config-size-migration-in.patch
30new file mode 100644
31index 0000000..37e5f34
32--- /dev/null
33+++ b/debian/patches/ubuntu/lp-1848497-virtio-balloon-fix-QEMU-4.0-config-size-migration-in.patch
34@@ -0,0 +1,137 @@
35+From 2bbadb08ce272d65e1f78621002008b07d1e0f03 Mon Sep 17 00:00:00 2001
36+From: Stefan Hajnoczi <stefanha@redhat.com>
37+Date: Wed, 10 Jul 2019 16:14:40 +0200
38+Subject: [PATCH] virtio-balloon: fix QEMU 4.0 config size migration
39+ incompatibility
40+
41+The virtio-balloon config size changed in QEMU 4.0 even for existing
42+machine types. Migration from QEMU 3.1 to 4.0 can fail in some
43+circumstances with the following error:
44+
45+ qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10 read: a1 device: 1 cmask: ff wmask: c0 w1cmask:0
46+
47+This happens because the virtio-balloon config size affects the VIRTIO
48+Legacy I/O Memory PCI BAR size.
49+
50+Introduce a qdev property called "qemu-4-0-config-size" and enable it
51+only for the QEMU 4.0 machine types. This way <4.0 machine types use
52+the old size, 4.0 uses the larger size, and >4.0 machine types use the
53+appropriate size depending on enabled virtio-balloon features.
54+
55+Live migration to and from old QEMUs to QEMU 4.1 works again as long as
56+a versioned machine type is specified (do not use just "pc"!).
57+
58+Originally-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
59+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
60+Message-Id: <20190710141440.27635-1-stefanha@redhat.com>
61+Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
62+Tested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
63+Tested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
64+Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
65+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
66+
67+Backport-Note: upstream commit was targetted for 4.1, needed changes to
68+work in 4.0 as HW_COMPAT code has the understanding of the "current"
69+level and when backported that is 4.0 and not 4.1 anymore.
70+Origin: backport, https://git.qemu.org/?p=qemu.git;a=commit;h=2bbadb08ce272d65e1f78621002008b07d1e0f03
71+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1848497
72+Last-Update: 2019-10-21
73+
74+---
75+ hw/core/machine.c | 2 ++
76+ hw/virtio/virtio-balloon.c | 28 +++++++++++++++++++++++++---
77+ include/hw/virtio/virtio-balloon.h | 2 ++
78+ 3 files changed, 29 insertions(+), 3 deletions(-)
79+
80+--- a/hw/core/machine.c
81++++ b/hw/core/machine.c
82+@@ -36,6 +36,7 @@ GlobalProperty hw_compat_3_1[] = {
83+ { "usb-kbd", "serial", "42" },
84+ { "virtio-blk-device", "discard", "false" },
85+ { "virtio-blk-device", "write-zeroes", "false" },
86++ { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
87+ };
88+ const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
89+
90+--- a/hw/virtio/virtio-balloon.c
91++++ b/hw/virtio/virtio-balloon.c
92+@@ -615,6 +615,22 @@ virtio_balloon_free_page_report_notify(N
93+ return 0;
94+ }
95+
96++static size_t virtio_balloon_config_size(VirtIOBalloon *s)
97++{
98++ uint64_t features = s->host_features;
99++
100++ if (s->qemu_4_0_config_size) {
101++ return sizeof(struct virtio_balloon_config);
102++ }
103++ if (virtio_has_feature(features, VIRTIO_BALLOON_F_PAGE_POISON)) {
104++ return sizeof(struct virtio_balloon_config);
105++ }
106++ if (virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
107++ return offsetof(struct virtio_balloon_config, poison_val);
108++ }
109++ return offsetof(struct virtio_balloon_config, free_page_report_cmd_id);
110++}
111++
112+ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
113+ {
114+ VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
115+@@ -635,7 +651,7 @@ static void virtio_balloon_get_config(Vi
116+ }
117+
118+ trace_virtio_balloon_get_config(config.num_pages, config.actual);
119+- memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
120++ memcpy(config_data, &config, virtio_balloon_config_size(dev));
121+ }
122+
123+ static int build_dimm_list(Object *obj, void *opaque)
124+@@ -679,7 +695,7 @@ static void virtio_balloon_set_config(Vi
125+ uint32_t oldactual = dev->actual;
126+ ram_addr_t vm_ram_size = get_current_ram_size();
127+
128+- memcpy(&config, config_data, sizeof(struct virtio_balloon_config));
129++ memcpy(&config, config_data, virtio_balloon_config_size(dev));
130+ dev->actual = le32_to_cpu(config.actual);
131+ if (dev->actual != oldactual) {
132+ qapi_event_send_balloon_change(vm_ram_size -
133+@@ -766,7 +782,7 @@ static void virtio_balloon_device_realiz
134+ int ret;
135+
136+ virtio_init(vdev, "virtio-balloon", VIRTIO_ID_BALLOON,
137+- sizeof(struct virtio_balloon_config));
138++ virtio_balloon_config_size(s));
139+
140+ ret = qemu_add_balloon_handler(virtio_balloon_to_target,
141+ virtio_balloon_stat, s);
142+@@ -897,6 +913,18 @@ static Property virtio_balloon_propertie
143+ VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
144+ DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features,
145+ VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
146++ /* QEMU 4.0 accidentally changed the config size even when free-page-hint
147++ * is disabled, resulting in QEMU 3.1 migration incompatibility. This
148++ * property retains this quirk for QEMU 4.1 machine types.
149++ * Backport:
150++ * Original patch is for 4.1 and sets "false", but in HW_COMPAT sets
151++ * 4.0 to "true" and 3.1 (and former) to "false".
152++ * Emulate that in the 4.0 code by setting true here, and keeping the
153++ * HW_COMPAT for 3.1 (4.0 HW_COMPAT doesn't exist yet anyway).
154++ * That way it will be forward compatible to qemu >=4.1
155++ */
156++ DEFINE_PROP_BOOL("qemu-4-0-config-size", VirtIOBalloon,
157++ qemu_4_0_config_size, true),
158+ DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD,
159+ IOThread *),
160+ DEFINE_PROP_END_OF_LIST(),
161+--- a/include/hw/virtio/virtio-balloon.h
162++++ b/include/hw/virtio/virtio-balloon.h
163+@@ -71,6 +71,8 @@ typedef struct VirtIOBalloon {
164+ int64_t stats_poll_interval;
165+ uint32_t host_features;
166+ PartiallyBalloonedPage *pbp;
167++
168++ bool qemu_4_0_config_size;
169+ } VirtIOBalloon;
170+
171+ #endif
172diff --git a/debian/patches/ubuntu/lp-1848556-curl-Handle-success-in-multi_check_completion.patch b/debian/patches/ubuntu/lp-1848556-curl-Handle-success-in-multi_check_completion.patch
173new file mode 100644
174index 0000000..37d4407
175--- /dev/null
176+++ b/debian/patches/ubuntu/lp-1848556-curl-Handle-success-in-multi_check_completion.patch
177@@ -0,0 +1,152 @@
178+From bfb23b480a49114315877aacf700b49453e0f9d9 Mon Sep 17 00:00:00 2001
179+From: Max Reitz <mreitz@redhat.com>
180+Date: Tue, 10 Sep 2019 14:41:35 +0200
181+Subject: [PATCH] curl: Handle success in multi_check_completion
182+
183+Background: As of cURL 7.59.0, it verifies that several functions are
184+not called from within a callback. Among these functions is
185+curl_multi_add_handle().
186+
187+curl_read_cb() is a callback from cURL and not a coroutine. Waking up
188+acb->co will lead to entering it then and there, which means the current
189+request will settle and the caller (if it runs in the same coroutine)
190+may then issue the next request. In such a case, we will enter
191+curl_setup_preadv() effectively from within curl_read_cb().
192+
193+Calling curl_multi_add_handle() will then fail and the new request will
194+not be processed.
195+
196+Fix this by not letting curl_read_cb() wake up acb->co. Instead, leave
197+the whole business of settling the AIOCB objects to
198+curl_multi_check_completion() (which is called from our timer callback
199+and our FD handler, so not from any cURL callbacks).
200+
201+Reported-by: Natalie Gavrielov <ngavrilo@redhat.com>
202+Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1740193
203+Cc: qemu-stable@nongnu.org
204+Signed-off-by: Max Reitz <mreitz@redhat.com>
205+Message-id: 20190910124136.10565-7-mreitz@redhat.com
206+Reviewed-by: John Snow <jsnow@redhat.com>
207+Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
208+Signed-off-by: Max Reitz <mreitz@redhat.com>
209+
210+Origin: upstream, https://git.qemu.org/?p=qemu.git;a=commit;h=bfb23b48
211+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1848556
212+Last-Update: 2019-10-21
213+
214+---
215+ block/curl.c | 69 ++++++++++++++++++++++------------------------------
216+ 1 file changed, 29 insertions(+), 40 deletions(-)
217+
218+diff --git a/block/curl.c b/block/curl.c
219+index fd70f1ebc4..c343c7ed3d 100644
220+--- a/block/curl.c
221++++ b/block/curl.c
222+@@ -229,7 +229,6 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
223+ {
224+ CURLState *s = ((CURLState*)opaque);
225+ size_t realsize = size * nmemb;
226+- int i;
227+
228+ trace_curl_read_cb(realsize);
229+
230+@@ -245,32 +244,6 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
231+ memcpy(s->orig_buf + s->buf_off, ptr, realsize);
232+ s->buf_off += realsize;
233+
234+- for(i=0; i<CURL_NUM_ACB; i++) {
235+- CURLAIOCB *acb = s->acb[i];
236+-
237+- if (!acb)
238+- continue;
239+-
240+- if ((s->buf_off >= acb->end)) {
241+- size_t request_length = acb->bytes;
242+-
243+- qemu_iovec_from_buf(acb->qiov, 0, s->orig_buf + acb->start,
244+- acb->end - acb->start);
245+-
246+- if (acb->end - acb->start < request_length) {
247+- size_t offset = acb->end - acb->start;
248+- qemu_iovec_memset(acb->qiov, offset, 0,
249+- request_length - offset);
250+- }
251+-
252+- acb->ret = 0;
253+- s->acb[i] = NULL;
254+- qemu_mutex_unlock(&s->s->mutex);
255+- aio_co_wake(acb->co);
256+- qemu_mutex_lock(&s->s->mutex);
257+- }
258+- }
259+-
260+ read_end:
261+ /* curl will error out if we do not return this value */
262+ return size * nmemb;
263+@@ -351,13 +324,14 @@ static void curl_multi_check_completion(BDRVCURLState *s)
264+ break;
265+
266+ if (msg->msg == CURLMSG_DONE) {
267++ int i;
268+ CURLState *state = NULL;
269++ bool error = msg->data.result != CURLE_OK;
270++
271+ curl_easy_getinfo(msg->easy_handle, CURLINFO_PRIVATE,
272+ (char **)&state);
273+
274+- /* ACBs for successful messages get completed in curl_read_cb */
275+- if (msg->data.result != CURLE_OK) {
276+- int i;
277++ if (error) {
278+ static int errcount = 100;
279+
280+ /* Don't lose the original error message from curl, since
281+@@ -369,20 +343,35 @@ static void curl_multi_check_completion(BDRVCURLState *s)
282+ error_report("curl: further errors suppressed");
283+ }
284+ }
285++ }
286+
287+- for (i = 0; i < CURL_NUM_ACB; i++) {
288+- CURLAIOCB *acb = state->acb[i];
289++ for (i = 0; i < CURL_NUM_ACB; i++) {
290++ CURLAIOCB *acb = state->acb[i];
291+
292+- if (acb == NULL) {
293+- continue;
294+- }
295++ if (acb == NULL) {
296++ continue;
297++ }
298++
299++ if (!error) {
300++ /* Assert that we have read all data */
301++ assert(state->buf_off >= acb->end);
302++
303++ qemu_iovec_from_buf(acb->qiov, 0,
304++ state->orig_buf + acb->start,
305++ acb->end - acb->start);
306+
307+- acb->ret = -EIO;
308+- state->acb[i] = NULL;
309+- qemu_mutex_unlock(&s->mutex);
310+- aio_co_wake(acb->co);
311+- qemu_mutex_lock(&s->mutex);
312++ if (acb->end - acb->start < acb->bytes) {
313++ size_t offset = acb->end - acb->start;
314++ qemu_iovec_memset(acb->qiov, offset, 0,
315++ acb->bytes - offset);
316++ }
317+ }
318++
319++ acb->ret = error ? -EIO : 0;
320++ state->acb[i] = NULL;
321++ qemu_mutex_unlock(&s->mutex);
322++ aio_co_wake(acb->co);
323++ qemu_mutex_lock(&s->mutex);
324+ }
325+
326+ curl_clean_state(state);
327+--
328+2.23.0
329+

Subscribers

People subscribed via source and target branches