mirror of
https://git.proxmox.com/git/pve-qemu
synced 2025-08-16 00:06:27 +00:00
441072fc57
Commit f06b222 ("fixes for QEMU 9.0") included a revert for the QEMU
commit 2ce6cff94d ("virtio-pci: fix use of a released vector"). That
commit caused some regressions which sounded just as bad as the fix.
Those regressions have now been addressed upstream, so pick up the fix
and drop the revert. Dropping the revert fixes the original issue that
commit 2ce6cff94d ("virtio-pci: fix use of a released vector")
addressed.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
93 lines
3.7 KiB
Diff
93 lines
3.7 KiB
Diff
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)
|