Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 include/hw/virtio/vhost.h |  1 +
 hw/virtio/vhost.c         | 17 +++++++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 93cc3f1ae3..ef920a8076 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -84,6 +84,7 @@ struct vhost_dev {
     uint64_t backend_cap;
     bool started;
     bool log_enabled;
+    bool sw_lm_enabled;
     uint64_t log_size;
     VhostShadowVirtqueue *sw_lm_shadow_vq[2];
     VirtIOHandleOutput sw_lm_vq_handler;
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index a55b684b5f..1d55e26d45 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -988,11 +988,16 @@ static int vhost_sw_live_migration_start(struct vhost_dev *dev)
 static int vhost_sw_live_migration_enable(struct vhost_dev *dev,
                                           bool enable_lm)
 {
+    int r;
+
     if (enable_lm) {
-        return vhost_sw_live_migration_start(dev);
+        r = vhost_sw_live_migration_start(dev);
     } else {
-        return vhost_sw_live_migration_stop(dev);
+        r = vhost_sw_live_migration_stop(dev);
     }
+
+    dev->sw_lm_enabled = enable_lm;
+    return r;
 }
 
 static void vhost_sw_lm_global_start(MemoryListener *listener)
@@ -1466,6 +1471,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
     hdev->log = NULL;
     hdev->log_size = 0;
     hdev->log_enabled = false;
+    hdev->sw_lm_enabled = false;
     hdev->started = false;
     memory_listener_register(&hdev->memory_listener, &address_space_memory);
     QLIST_INSERT_HEAD(&vhost_devices, hdev, entry);
@@ -1571,6 +1577,13 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
     BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
     int i, r;
 
+    if (hdev->sw_lm_enabled) {
+        /* We've been called after migration is completed, so no need to
+           disable it again
+        */
+        return;
+    }
+
     for (i = 0; i < hdev->nvqs; ++i) {
         r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i,
                                          false);
-- 
2.18.4

On Fri, Nov 20, 2020 at 07:50:46PM +0100, Eugenio Pérez wrote:
> @@ -1571,6 +1577,13 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
>      BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
>      int i, r;
>  
> +    if (hdev->sw_lm_enabled) {
> +        /* We've been called after migration is completed, so no need to
> +           disable it again
> +        */
> +        return;
> +    }
> +
>      for (i = 0; i < hdev->nvqs; ++i) {
>          r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i,
>                                           false);

What is the purpose of this?

On Tue, Dec 8, 2020 at 8:21 AM Stefan Hajnoczi <stefanha@gmail.com> wrote:
>
> On Fri, Nov 20, 2020 at 07:50:46PM +0100, Eugenio Pérez wrote:
> > @@ -1571,6 +1577,13 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
> >      BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
> >      int i, r;
> >
> > +    if (hdev->sw_lm_enabled) {
> > +        /* We've been called after migration is completed, so no need to
> > +           disable it again
> > +        */
> > +        return;
> > +    }
> > +
> >      for (i = 0; i < hdev->nvqs; ++i) {
> >          r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i,
> >                                           false);
>
> What is the purpose of this?

It is again a quick hack to get shadow_vq POC working. Again, it
deserves a better comment :).

If I recall correctly, vhost-net calls vhost_dev_disable_notifiers
again on destruction, and it calls to memory_region_del_eventfd, then
virtio_pci_ioeventfd_assign, which is not safe to call again because
of the i != mr->ioeventfd_nb assertion.

The right fix for this should be either in virtio-pci (more generic,
but not sure if calling it again is the expected semantic of it),
individual vhost devices (less generic) or where it is at this moment,
but with the right comment.

Thanks!