From e9b39dadb638a8e68f0f8f30991c6307000cc293 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Tue, 11 Feb 2020 10:48:59 +0100 Subject: [PATCH 1/4] qcow2: Fix qcow2_alloc_cluster_abort() for external data file For external data file, cluster allocations return an offset in the data file and are not refcounted. In this case, there is nothing to do for qcow2_alloc_cluster_abort(). Freeing the same offset in the qcow2 file is wrong and causes crashes in the better case or image corruption in the worse case. Signed-off-by: Kevin Wolf Message-Id: <20200211094900.17315-3-kwolf@redhat.com> Signed-off-by: Kevin Wolf --- ...alloc_cluster_abort-for-external-dat.patch | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 qcow2-Fix-qcow2_alloc_cluster_abort-for-external-dat.patch diff --git a/qcow2-Fix-qcow2_alloc_cluster_abort-for-external-dat.patch b/qcow2-Fix-qcow2_alloc_cluster_abort-for-external-dat.patch new file mode 100644 index 00000000..8d9b71c7 --- /dev/null +++ b/qcow2-Fix-qcow2_alloc_cluster_abort-for-external-dat.patch @@ -0,0 +1,39 @@ +From fad649b88c93d0567be4e426f23063b439037095 Mon Sep 17 00:00:00 2001 +From: Kevin Wolf +Date: Tue, 11 Feb 2020 10:48:59 +0100 +Subject: [PATCH] qcow2: Fix qcow2_alloc_cluster_abort() for external data file + +For external data file, cluster allocations return an offset in the data +file and are not refcounted. In this case, there is nothing to do for +qcow2_alloc_cluster_abort(). Freeing the same offset in the qcow2 file +is wrong and causes crashes in the better case or image corruption in +the worse case. + +Signed-off-by: Kevin Wolf +Message-Id: <20200211094900.17315-3-kwolf@redhat.com> +Signed-off-by: Kevin Wolf +--- + block/qcow2-cluster.c | 7 +++++-- + 1 file changed, 5 insertions(+), 2 deletions(-) + +diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c +index f8576031b6..7e7e051437 100644 +--- a/block/qcow2-cluster.c ++++ b/block/qcow2-cluster.c +@@ -1026,8 +1026,11 @@ err: + void qcow2_alloc_cluster_abort(BlockDriverState *bs, QCowL2Meta *m) + { + BDRVQcow2State *s = bs->opaque; +- qcow2_free_clusters(bs, m->alloc_offset, m->nb_clusters << s->cluster_bits, +- QCOW2_DISCARD_NEVER); ++ if (!has_data_file(bs)) { ++ qcow2_free_clusters(bs, m->alloc_offset, ++ m->nb_clusters << s->cluster_bits, ++ QCOW2_DISCARD_NEVER); ++ } + } + + /* +-- +2.27.0 + -- Gitee From d59e6f7dbefdf2dc6fb7e3cf22d4060f330d5de3 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Thu, 26 Mar 2020 16:36:28 +0100 Subject: [PATCH 2/4] mirror: Wait only for in-flight operations mirror_wait_for_free_in_flight_slot() just picks a random operation to wait for. However, a MirrorOp is already in s->ops_in_flight when mirror_co_read() waits for free slots, so if not enough slots are immediately available, an operation can end up waiting for itself, or two or more operations can wait for each other to complete, which results in a hang. Fix this by adding a flag to MirrorOp that tells us if the request is already in flight (and therefore occupies slots that it will later free), and picking only such operations for waiting. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1794692 Signed-off-by: Kevin Wolf Message-Id: <20200326153628.4869-3-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- ...r-Wait-only-for-in-flight-operations.patch | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 mirror-Wait-only-for-in-flight-operations.patch diff --git a/mirror-Wait-only-for-in-flight-operations.patch b/mirror-Wait-only-for-in-flight-operations.patch new file mode 100644 index 00000000..d1b00c05 --- /dev/null +++ b/mirror-Wait-only-for-in-flight-operations.patch @@ -0,0 +1,83 @@ +From b4e1ea1c59e4dd8cc95b97ccc4eb1d3957fe5489 Mon Sep 17 00:00:00 2001 +From: Kevin Wolf +Date: Thu, 26 Mar 2020 16:36:28 +0100 +Subject: [PATCH] mirror: Wait only for in-flight operations + +mirror_wait_for_free_in_flight_slot() just picks a random operation to +wait for. However, a MirrorOp is already in s->ops_in_flight when +mirror_co_read() waits for free slots, so if not enough slots are +immediately available, an operation can end up waiting for itself, or +two or more operations can wait for each other to complete, which +results in a hang. + +Fix this by adding a flag to MirrorOp that tells us if the request is +already in flight (and therefore occupies slots that it will later +free), and picking only such operations for waiting. + +Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1794692 +Signed-off-by: Kevin Wolf +Message-Id: <20200326153628.4869-3-kwolf@redhat.com> +Reviewed-by: Eric Blake +Signed-off-by: Kevin Wolf +--- + block/mirror.c | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +diff --git a/block/mirror.c b/block/mirror.c +index 8f0d4544d8..abcf60a961 100644 +--- a/block/mirror.c ++++ b/block/mirror.c +@@ -100,6 +100,7 @@ struct MirrorOp { + + bool is_pseudo_op; + bool is_active_write; ++ bool is_in_flight; + CoQueue waiting_requests; + + QTAILQ_ENTRY(MirrorOp) next; +@@ -290,7 +291,9 @@ mirror_wait_for_any_operation(MirrorBlockJob *s, bool active) + * caller of this function. Since there is only one pseudo op + * at any given time, we will always find some real operation + * to wait on. */ +- if (!op->is_pseudo_op && op->is_active_write == active) { ++ if (!op->is_pseudo_op && op->is_in_flight && ++ op->is_active_write == active) ++ { + qemu_co_queue_wait(&op->waiting_requests, NULL); + return; + } +@@ -364,6 +367,7 @@ static void coroutine_fn mirror_co_read(void *opaque) + /* Copy the dirty cluster. */ + s->in_flight++; + s->bytes_in_flight += op->bytes; ++ op->is_in_flight = true; + trace_mirror_one_iteration(s, op->offset, op->bytes); + + ret = bdrv_co_preadv(s->mirror_top_bs->backing, op->offset, op->bytes, +@@ -379,6 +383,7 @@ static void coroutine_fn mirror_co_zero(void *opaque) + op->s->in_flight++; + op->s->bytes_in_flight += op->bytes; + *op->bytes_handled = op->bytes; ++ op->is_in_flight = true; + + ret = blk_co_pwrite_zeroes(op->s->target, op->offset, op->bytes, + op->s->unmap ? BDRV_REQ_MAY_UNMAP : 0); +@@ -393,6 +398,7 @@ static void coroutine_fn mirror_co_discard(void *opaque) + op->s->in_flight++; + op->s->bytes_in_flight += op->bytes; + *op->bytes_handled = op->bytes; ++ op->is_in_flight = true; + + ret = blk_co_pdiscard(op->s->target, op->offset, op->bytes); + mirror_write_complete(op, ret); +@@ -1305,6 +1311,7 @@ static MirrorOp *coroutine_fn active_write_prepare(MirrorBlockJob *s, + .offset = offset, + .bytes = bytes, + .is_active_write = true, ++ .is_in_flight = true, + }; + qemu_co_queue_init(&op->waiting_requests); + QTAILQ_INSERT_TAIL(&s->ops_in_flight, op, next); +-- +2.27.0 + -- Gitee From 1f5c5e9ea85b380e91367008b1c9444a8478fea4 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Thu, 22 Jul 2021 11:27:53 +0800 Subject: [PATCH 3/4] spec: Update patch and changelog with !166 mirror/qcow2: backport qemu-4.1 bugfix !166 qcow2: Fix qcow2_alloc_cluster_abort() for external data file mirror: Wait only for in-flight operations Signed-off-by: Chen Qun --- qemu.spec | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/qemu.spec b/qemu.spec index 9170229f..17bff9ba 100644 --- a/qemu.spec +++ b/qemu.spec @@ -384,6 +384,8 @@ Patch0371: iotests-143-Create-socket-in-SOCK_DIR.patch Patch0372: nbd-server-Avoid-long-error-message-assertions-CVE-2.patch Patch0373: block-Call-attention-to-truncation-of-long-NBD-expor.patch Patch0374: qemu-img-convert-Don-t-pre-zero-images.patch +Patch0375: qcow2-Fix-qcow2_alloc_cluster_abort-for-external-dat.patch +Patch0376: mirror-Wait-only-for-in-flight-operations.patch BuildRequires: flex BuildRequires: gcc @@ -778,6 +780,10 @@ getent passwd qemu >/dev/null || \ %endif %changelog +* Thu Jul 22 2021 Chen Qun +- qcow2: Fix qcow2_alloc_cluster_abort() for external data file +- mirror: Wait only for in-flight operations + * Wed Jul 21 2021 Chen Qun - block/curl: HTTP header fields allow whitespace around values - block/curl: HTTP header field names are case insensitive -- Gitee From 0cfcdff7016ad71843a1ae3c58fff4501977d2c6 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Thu, 22 Jul 2021 11:27:53 +0800 Subject: [PATCH 4/4] spec: Update release version with !166 increase release verison by one Signed-off-by: Chen Qun --- qemu.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu.spec b/qemu.spec index 17bff9ba..0c50ba83 100644 --- a/qemu.spec +++ b/qemu.spec @@ -1,6 +1,6 @@ Name: qemu Version: 4.1.0 -Release: 66 +Release: 67 Epoch: 2 Summary: QEMU is a generic and open source machine emulator and virtualizer License: GPLv2 and BSD and MIT and CC-BY-SA-4.0 -- Gitee