On 10/30/2016 10:23 PM, Michael S. Tsirkin wrote:
> From: Paolo Bonzini <pbonz...@redhat.com>
>
> Now that there is not anymore a switch from the generic ioeventfd handler
> to the dataplane handler, virtio_bus_set_host_notifier(assign=true) is
> always called with !bus->ioeventfd_started, hence virtio_bus_stop_ioeventfd
> does nothing in this case. Move the invocation to vhost.c, which is the
> only place that needs it.
>
> Reviewed-by: Cornelia Huck <cornelia.h...@de.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> ---
> include/hw/virtio/virtio-bus.h | 6 ------
> hw/virtio/vhost.c | 3 +++
> hw/virtio/virtio-bus.c | 23 ++++++++---------------
> 3 files changed, 11 insertions(+), 21 deletions(-)
This breaks vhost-net for s390/kvm after rebooting the guest. (ping fails and
ifconfig shows no packets is TXed)
Any idea?
>
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index af6b5c4..cbdf745 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -94,12 +94,6 @@ struct VirtioBusState {
> BusState parent_obj;
>
> /*
> - * Set if the default ioeventfd handlers are disabled by vhost
> - * or dataplane.
> - */
> - bool ioeventfd_disabled;
> -
> - /*
> * Set if ioeventfd has been started.
> */
> bool ioeventfd_started;
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 501a5f4..131f164 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1196,6 +1196,7 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev,
> VirtIODevice *vdev)
> goto fail;
> }
>
> + virtio_device_stop_ioeventfd(vdev);
> for (i = 0; i < hdev->nvqs; ++i) {
> r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +
> i,
> true);
> @@ -1215,6 +1216,7 @@ fail_vq:
> }
> assert (e >= 0);
> }
> + virtio_device_start_ioeventfd(vdev);
> fail:
> return r;
> }
> @@ -1237,6 +1239,7 @@ void vhost_dev_disable_notifiers(struct vhost_dev
> *hdev, VirtIODevice *vdev)
> }
> assert (r >= 0);
> }
> + virtio_device_start_ioeventfd(vdev);
> }
>
> /* Test and clear event pending status.
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index a619445..b0e4544 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -190,7 +190,7 @@ int virtio_bus_start_ioeventfd(VirtioBusState *bus)
> if (!k->ioeventfd_assign || !k->ioeventfd_enabled(proxy)) {
> return -ENOSYS;
> }
> - if (bus->ioeventfd_started || bus->ioeventfd_disabled) {
> + if (bus->ioeventfd_started) {
> return 0;
> }
> r = vdc->start_ioeventfd(vdev);
> @@ -226,8 +226,8 @@ bool virtio_bus_ioeventfd_enabled(VirtioBusState *bus)
> }
>
> /*
> - * This function switches from/to the generic ioeventfd handler.
> - * assign==false means 'use generic ioeventfd handler'.
> + * This function switches ioeventfd on/off in the device.
> + * The caller must set or clear the handlers for the EventNotifier.
> */
> int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
> {
> @@ -237,19 +237,12 @@ int virtio_bus_set_host_notifier(VirtioBusState *bus,
> int n, bool assign)
> if (!k->ioeventfd_assign) {
> return -ENOSYS;
> }
> - bus->ioeventfd_disabled = assign;
> if (assign) {
> - /*
> - * Stop using the generic ioeventfd, we are doing eventfd handling
> - * ourselves below
> - *
> - * FIXME: We should just switch the handler and not deassign the
> - * ioeventfd.
> - * Otherwise, there's a window where we don't have an
> - * ioeventfd and we may end up with a notification where
> - * we don't expect one.
> - */
> - virtio_bus_stop_ioeventfd(bus);
> + assert(!bus->ioeventfd_started);
> + } else {
> + if (!bus->ioeventfd_started) {
> + return 0;
> + }
> }
> return set_host_notifier_internal(proxy, bus, n, assign);
> }
>
