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

[Christian Ehrhardt ]

[Christian Ehrhardt  (paelzer)]

SRU part of https://code.launchpad.net/~paelzer/ubuntu/+source/qemu/+git/qemu/+merge/374770

PPA: https://launchpad.net/~paelzer/+archive/ubuntu/bug-1848556-qemu-img-curl-hang-eoan-1848497-virt-balloon-migrate

[Rafael David Tinoco (rafaeldtinoco)]

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

[Christian Ehrhardt  (paelzer)]

Tagged and uploaded

[Christian Ehrhardt  (paelzer)]

Migrated, setting merged

diff --git a/debian/changelog b/debian/changelog
index 430c149..79ff220 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
+qemu (1:4.0+dfsg-0ubuntu9.1) eoan; urgency=medium
+
+  * d/p/ubuntu/lp-1848556-curl-Handle-success-in-multi_check_completion.patch:
+    fix a potential hang when qemu or qemu-img where accessing http backed
+    disks via libcurl (LP: #1848556)
+  * d/p/u/lp-1848497-virtio-balloon-fix-QEMU-4.0-config-size-migration-in.patch:
+    fix migration issue from qemu <4.0 when using virtio-balloon (LP: #1848497)
+
+ -- Christian Ehrhardt <[e-mail]>  Mon, 21 Oct 2019 14:51:45 +0200
+
 qemu (1:4.0+dfsg-0ubuntu9) eoan; urgency=medium
 
   * d/p/lp-1842774-s390x-cpumodel-Add-the-z15-name-to-the-description-o.patch:
diff --git a/debian/patches/series b/debian/patches/series
index 72b9e84..4f2418d 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -31,3 +31,5 @@ ubuntu/lp-1836154-s390x-cpumodel-also-change-name-of-vxbeh.patch
 ubuntu/lp-1841066-i386-x86_cpu_list_feature_names-function.patch
 ubuntu/lp-1841066-i386-unavailable-features-QOM-property.patch
 lp-1842774-s390x-cpumodel-Add-the-z15-name-to-the-description-o.patch
+ubuntu/lp-1848556-curl-Handle-success-in-multi_check_completion.patch
+ubuntu/lp-1848497-virtio-balloon-fix-QEMU-4.0-config-size-migration-in.patch
diff --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
new file mode 100644
index 0000000..37e5f34
--- /dev/null
+++ b/debian/patches/ubuntu/lp-1848497-virtio-balloon-fix-QEMU-4.0-config-size-migration-in.patch
@@ -0,0 +1,137 @@
+From 2bbadb08ce272d65e1f78621002008b07d1e0f03 Mon Sep 17 00:00:00 2001
+From: Stefan Hajnoczi <[e-mail]>
+Date: Wed, 10 Jul 2019 16:14:40 +0200
+Subject: [PATCH] virtio-balloon: fix QEMU 4.0 config size migration
+ incompatibility
+
+The virtio-balloon config size changed in QEMU 4.0 even for existing
+machine types.  Migration from QEMU 3.1 to 4.0 can fail in some
+circumstances with the following error:
+
+  qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10 read: a1 device: 1 cmask: ff wmask: c0 w1cmask:0
+
+This happens because the virtio-balloon config size affects the VIRTIO
+Legacy I/O Memory PCI BAR size.
+
+Introduce a qdev property called "qemu-4-0-config-size" and enable it
+only for the QEMU 4.0 machine types.  This way <4.0 machine types use
+the old size, 4.0 uses the larger size, and >4.0 machine types use the
+appropriate size depending on enabled virtio-balloon features.
+
+Live migration to and from old QEMUs to QEMU 4.1 works again as long as
+a versioned machine type is specified (do not use just "pc"!).
+
+Originally-by: Wolfgang Bumiller <[e-mail]>
+Signed-off-by: Stefan Hajnoczi <[e-mail]>
+Message-Id: <[e-mail]>
+Reviewed-by: Dr. David Alan Gilbert <[e-mail]>
+Tested-by: Dr. David Alan Gilbert <[e-mail]>
+Tested-by: Wolfgang Bumiller <[e-mail]>
+Reviewed-by: Michael S. Tsirkin <[e-mail]>
+Signed-off-by: Michael S. Tsirkin <[e-mail]>
+
+Backport-Note: upstream commit was targetted for 4.1, needed changes to
+work in 4.0 as HW_COMPAT code has the understanding of the "current"
+level and when backported that is 4.0 and not 4.1 anymore.
+Origin: backport, https://git.qemu.org/?p=qemu.git;a=commit;h=2bbadb08ce272d65e1f78621002008b07d1e0f03
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1848497
+Last-Update: 2019-10-21
+
+---
+ hw/core/machine.c                  |  2 ++
+ hw/virtio/virtio-balloon.c         | 28 +++++++++++++++++++++++++---
+ include/hw/virtio/virtio-balloon.h |  2 ++
+ 3 files changed, 29 insertions(+), 3 deletions(-)
+
+--- a/hw/core/machine.c
++++ b/hw/core/machine.c
+@@ -36,6 +36,7 @@ GlobalProperty hw_compat_3_1[] = {
+     { "usb-kbd", "serial", "42" },
+     { "virtio-blk-device", "discard", "false" },
+     { "virtio-blk-device", "write-zeroes", "false" },
++    { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
+ };
+ const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
+ 
+--- a/hw/virtio/virtio-balloon.c
++++ b/hw/virtio/virtio-balloon.c
+@@ -615,6 +615,22 @@ virtio_balloon_free_page_report_notify(N
+     return 0;
+ }
+ 
++static size_t virtio_balloon_config_size(VirtIOBalloon *s)
++{
++    uint64_t features = s->host_features;
++
++    if (s->qemu_4_0_config_size) {
++        return sizeof(struct virtio_balloon_config);
++    }
++    if (virtio_has_feature(features, VIRTIO_BALLOON_F_PAGE_POISON)) {
++        return sizeof(struct virtio_balloon_config);
++    }
++    if (virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
++        return offsetof(struct virtio_balloon_config, poison_val);
++    }
++    return offsetof(struct virtio_balloon_config, free_page_report_cmd_id);
++}
++
+ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
+ {
+     VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
+@@ -635,7 +651,7 @@ static void virtio_balloon_get_config(Vi
+     }
+ 
+     trace_virtio_balloon_get_config(config.num_pages, config.actual);
+-    memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
++    memcpy(config_data, &config, virtio_balloon_config_size(dev));
+ }
+ 
+ static int build_dimm_list(Object *obj, void *opaque)
+@@ -679,7 +695,7 @@ static void virtio_balloon_set_config(Vi
+     uint32_t oldactual = dev->actual;
+     ram_addr_t vm_ram_size = get_current_ram_size();
+ 
+-    memcpy(&config, config_data, sizeof(struct virtio_balloon_config));
++    memcpy(&config, config_data, virtio_balloon_config_size(dev));
+     dev->actual = le32_to_cpu(config.actual);
+     if (dev->actual != oldactual) {
+         qapi_event_send_balloon_change(vm_ram_size -
+@@ -766,7 +782,7 @@ static void virtio_balloon_device_realiz
+     int ret;
+ 
+     virtio_init(vdev, "virtio-balloon", VIRTIO_ID_BALLOON,
+-                sizeof(struct virtio_balloon_config));
++                virtio_balloon_config_size(s));
+ 
+     ret = qemu_add_balloon_handler(virtio_balloon_to_target,
+                                    virtio_balloon_stat, s);
+@@ -897,6 +913,18 @@ static Property virtio_balloon_propertie
+                     VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
+     DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features,
+                     VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
++    /* QEMU 4.0 accidentally changed the config size even when free-page-hint
++     * is disabled, resulting in QEMU 3.1 migration incompatibility.  This
++     * property retains this quirk for QEMU 4.1 machine types.
++     * Backport:
++     * Original patch is for 4.1 and sets "false", but in HW_COMPAT sets
++     * 4.0 to "true" and 3.1 (and former) to "false".
++     * Emulate that in the 4.0 code by setting true here, and keeping the
++     * HW_COMPAT for 3.1 (4.0 HW_COMPAT doesn't exist yet anyway).
++     * That way it will be forward compatible to qemu >=4.1
++     */
++    DEFINE_PROP_BOOL("qemu-4-0-config-size", VirtIOBalloon,
++                     qemu_4_0_config_size, true),
+     DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD,
+                      IOThread *),
+     DEFINE_PROP_END_OF_LIST(),
+--- a/include/hw/virtio/virtio-balloon.h
++++ b/include/hw/virtio/virtio-balloon.h
+@@ -71,6 +71,8 @@ typedef struct VirtIOBalloon {
+     int64_t stats_poll_interval;
+     uint32_t host_features;
+     PartiallyBalloonedPage *pbp;
++
++    bool qemu_4_0_config_size;
+ } VirtIOBalloon;
+ 
+ #endif
diff --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
new file mode 100644
index 0000000..37d4407
--- /dev/null
+++ b/debian/patches/ubuntu/lp-1848556-curl-Handle-success-in-multi_check_completion.patch
@@ -0,0 +1,152 @@
+From bfb23b480a49114315877aacf700b49453e0f9d9 Mon Sep 17 00:00:00 2001
+From: Max Reitz <[e-mail]>
+Date: Tue, 10 Sep 2019 14:41:35 +0200
+Subject: [PATCH] curl: Handle success in multi_check_completion
+
+Background: As of cURL 7.59.0, it verifies that several functions are
+not called from within a callback.  Among these functions is
+curl_multi_add_handle().
+
+curl_read_cb() is a callback from cURL and not a coroutine.  Waking up
+acb->co will lead to entering it then and there, which means the current
+request will settle and the caller (if it runs in the same coroutine)
+may then issue the next request.  In such a case, we will enter
+curl_setup_preadv() effectively from within curl_read_cb().
+
+Calling curl_multi_add_handle() will then fail and the new request will
+not be processed.
+
+Fix this by not letting curl_read_cb() wake up acb->co.  Instead, leave
+the whole business of settling the AIOCB objects to
+curl_multi_check_completion() (which is called from our timer callback
+and our FD handler, so not from any cURL callbacks).
+
+Reported-by: Natalie Gavrielov <[e-mail]>
+Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1740193
+Cc: [e-mail]
+Signed-off-by: Max Reitz <[e-mail]>
+Message-id: [e-mail]
+Reviewed-by: John Snow <[e-mail]>
+Reviewed-by: Maxim Levitsky <[e-mail]>
+Signed-off-by: Max Reitz <[e-mail]>
+
+Origin: upstream, https://git.qemu.org/?p=qemu.git;a=commit;h=bfb23b48
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1848556
+Last-Update: 2019-10-21
+
+---
+ block/curl.c | 69 ++++++++++++++++++++++------------------------------
+ 1 file changed, 29 insertions(+), 40 deletions(-)
+
+diff --git a/block/curl.c b/block/curl.c
+index fd70f1ebc4..c343c7ed3d 100644
+--- a/block/curl.c
++++ b/block/curl.c
+@@ -229,7 +229,6 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
+ {
+     CURLState *s = ((CURLState*)opaque);
+     size_t realsize = size * nmemb;
+-    int i;
+ 
+     trace_curl_read_cb(realsize);
+ 
+@@ -245,32 +244,6 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
+     memcpy(s->orig_buf + s->buf_off, ptr, realsize);
+     s->buf_off += realsize;
+ 
+-    for(i=0; i<CURL_NUM_ACB; i++) {
+-        CURLAIOCB *acb = s->acb[i];
+-
+-        if (!acb)
+-            continue;
+-
+-        if ((s->buf_off >= acb->end)) {
+-            size_t request_length = acb->bytes;
+-
+-            qemu_iovec_from_buf(acb->qiov, 0, s->orig_buf + acb->start,
+-                                acb->end - acb->start);
+-
+-            if (acb->end - acb->start < request_length) {
+-                size_t offset = acb->end - acb->start;
+-                qemu_iovec_memset(acb->qiov, offset, 0,
+-                                  request_length - offset);
+-            }
+-
+-            acb->ret = 0;
+-            s->acb[i] = NULL;
+-            qemu_mutex_unlock(&s->s->mutex);
+-            aio_co_wake(acb->co);
+-            qemu_mutex_lock(&s->s->mutex);
+-        }
+-    }
+-
+ read_end:
+     /* curl will error out if we do not return this value */
+     return size * nmemb;
+@@ -351,13 +324,14 @@ static void curl_multi_check_completion(BDRVCURLState *s)
+             break;
+ 
+         if (msg->msg == CURLMSG_DONE) {
++            int i;
+             CURLState *state = NULL;
++            bool error = msg->data.result != CURLE_OK;
++
+             curl_easy_getinfo(msg->easy_handle, CURLINFO_PRIVATE,
+                               (char **)&state);
+ 
+-            /* ACBs for successful messages get completed in curl_read_cb */
+-            if (msg->data.result != CURLE_OK) {
+-                int i;
++            if (error) {
+                 static int errcount = 100;
+ 
+                 /* Don't lose the original error message from curl, since
+@@ -369,20 +343,35 @@ static void curl_multi_check_completion(BDRVCURLState *s)
+                         error_report("curl: further errors suppressed");
+                     }
+                 }
++            }
+ 
+-                for (i = 0; i < CURL_NUM_ACB; i++) {
+-                    CURLAIOCB *acb = state->acb[i];
++            for (i = 0; i < CURL_NUM_ACB; i++) {
++                CURLAIOCB *acb = state->acb[i];
+ 
+-                    if (acb == NULL) {
+-                        continue;
+-                    }
++                if (acb == NULL) {
++                    continue;
++                }
++
++                if (!error) {
++                    /* Assert that we have read all data */
++                    assert(state->buf_off >= acb->end);
++
++                    qemu_iovec_from_buf(acb->qiov, 0,
++                                        state->orig_buf + acb->start,
++                                        acb->end - acb->start);
+ 
+-                    acb->ret = -EIO;
+-                    state->acb[i] = NULL;
+-                    qemu_mutex_unlock(&s->mutex);
+-                    aio_co_wake(acb->co);
+-                    qemu_mutex_lock(&s->mutex);
++                    if (acb->end - acb->start < acb->bytes) {
++                        size_t offset = acb->end - acb->start;
++                        qemu_iovec_memset(acb->qiov, offset, 0,
++                                          acb->bytes - offset);
++                    }
+                 }
++
++                acb->ret = error ? -EIO : 0;
++                state->acb[i] = NULL;
++                qemu_mutex_unlock(&s->mutex);
++                aio_co_wake(acb->co);
++                qemu_mutex_lock(&s->mutex);
+             }
+ 
+             curl_clean_state(state);
+-- 
+2.23.0
+