[Qemu-devel] [PULL v2 0/3] virtio: quick fix

[Qemu-devel] [PULL v2 0/3] virtio: quick fix

[Michael S. Tsirkin]

So this just reverts Gal's patches. I hope he'll be able to resubmit
shortly but it's causing too many issues to too many people to try to do
incremental fixups, and this way there's no timing pressure on anyone.

The following changes since commit 52483b067cce4a88ffbf8fbeea26de7549d2ad23:

  Merge remote-tracking branch 'remotes/huth/tags/pull-request-2018-01-22' into staging (2018-01-23 10:15:09 +0000)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to e044c0f8dd028ea296396ad8066ba436e9acf350:

  Revert "qemu: add a cleanup callback function to EventNotifier" (2018-01-24 19:20:19 +0200)

----------------------------------------------------------------
virtio: quick fix

Fixes a regression in virtio that's causing issues
for many people.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Michael S. Tsirkin (3):
      Revert "virtio: improve virtio devices initialization time"
      Revert "virtio: postpone the execution of event_notifier_cleanup function"
      Revert "qemu: add a cleanup callback function to EventNotifier"

 include/qemu/event_notifier.h |  1 -
 accel/kvm/kvm-all.c           |  4 ----
 hw/virtio/virtio-bus.c        | 19 ++++++++-----------
 hw/virtio/virtio.c            |  5 -----
 util/event_notifier-posix.c   |  5 +----
 util/event_notifier-win32.c   |  2 --
 6 files changed, 9 insertions(+), 27 deletions(-)

[Qemu-devel] [PULL v2 1/3] Revert "virtio: improve virtio devices initialization time"

[Michael S. Tsirkin]

This reverts commit 6f0bb230722931d17fb284eee8efd40b9d653822.

This reverts commit f87d72f5c5bff0837d409a56bd34f439a90119ca as that is
reported to break cleanup and migration.

Cc: Gal Hammer <ghammer@redhat.com>
Cc: Sitong Liu <siliu@redhat.com>
Cc: Xiaoling Gao <xiagao@redhat.com>
Suggested-by: Greg Kurz <groug@kaod.org>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reported-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
Reported-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
---
 hw/virtio/virtio.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 3ac3491..d6002ee 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2574,7 +2574,6 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
     VirtioBusState *qbus = VIRTIO_BUS(qdev_get_parent_bus(DEVICE(vdev)));
     int n, r, err;
 
-    memory_region_transaction_begin();
     for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
         VirtQueue *vq = &vdev->vq[n];
         if (!virtio_queue_get_num(vdev, n)) {
@@ -2597,7 +2596,6 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
         }
         event_notifier_set(&vq->host_notifier);
     }
-    memory_region_transaction_commit();
     return 0;
 
 assign_error:
@@ -2611,7 +2609,6 @@ assign_error:
         r = virtio_bus_set_host_notifier(qbus, n, false);
         assert(r >= 0);
     }
-    memory_region_transaction_commit();
     return err;
 }
 
@@ -2628,7 +2625,6 @@ static void virtio_device_stop_ioeventfd_impl(VirtIODevice *vdev)
     VirtioBusState *qbus = VIRTIO_BUS(qdev_get_parent_bus(DEVICE(vdev)));
     int n, r;
 
-    memory_region_transaction_begin();
     for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
         VirtQueue *vq = &vdev->vq[n];
 
@@ -2639,7 +2635,6 @@ static void virtio_device_stop_ioeventfd_impl(VirtIODevice *vdev)
         r = virtio_bus_set_host_notifier(qbus, n, false);
         assert(r >= 0);
     }
-    memory_region_transaction_commit();
 }
 
 void virtio_device_stop_ioeventfd(VirtIODevice *vdev)
-- 
MST

[Qemu-devel] [PULL v2 2/3] Revert "virtio: postpone the execution of event_notifier_cleanup function"

[Michael S. Tsirkin]

This reverts commit 4fe6d78b2e241f41208dfb07605aace4becfc747 as it is
reported to break cleanup and migration.

Cc: Gal Hammer <ghammer@redhat.com>
Cc: Sitong Liu <siliu@redhat.com>
Cc: Xiaoling Gao <xiagao@redhat.com>
Suggested-by: Greg Kurz <groug@kaod.org>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reported-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
Reported-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
---
 accel/kvm/kvm-all.c    |  4 ----
 hw/virtio/virtio-bus.c | 19 ++++++++-----------
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 071f4f5..f290f48 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -812,10 +812,6 @@ static void kvm_mem_ioeventfd_del(MemoryListener *listener,
     if (r < 0) {
         abort();
     }
-
-    if (e->cleanup) {
-        e->cleanup(e);
-    }
 }
 
 static void kvm_io_ioeventfd_add(MemoryListener *listener,
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 8106346..3042232 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -256,15 +256,6 @@ bool virtio_bus_ioeventfd_enabled(VirtioBusState *bus)
     return k->ioeventfd_assign && k->ioeventfd_enabled(proxy);
 }
 
-static void virtio_bus_cleanup_event_notifier(EventNotifier *notifier)
-{
-    /* Test and clear notifier after disabling event,
-     * in case poll callback didn't have time to run.
-     */
-    virtio_queue_host_notifier_read(notifier);
-    event_notifier_cleanup(notifier);
-}
-
 /*
  * This function switches ioeventfd on/off in the device.
  * The caller must set or clear the handlers for the EventNotifier.
@@ -292,13 +283,19 @@ int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
         r = k->ioeventfd_assign(proxy, notifier, n, true);
         if (r < 0) {
             error_report("%s: unable to assign ioeventfd: %d", __func__, r);
-            virtio_bus_cleanup_event_notifier(notifier);
+            goto cleanup_event_notifier;
         }
+        return 0;
     } else {
-        notifier->cleanup = virtio_bus_cleanup_event_notifier;
         k->ioeventfd_assign(proxy, notifier, n, false);
     }
 
+cleanup_event_notifier:
+    /* Test and clear notifier after disabling event,
+     * in case poll callback didn't have time to run.
+     */
+    virtio_queue_host_notifier_read(notifier);
+    event_notifier_cleanup(notifier);
     return r;
 }
 
-- 
MST

[Qemu-devel] [PULL v2 3/3] Revert "qemu: add a cleanup callback function to EventNotifier"

[Michael S. Tsirkin]

This reverts commit f87d72f5c5bff0837d409a56bd34f439a90119ca as that is
part of a patchset reported to break cleanup and migration.

Cc: Gal Hammer <ghammer@redhat.com>
Cc: Sitong Liu <siliu@redhat.com>
Cc: Xiaoling Gao <xiagao@redhat.com>
Suggested-by: Greg Kurz <groug@kaod.org>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reported-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
Reported-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
---
 include/qemu/event_notifier.h | 1 -
 util/event_notifier-posix.c   | 5 +----
 util/event_notifier-win32.c   | 2 --
 3 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/include/qemu/event_notifier.h b/include/qemu/event_notifier.h
index b30a454..599c99f 100644
--- a/include/qemu/event_notifier.h
+++ b/include/qemu/event_notifier.h
@@ -26,7 +26,6 @@ struct EventNotifier {
     int rfd;
     int wfd;
 #endif
-    void (*cleanup)(EventNotifier *);
 };
 
 typedef void EventNotifierHandler(EventNotifier *);
diff --git a/util/event_notifier-posix.c b/util/event_notifier-posix.c
index 6525666..73c4046 100644
--- a/util/event_notifier-posix.c
+++ b/util/event_notifier-posix.c
@@ -29,7 +29,6 @@ void event_notifier_init_fd(EventNotifier *e, int fd)
 {
     e->rfd = fd;
     e->wfd = fd;
-    e->cleanup = NULL;
 }
 #endif
 
@@ -66,7 +65,6 @@ int event_notifier_init(EventNotifier *e, int active)
         e->rfd = fds[0];
         e->wfd = fds[1];
     }
-    e->cleanup = NULL;
     if (active) {
         event_notifier_set(e);
     }
@@ -82,11 +80,10 @@ void event_notifier_cleanup(EventNotifier *e)
 {
     if (e->rfd != e->wfd) {
         close(e->rfd);
+        e->rfd = -1;
     }
     close(e->wfd);
-    e->rfd = -1;
     e->wfd = -1;
-    e->cleanup = NULL;
 }
 
 int event_notifier_get_fd(const EventNotifier *e)
diff --git a/util/event_notifier-win32.c b/util/event_notifier-win32.c
index eff8670..62c53b0 100644
--- a/util/event_notifier-win32.c
+++ b/util/event_notifier-win32.c
@@ -19,7 +19,6 @@ int event_notifier_init(EventNotifier *e, int active)
 {
     e->event = CreateEvent(NULL, TRUE, FALSE, NULL);
     assert(e->event);
-    e->cleanup = NULL;
     return 0;
 }
 
@@ -27,7 +26,6 @@ void event_notifier_cleanup(EventNotifier *e)
 {
     CloseHandle(e->event);
     e->event = NULL;
-    e->cleanup = NULL;
 }
 
 HANDLE event_notifier_get_handle(EventNotifier *e)
-- 
MST

Re: [Qemu-devel] [PULL v2 0/3] virtio: quick fix

[Peter Maydell]

On 24 January 2018 at 17:22, Michael S. Tsirkin <mst@redhat.com> wrote:
> So this just reverts Gal's patches. I hope he'll be able to resubmit
> shortly but it's causing too many issues to too many people to try to do
> incremental fixups, and this way there's no timing pressure on anyone.
>
> The following changes since commit 52483b067cce4a88ffbf8fbeea26de7549d2ad23:
>
>   Merge remote-tracking branch 'remotes/huth/tags/pull-request-2018-01-22' into staging (2018-01-23 10:15:09 +0000)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to e044c0f8dd028ea296396ad8066ba436e9acf350:
>
>   Revert "qemu: add a cleanup callback function to EventNotifier" (2018-01-24 19:20:19 +0200)
>
> ----------------------------------------------------------------
> virtio: quick fix
>
> Fixes a regression in virtio that's causing issues
> for many people.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ----------------------------------------------------------------
> Michael S. Tsirkin (3):
>       Revert "virtio: improve virtio devices initialization time"
>       Revert "virtio: postpone the execution of event_notifier_cleanup function"
>       Revert "qemu: add a cleanup callback function to EventNotifier"

Applied, thanks.

-- PMM