On 7/7/2023 7:02 PM, Eugenio Perez Martin wrote:
On Fri, Jul 7, 2023 at 12:18 PM Zhu Lingshan <lingshan....@intel.com> wrote:
In the poweroff routine, no need to fetch last available index.
This commit also provides a better debug message in the vhost
caller vhost_virtqueue_stop, because if vhost does not fetch
the last avail idx successfully, maybe the device does not
suspend, vhost will sync last avail idx to vring used idx as a
work around, not a failure.
Signed-off-by: Zhu Lingshan <lingshan....@intel.com>
CCing MST.
---
hw/virtio/vhost-vdpa.c | 4 ++++
hw/virtio/vhost.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 3c575a9a6e..f62952e1c7 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -26,6 +26,7 @@
#include "cpu.h"
#include "trace.h"
#include "qapi/error.h"
+#include "sysemu/runstate.h"
/*
* Return one past the end of the end of section. Be careful with uint64_t
@@ -1391,6 +1392,9 @@ static int vhost_vdpa_get_vring_base(struct vhost_dev
*dev,
struct vhost_vdpa *v = dev->opaque;
int ret;
+ if (runstate_check(RUN_STATE_SHUTDOWN))
+ return 0;
+
QEMU coding style mandates braces around the "if" body (CODING_STYLE.rst file).
Apart from that I think we should add a comment here. Something in the line of:
Some devices do not support the call properly, and we don't need to
retrieve the indexes if we're not migrating. Skip it in this case.
V2 will include these changes, thanks!
if (v->shadow_vqs_enabled) {
ring->num = virtio_queue_get_last_avail_idx(dev->vdev, ring->index);
return 0;
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 82394331bf..84712743e0 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1262,7 +1262,7 @@ void vhost_virtqueue_stop(struct vhost_dev *dev,
r = dev->vhost_ops->vhost_get_vring_base(dev, &state);
if (r < 0) {
- VHOST_OPS_DEBUG(r, "vhost VQ %u ring restore failed: %d", idx, r);
+ VHOST_OPS_DEBUG(r, "sync last avail idx to the used idx for vhost VQ
%u", idx);
Guest's used idx? Also, maybe it is worth splitting this in a separated patch.
Yes, guest used idx is much clearer. I am not sure whether this worthy a
separated
patch, this is only a comment.
Apart from the nitpicking, I think the general line of the patch is
the way to go :).
Thanks!
Thanks!
/* Connection to the backend is broken, so let's sync internal
* last avail idx to the device used idx.
*/
--
2.39.3
