some more stable fixes for QEMU 9.0.2

Fix the two issues reported in the community forum[0][1], i.e.
regression in LSI-53c895a controller and ignored boot order for USB
storage (only possible via custom arguments in Proxmox VE), both
causing boot failures, and pick up fixes for VirtIO, ARM emulation,
char IO device and a graph lock fix for the block layer.

The block-copy patches that serve as a preparation for fleecing are
moved to the extra folder, because the graph lock fix requires them
to be present first. They have been applied upstream in the meantime
and should drop out with the rebase on 9.1.

[0]: https://forum.proxmox.com/threads/149772/post-679433
[1]: https://forum.proxmox.com/threads/149772/post-683459

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
This commit is contained in:
Fiona Ebner 2024-07-25 11:45:54 +02:00 committed by Thomas Lamprecht
parent cf40e92996
commit 4154eea6e6
25 changed files with 485 additions and 42 deletions

View File

@ -258,7 +258,7 @@ index 1bdce3b657..0c5c72df2e 100644
errp);
if (!job) {
diff --git a/blockdev.c b/blockdev.c
index 057601dcf0..8682814a7a 100644
index 4c33c3f5f0..f3e508a6a7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2776,6 +2776,9 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs,
@ -349,7 +349,7 @@ index 057601dcf0..8682814a7a 100644
has_granularity, granularity,
has_buf_size, buf_size,
diff --git a/include/block/block_int-global-state.h b/include/block/block_int-global-state.h
index d2201e27f4..cc1387ae02 100644
index eb2d92a226..f0c642b194 100644
--- a/include/block/block_int-global-state.h
+++ b/include/block/block_int-global-state.h
@@ -158,7 +158,9 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
@ -364,10 +364,10 @@ index d2201e27f4..cc1387ae02 100644
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4b18e01b85..0902b0a024 100644
index b179d65520..905da8be72 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2170,6 +2170,15 @@
@@ -2174,6 +2174,15 @@
# destination (all the disk, only the sectors allocated in the
# topmost image, or only new I/O).
#
@ -383,7 +383,7 @@ index 4b18e01b85..0902b0a024 100644
# @granularity: granularity of the dirty bitmap, default is 64K if the
# image format doesn't have clusters, 4K if the clusters are
# smaller than that, else the cluster size. Must be a power of 2
@@ -2212,7 +2221,9 @@
@@ -2216,7 +2225,9 @@
{ 'struct': 'DriveMirror',
'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
'*format': 'str', '*node-name': 'str', '*replaces': 'str',
@ -394,7 +394,7 @@ index 4b18e01b85..0902b0a024 100644
'*speed': 'int', '*granularity': 'uint32',
'*buf-size': 'int', '*on-source-error': 'BlockdevOnError',
'*on-target-error': 'BlockdevOnError',
@@ -2492,6 +2503,15 @@
@@ -2496,6 +2507,15 @@
# destination (all the disk, only the sectors allocated in the
# topmost image, or only new I/O).
#
@ -410,7 +410,7 @@ index 4b18e01b85..0902b0a024 100644
# @granularity: granularity of the dirty bitmap, default is 64K if the
# image format doesn't have clusters, 4K if the clusters are
# smaller than that, else the cluster size. Must be a power of 2
@@ -2540,7 +2560,8 @@
@@ -2544,7 +2564,8 @@
{ 'command': 'blockdev-mirror',
'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
'*replaces': 'str',

View File

@ -16,7 +16,7 @@ Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
1 file changed, 3 insertions(+)
diff --git a/blockdev.c b/blockdev.c
index 8682814a7a..5b75a085ee 100644
index f3e508a6a7..37b8437f3e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2873,6 +2873,9 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs,

View File

@ -62,7 +62,7 @@ index 6b3cce1007..2f1223852b 100644
if (bitmap_mode != BITMAP_SYNC_MODE_NEVER) {
diff --git a/blockdev.c b/blockdev.c
index 5b75a085ee..d27d8c38ec 100644
index 37b8437f3e..ed8198f351 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2852,7 +2852,36 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs,

View File

@ -45,10 +45,10 @@ Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
10 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 16d611c4ca..1963e47ab9 100644
index ec29d6b810..3dd2e229d2 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -332,7 +332,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
@@ -356,7 +356,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
BlockDriverState *target, int64_t speed,
MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
BitmapSyncMode bitmap_mode,
@ -57,7 +57,7 @@ index 16d611c4ca..1963e47ab9 100644
const char *filter_node_name,
BackupPerf *perf,
BlockdevOnError on_source_error,
@@ -433,7 +433,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
@@ -457,7 +457,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
goto error;
}
@ -203,10 +203,10 @@ index ca6bd0a720..0415a5e8b7 100644
BLOCKDEV_ON_ERROR_REPORT, JOB_INTERNAL,
backup_job_completed, bs, NULL, &local_err);
diff --git a/blockdev.c b/blockdev.c
index 5e5dbc1da9..1054a69279 100644
index 057601dcf0..4c33c3f5f0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2727,7 +2727,7 @@ static BlockJob *do_backup_common(BackupCommon *backup,
@@ -2726,7 +2726,7 @@ static BlockJob *do_backup_common(BackupCommon *backup,
job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
backup->sync, bmap, backup->bitmap_mode,
@ -241,10 +241,10 @@ index 8b41643bfa..bdc703bacd 100644
/* Function should be called prior any actual copy request */
diff --git a/include/block/block_int-global-state.h b/include/block/block_int-global-state.h
index cc1387ae02..f0c642b194 100644
index d2201e27f4..eb2d92a226 100644
--- a/include/block/block_int-global-state.h
+++ b/include/block/block_int-global-state.h
@@ -195,7 +195,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
@@ -193,7 +193,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
MirrorSyncMode sync_mode,
BdrvDirtyBitmap *sync_bitmap,
BitmapSyncMode bitmap_mode,
@ -254,10 +254,10 @@ index cc1387ae02..f0c642b194 100644
BackupPerf *perf,
BlockdevOnError on_source_error,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index cb58a664ef..282e2e8a8c 100644
index 4b18e01b85..b179d65520 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1849,6 +1849,9 @@
@@ -1610,6 +1610,9 @@
# node specified by @drive. If this option is not given, a node
# name is autogenerated. (Since: 4.2)
#
@ -267,7 +267,7 @@ index cb58a664ef..282e2e8a8c 100644
# @x-perf: Performance options. (Since 6.0)
#
# Features:
@@ -1870,6 +1873,7 @@
@@ -1631,6 +1634,7 @@
'*on-target-error': 'BlockdevOnError',
'*auto-finalize': 'bool', '*auto-dismiss': 'bool',
'*filter-node-name': 'str',

View File

@ -0,0 +1,92 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Thomas Huth <thuth@redhat.com>
Date: Tue, 18 Jun 2024 14:19:58 +0200
Subject: [PATCH] hw/virtio: Fix the de-initialization of vhost-user devices
The unrealize functions of the various vhost-user devices are
calling the corresponding vhost_*_set_status() functions with a
status of 0 to shut down the device correctly.
Now these vhost_*_set_status() functions all follow this scheme:
bool should_start = virtio_device_should_start(vdev, status);
if (vhost_dev_is_started(&vvc->vhost_dev) == should_start) {
return;
}
if (should_start) {
/* ... do the initialization stuff ... */
} else {
/* ... do the cleanup stuff ... */
}
The problem here is virtio_device_should_start(vdev, 0) currently
always returns "true" since it internally only looks at vdev->started
instead of looking at the "status" parameter. Thus once the device
got started once, virtio_device_should_start() always returns true
and thus the vhost_*_set_status() functions return early, without
ever doing any clean-up when being called with status == 0. This
causes e.g. problems when trying to hot-plug and hot-unplug a vhost
user devices multiple times since the de-initialization step is
completely skipped during the unplug operation.
This bug has been introduced in commit 9f6bcfd99f ("hw/virtio: move
vm_running check to virtio_device_started") which replaced
should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;
with
should_start = virtio_device_started(vdev, status);
which later got replaced by virtio_device_should_start(). This blocked
the possibility to set should_start to false in case the status flag
VIRTIO_CONFIG_S_DRIVER_OK was not set.
Fix it by adjusting the virtio_device_should_start() function to
only consider the status flag instead of vdev->started. Since this
function is only used in the various vhost_*_set_status() functions
for exactly the same purpose, it should be fine to fix it in this
central place there without any risk to change the behavior of other
code.
Fixes: 9f6bcfd99f ("hw/virtio: move vm_running check to virtio_device_started")
Buglink: https://issues.redhat.com/browse/RHEL-40708
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20240618121958.88673-1-thuth@redhat.com>
Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
(cherry picked from commit d72479b11797c28893e1e3fc565497a9cae5ca16)
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
include/hw/virtio/virtio.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 7d5ffdc145..2eafad17b8 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -470,9 +470,9 @@ static inline bool virtio_device_started(VirtIODevice *vdev, uint8_t status)
* @vdev - the VirtIO device
* @status - the devices status bits
*
- * This is similar to virtio_device_started() but also encapsulates a
- * check on the VM status which would prevent a device starting
- * anyway.
+ * This is similar to virtio_device_started() but ignores vdev->started
+ * and also encapsulates a check on the VM status which would prevent a
+ * device from starting anyway.
*/
static inline bool virtio_device_should_start(VirtIODevice *vdev, uint8_t status)
{
@@ -480,7 +480,7 @@ static inline bool virtio_device_should_start(VirtIODevice *vdev, uint8_t status
return false;
}
- return virtio_device_started(vdev, status);
+ return status & VIRTIO_CONFIG_S_DRIVER_OK;
}
static inline void virtio_set_started(VirtIODevice *vdev, bool started)

View File

@ -0,0 +1,43 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Daniyal Khan <danikhan632@gmail.com>
Date: Wed, 17 Jul 2024 16:01:47 +1000
Subject: [PATCH] target/arm: Use float_status copy in sme_fmopa_s
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
We made a copy above because the fp exception flags
are not propagated back to the FPST register, but
then failed to use the copy.
Cc: qemu-stable@nongnu.org
Fixes: 558e956c719 ("target/arm: Implement FMOPA, FMOPS (non-widening)")
Signed-off-by: Daniyal Khan <danikhan632@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20240717060149.204788-2-richard.henderson@linaro.org
[rth: Split from a larger patch]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
(cherry picked from commit 31d93fedf41c24b0badb38cd9317590d1ef74e37)
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
target/arm/tcg/sme_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/arm/tcg/sme_helper.c b/target/arm/tcg/sme_helper.c
index e2e0575039..5a6dd76489 100644
--- a/target/arm/tcg/sme_helper.c
+++ b/target/arm/tcg/sme_helper.c
@@ -916,7 +916,7 @@ void HELPER(sme_fmopa_s)(void *vza, void *vzn, void *vzm, void *vpn,
if (pb & 1) {
uint32_t *a = vza_row + H1_4(col);
uint32_t *m = vzm + H1_4(col);
- *a = float32_muladd(n, *m, *a, 0, vst);
+ *a = float32_muladd(n, *m, *a, 0, &fpst);
}
col += 4;
pb >>= 4;

View File

@ -0,0 +1,62 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Richard Henderson <richard.henderson@linaro.org>
Date: Wed, 17 Jul 2024 16:01:48 +1000
Subject: [PATCH] target/arm: Use FPST_F16 for SME FMOPA (widening)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This operation has float16 inputs and thus must use
the FZ16 control not the FZ control.
Cc: qemu-stable@nongnu.org
Fixes: 3916841ac75 ("target/arm: Implement FMOPA, FMOPS (widening)")
Reported-by: Daniyal Khan <danikhan632@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20240717060149.204788-3-richard.henderson@linaro.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2374
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
(cherry picked from commit 207d30b5fdb5b45a36f26eefcf52fe2c1714dd4f)
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
target/arm/tcg/translate-sme.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/target/arm/tcg/translate-sme.c b/target/arm/tcg/translate-sme.c
index 46c7fce8b4..185a8a917b 100644
--- a/target/arm/tcg/translate-sme.c
+++ b/target/arm/tcg/translate-sme.c
@@ -304,6 +304,7 @@ static bool do_outprod(DisasContext *s, arg_op *a, MemOp esz,
}
static bool do_outprod_fpst(DisasContext *s, arg_op *a, MemOp esz,
+ ARMFPStatusFlavour e_fpst,
gen_helper_gvec_5_ptr *fn)
{
int svl = streaming_vec_reg_size(s);
@@ -319,15 +320,18 @@ static bool do_outprod_fpst(DisasContext *s, arg_op *a, MemOp esz,
zm = vec_full_reg_ptr(s, a->zm);
pn = pred_full_reg_ptr(s, a->pn);
pm = pred_full_reg_ptr(s, a->pm);
- fpst = fpstatus_ptr(FPST_FPCR);
+ fpst = fpstatus_ptr(e_fpst);
fn(za, zn, zm, pn, pm, fpst, tcg_constant_i32(desc));
return true;
}
-TRANS_FEAT(FMOPA_h, aa64_sme, do_outprod_fpst, a, MO_32, gen_helper_sme_fmopa_h)
-TRANS_FEAT(FMOPA_s, aa64_sme, do_outprod_fpst, a, MO_32, gen_helper_sme_fmopa_s)
-TRANS_FEAT(FMOPA_d, aa64_sme_f64f64, do_outprod_fpst, a, MO_64, gen_helper_sme_fmopa_d)
+TRANS_FEAT(FMOPA_h, aa64_sme, do_outprod_fpst, a,
+ MO_32, FPST_FPCR_F16, gen_helper_sme_fmopa_h)
+TRANS_FEAT(FMOPA_s, aa64_sme, do_outprod_fpst, a,
+ MO_32, FPST_FPCR, gen_helper_sme_fmopa_s)
+TRANS_FEAT(FMOPA_d, aa64_sme_f64f64, do_outprod_fpst, a,
+ MO_64, FPST_FPCR, gen_helper_sme_fmopa_d)
/* TODO: FEAT_EBF16 */
TRANS_FEAT(BFMOPA, aa64_sme, do_outprod, a, MO_32, gen_helper_sme_bfmopa)

View File

@ -0,0 +1,60 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Wed, 10 Jul 2024 17:25:29 +0200
Subject: [PATCH] scsi: fix regression and honor bootindex again for legacy
drives
Commit 3089637461 ("scsi: Don't ignore most usb-storage properties")
removed the call to object_property_set_int() and thus the 'set'
method for the bootindex property was also not called anymore. Here
that method is device_set_bootindex() (as configured by
scsi_dev_instance_init() -> device_add_bootindex_property()) which as
a side effect registers the device via add_boot_device_path().
As reported by a downstream user [0], the bootindex property did not
have the desired effect anymore for legacy drives. Fix the regression
by explicitly calling the add_boot_device_path() function after
checking that the bootindex is not yet used (to avoid
add_boot_device_path() calling exit()).
[0]: https://forum.proxmox.com/threads/149772/post-679433
Cc: qemu-stable@nongnu.org
Fixes: 3089637461 ("scsi: Don't ignore most usb-storage properties")
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.kernel.org/r/20240710152529.1737407-1-f.ebner@proxmox.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 57a8a80d1a5b28797b21d30bfc60601945820e51)
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
hw/scsi/scsi-bus.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 9e40b0c920..53eff5dd3d 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -384,6 +384,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
DeviceState *dev;
SCSIDevice *s;
DriveInfo *dinfo;
+ Error *local_err = NULL;
if (blk_is_sg(blk)) {
driver = "scsi-generic";
@@ -403,6 +404,14 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
s = SCSI_DEVICE(dev);
s->conf = *conf;
+ check_boot_index(conf->bootindex, &local_err);
+ if (local_err) {
+ object_unparent(OBJECT(dev));
+ error_propagate(errp, local_err);
+ return NULL;
+ }
+ add_boot_device_path(conf->bootindex, dev, NULL);
+
qdev_prop_set_uint32(dev, "scsi-id", unit);
if (object_property_find(OBJECT(dev), "removable")) {
qdev_prop_set_bit(dev, "removable", removable);

View File

@ -0,0 +1,48 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 15 Jul 2024 15:14:03 +0200
Subject: [PATCH] hw/scsi/lsi53c895a: bump instruction limit in scripts
processing to fix regression
Commit 9876359990 ("hw/scsi/lsi53c895a: add timer to scripts
processing") reduced the maximum allowed instruction count by
a factor of 100 all the way down to 100.
This causes the "Check Point R81.20 Gaia" appliance [0] to fail to
boot after fully finishing the installation via the appliance's web
interface (there is already one reboot before that).
With a limit of 150, the appliance still fails to boot, while with a
limit of 200, it works. Bump to 500 to fix the regression and be on
the safe side.
Originally reported in the Proxmox community forum[1].
[0]: https://support.checkpoint.com/results/download/124397
[1]: https://forum.proxmox.com/threads/149772/post-683459
Cc: qemu-stable@nongnu.org
Fixes: 9876359990 ("hw/scsi/lsi53c895a: add timer to scripts processing")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Acked-by: Sven Schnelle <svens@stackframe.org>
Link: https://lore.kernel.org/r/20240715131403.223239-1-f.ebner@proxmox.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit a4975023fb13cf229bd59c9ceec1b8cbdc5b9a20)
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
hw/scsi/lsi53c895a.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index eb9828dd5e..f1935e5328 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -188,7 +188,7 @@ static const char *names[] = {
#define LSI_TAG_VALID (1 << 16)
/* Maximum instructions to process. */
-#define LSI_MAX_INSN 100
+#define LSI_MAX_INSN 500
typedef struct lsi_request {
SCSIRequest *req;

View File

@ -0,0 +1,38 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Thu, 27 Jun 2024 20:12:44 +0200
Subject: [PATCH] block-copy: Fix missing graph lock
The graph lock needs to be held when calling bdrv_co_pdiscard(). Fix
block_copy_task_entry() to take it for the call.
WITH_GRAPH_RDLOCK_GUARD() was implemented in a weak way because of
limitations in clang's Thread Safety Analysis at the time, so that it
only asserts that the lock is held (which allows calling functions that
require the lock), but we never deal with the unlocking (so even after
the scope of the guard, the compiler assumes that the lock is still
held). This is why the compiler didn't catch this locking error.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
(picked from https://lore.kernel.org/qemu-devel/20240627181245.281403-2-kwolf@redhat.com/)
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
block/block-copy.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index 7e3b378528..cc618e4561 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -595,7 +595,9 @@ static coroutine_fn int block_copy_task_entry(AioTask *task)
if (s->discard_source && ret == 0) {
int64_t nbytes =
MIN(t->req.offset + t->req.bytes, s->len) - t->req.offset;
- bdrv_co_pdiscard(s->source, t->req.offset, nbytes);
+ WITH_GRAPH_RDLOCK_GUARD() {
+ bdrv_co_pdiscard(s->source, t->req.offset, nbytes);
+ }
}
return ret;

View File

@ -0,0 +1,93 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Sergey Dyasli <sergey.dyasli@nutanix.com>
Date: Fri, 12 Jul 2024 09:26:59 +0000
Subject: [PATCH] Revert "qemu-char: do not operate on sources from finalize
callbacks"
This reverts commit 2b316774f60291f57ca9ecb6a9f0712c532cae34.
After 038b4217884c ("Revert "chardev: use a child source for qio input
source"") we've been observing the "iwp->src == NULL" assertion
triggering periodically during the initial capabilities querying by
libvirtd. One of possible backtraces:
Thread 1 (Thread 0x7f16cd4f0700 (LWP 43858)):
0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
1 0x00007f16c6c21e65 in __GI_abort () at abort.c:79
2 0x00007f16c6c21d39 in __assert_fail_base at assert.c:92
3 0x00007f16c6c46e86 in __GI___assert_fail (assertion=assertion@entry=0x562e9bcdaadd "iwp->src == NULL", file=file@entry=0x562e9bcdaac8 "../chardev/char-io.c", line=line@entry=99, function=function@entry=0x562e9bcdab10 <__PRETTY_FUNCTION__.20549> "io_watch_poll_finalize") at assert.c:101
4 0x0000562e9ba20c2c in io_watch_poll_finalize (source=<optimized out>) at ../chardev/char-io.c:99
5 io_watch_poll_finalize (source=<optimized out>) at ../chardev/char-io.c:88
6 0x00007f16c904aae0 in g_source_unref_internal () from /lib64/libglib-2.0.so.0
7 0x00007f16c904baf9 in g_source_destroy_internal () from /lib64/libglib-2.0.so.0
8 0x0000562e9ba20db0 in io_remove_watch_poll (source=0x562e9d6720b0) at ../chardev/char-io.c:147
9 remove_fd_in_watch (chr=chr@entry=0x562e9d5f3800) at ../chardev/char-io.c:153
10 0x0000562e9ba23ffb in update_ioc_handlers (s=0x562e9d5f3800) at ../chardev/char-socket.c:592
11 0x0000562e9ba2072f in qemu_chr_fe_set_handlers_full at ../chardev/char-fe.c:279
12 0x0000562e9ba207a9 in qemu_chr_fe_set_handlers at ../chardev/char-fe.c:304
13 0x0000562e9ba2ca75 in monitor_qmp_setup_handlers_bh (opaque=0x562e9d4c2c60) at ../monitor/qmp.c:509
14 0x0000562e9bb6222e in aio_bh_poll (ctx=ctx@entry=0x562e9d4c2f20) at ../util/async.c:216
15 0x0000562e9bb4de0a in aio_poll (ctx=0x562e9d4c2f20, blocking=blocking@entry=true) at ../util/aio-posix.c:722
16 0x0000562e9b99dfaa in iothread_run (opaque=0x562e9d4c26f0) at ../iothread.c:63
17 0x0000562e9bb505a4 in qemu_thread_start (args=0x562e9d4c7ea0) at ../util/qemu-thread-posix.c:543
18 0x00007f16c70081ca in start_thread (arg=<optimized out>) at pthread_create.c:479
19 0x00007f16c6c398d3 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
io_remove_watch_poll(), which makes sure that iwp->src is NULL, calls
g_source_destroy() which finds that iwp->src is not NULL in the finalize
callback. This can only happen if another thread has managed to trigger
io_watch_poll_prepare() callback in the meantime.
Move iwp->src destruction back to the finalize callback to prevent the
described race, and also remove the stale comment. The deadlock glib bug
was fixed back in 2010 by b35820285668 ("gmain: move finalization of
GSource outside of context lock").
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Sergey Dyasli <sergey.dyasli@nutanix.com>
Link: https://lore.kernel.org/r/20240712092659.216206-1-sergey.dyasli@nutanix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit e0bf95443ee9326d44031373420cf9f3513ee255)
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
chardev/char-io.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/chardev/char-io.c b/chardev/char-io.c
index dab77b112e..3be17b51ca 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -87,16 +87,12 @@ static gboolean io_watch_poll_dispatch(GSource *source, GSourceFunc callback,
static void io_watch_poll_finalize(GSource *source)
{
- /*
- * Due to a glib bug, removing the last reference to a source
- * inside a finalize callback causes recursive locking (and a
- * deadlock). This is not a problem inside other callbacks,
- * including dispatch callbacks, so we call io_remove_watch_poll
- * to remove this source. At this point, iwp->src must
- * be NULL, or we would leak it.
- */
IOWatchPoll *iwp = io_watch_poll_from_source(source);
- assert(iwp->src == NULL);
+ if (iwp->src) {
+ g_source_destroy(iwp->src);
+ g_source_unref(iwp->src);
+ iwp->src = NULL;
+ }
}
static GSourceFuncs io_watch_poll_funcs = {
@@ -139,11 +135,6 @@ static void io_remove_watch_poll(GSource *source)
IOWatchPoll *iwp;
iwp = io_watch_poll_from_source(source);
- if (iwp->src) {
- g_source_destroy(iwp->src);
- g_source_unref(iwp->src);
- iwp->src = NULL;
- }
g_source_destroy(&iwp->parent);
}

View File

@ -119,10 +119,10 @@ index 43bc0bd520..60e98c87f1 100644
};
return raw_co_create(&options, errp);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0902b0a024..0653c244cf 100644
index 905da8be72..3db587a6e4 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4952,6 +4952,10 @@
@@ -4956,6 +4956,10 @@
# @extent-size-hint: Extent size hint to add to the image file; 0 for
# not adding an extent size hint (default: 1 MB, since 5.1)
#
@ -133,7 +133,7 @@ index 0902b0a024..0653c244cf 100644
# Since: 2.12
##
{ 'struct': 'BlockdevCreateOptionsFile',
@@ -4959,7 +4963,8 @@
@@ -4963,7 +4967,8 @@
'size': 'size',
'*preallocation': 'PreallocMode',
'*nocow': 'bool',

View File

@ -25,7 +25,7 @@ Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index ec29d6b810..270957c0cd 100644
index 3dd2e229d2..eba5b11493 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -237,8 +237,8 @@ static void backup_init_bcs_bitmap(BackupBlockJob *job)
@ -48,7 +48,7 @@ index ec29d6b810..270957c0cd 100644
if (s->sync_mode == MIRROR_SYNC_MODE_TOP) {
int64_t offset = 0;
int64_t count;
@@ -501,6 +499,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
@@ -502,6 +500,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
&error_abort);
bdrv_graph_wrunlock();

View File

@ -199,7 +199,7 @@ index 0000000000..e46abf1070
+ return bs;
+}
diff --git a/block/backup.c b/block/backup.c
index 270957c0cd..16d611c4ca 100644
index eba5b11493..1963e47ab9 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -29,28 +29,6 @@
@ -231,7 +231,7 @@ index 270957c0cd..16d611c4ca 100644
static const BlockJobDriver backup_job_driver;
static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret)
@@ -461,6 +439,14 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
@@ -462,6 +440,14 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
}
cluster_size = block_copy_cluster_size(bcs);

View File

@ -167,7 +167,7 @@ index d954bec6f1..5000c084c5 100644
+ hmp_handle_error(mon, error);
+}
diff --git a/blockdev.c b/blockdev.c
index d27d8c38ec..5e5dbc1da9 100644
index ed8198f351..1054a69279 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -37,6 +37,7 @@
@ -1683,7 +1683,7 @@ index 0000000000..c755bf302b
+ return ret;
+}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0653c244cf..dbd5d9b993 100644
index 3db587a6e4..d05fffce1d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -851,6 +851,239 @@

View File

@ -368,10 +368,10 @@ index 6de51c34cb..3bc039f60f 100644
summary_info += {'libdaxctl support': libdaxctl}
summary_info += {'libudev': libudev}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index dbd5d9b993..e79775656c 100644
index d05fffce1d..e7cf3d94f3 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3453,6 +3453,7 @@
@@ -3457,6 +3457,7 @@
'parallels', 'preallocate', 'qcow', 'qcow2', 'qed', 'quorum',
'raw', 'rbd',
{ 'name': 'replication', 'if': 'CONFIG_REPLICATION' },
@ -379,7 +379,7 @@ index dbd5d9b993..e79775656c 100644
'ssh', 'throttle', 'vdi', 'vhdx',
{ 'name': 'virtio-blk-vfio-pci', 'if': 'CONFIG_BLKIO' },
{ 'name': 'virtio-blk-vhost-user', 'if': 'CONFIG_BLKIO' },
@@ -3539,6 +3540,33 @@
@@ -3543,6 +3544,33 @@
{ 'struct': 'BlockdevOptionsNull',
'data': { '*size': 'int', '*latency-ns': 'uint64', '*read-zeroes': 'bool' } }
@ -413,7 +413,7 @@ index dbd5d9b993..e79775656c 100644
##
# @BlockdevOptionsNVMe:
#
@@ -4973,6 +5001,7 @@
@@ -4977,6 +5005,7 @@
'nfs': 'BlockdevOptionsNfs',
'null-aio': 'BlockdevOptionsNull',
'null-co': 'BlockdevOptionsNull',

View File

@ -186,7 +186,7 @@ index c755bf302b..5ebb6a3947 100644
ret->pbs_masterkey = true;
ret->backup_max_workers = true;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index e79775656c..cb58a664ef 100644
index e7cf3d94f3..282e2e8a8c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1004,6 +1004,11 @@

View File

@ -25,7 +25,7 @@ Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index 7e3b378528..adb1cbb440 100644
index cc618e4561..12d662e9d4 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -310,6 +310,7 @@ void block_copy_set_copy_opts(BlockCopyState *s, bool use_copy_range,

23
debian/patches/series vendored
View File

@ -4,6 +4,17 @@ extra/0003-ide-avoid-potential-deadlock-when-draining-during-tr.patch
extra/0004-Revert-x86-acpi-workaround-Windows-not-handling-name.patch
extra/0005-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
extra/0006-Revert-virtio-pci-fix-use-of-a-released-vector.patch
extra/0007-block-copy-before-write-fix-permission.patch
extra/0008-block-copy-before-write-support-unligned-snapshot-di.patch
extra/0009-block-copy-before-write-create-block_copy-bitmap-in-.patch
extra/0010-qapi-blockdev-backup-add-discard-source-parameter.patch
extra/0011-hw-virtio-Fix-the-de-initialization-of-vhost-user-de.patch
extra/0012-target-arm-Use-float_status-copy-in-sme_fmopa_s.patch
extra/0013-target-arm-Use-FPST_F16-for-SME-FMOPA-widening.patch
extra/0014-scsi-fix-regression-and-honor-bootindex-again-for-le.patch
extra/0015-hw-scsi-lsi53c895a-bump-instruction-limit-in-scripts.patch
extra/0016-block-copy-Fix-missing-graph-lock.patch
extra/0017-Revert-qemu-char-do-not-operate-on-sources-from-fina.patch
bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch
bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch
bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch
@ -53,11 +64,7 @@ pve/0040-Revert-block-rbd-fix-handling-of-holes-in-.bdrv_co_b.patch
pve/0041-Revert-block-rbd-implement-bdrv_co_block_status.patch
pve/0042-alloc-track-error-out-when-auto-remove-is-not-set.patch
pve/0043-alloc-track-avoid-seemingly-superfluous-child-permis.patch
pve/0044-block-copy-before-write-fix-permission.patch
pve/0045-block-copy-before-write-support-unligned-snapshot-di.patch
pve/0046-block-copy-before-write-create-block_copy-bitmap-in-.patch
pve/0047-qapi-blockdev-backup-add-discard-source-parameter.patch
pve/0048-copy-before-write-allow-specifying-minimum-cluster-s.patch
pve/0049-backup-add-minimum-cluster-size-to-performance-optio.patch
pve/0050-PVE-backup-add-fleecing-option.patch
pve/0051-PVE-backup-improve-error-when-copy-before-write-fail.patch
pve/0044-copy-before-write-allow-specifying-minimum-cluster-s.patch
pve/0045-backup-add-minimum-cluster-size-to-performance-optio.patch
pve/0046-PVE-backup-add-fleecing-option.patch
pve/0047-PVE-backup-improve-error-when-copy-before-write-fail.patch