Patchwork [PULL,30/34] virtio-bus: have callers tolerate new host notifier api

login
register
mail settings
Submitter Michael S. Tsirkin
Date June 24, 2016, 5:55 a.m.
Message ID <20160624085550-mutt-send-email-mst@redhat.com>
Download mbox | patch
Permalink /patch/39601/
State New
Headers show

Comments

Michael S. Tsirkin - June 24, 2016, 5:55 a.m.
From: Cornelia Huck <cornelia.huck@de.ibm.com>

Have vhost and dataplane use the new api for transports that
have been converted.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 14 +++++++++++---
 hw/scsi/virtio-scsi-dataplane.c | 20 +++++++++++++++-----
 hw/virtio/vhost.c               | 20 ++++++++++++++++----
 3 files changed, 42 insertions(+), 12 deletions(-)
Marc-André Lureau - June 29, 2016, 11:37 a.m.
Hi

On Fri, Jun 24, 2016 at 7:55 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> From: Cornelia Huck <cornelia.huck@de.ibm.com>
>
> Have vhost and dataplane use the new api for transports that
> have been converted.
>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Reviewed-by: Fam Zheng <famz@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---

This patch and further break vhost-user-test:

QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64  tests/vhost-user-test
/x86_64/vhost-user/read-guest-mem: binding does not support host notifiers
qemu-system-x86_64: unable to start vhost net: 38: falling back on
userspace virtio
**
ERROR:tests/vhost-user-test.c:162:wait_for_fds: assertion failed: (s->fds_num)
Aborted (core dumped)

(I wonder why it wasn't noticied)

>  hw/block/dataplane/virtio-blk.c | 14 +++++++++++---
>  hw/scsi/virtio-scsi-dataplane.c | 20 +++++++++++++++-----
>  hw/virtio/vhost.c               | 20 ++++++++++++++++----
>  3 files changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> index 2073f9a..fdf5fd1 100644
> --- a/hw/block/dataplane/virtio-blk.c
> +++ b/hw/block/dataplane/virtio-blk.c
> @@ -79,7 +79,8 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
>      }
>
>      /* Don't try if transport does not support notifiers. */
> -    if (!k->set_guest_notifiers || !k->set_host_notifier) {
> +    if (!k->set_guest_notifiers ||
> +        (!k->set_host_notifier && !k->ioeventfd_started)) {
>          error_setg(errp,
>                     "device is incompatible with dataplane "
>                     "(transport does not support notifiers)");
> @@ -157,7 +158,10 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>      s->guest_notifier = virtio_queue_get_guest_notifier(s->vq);
>
>      /* Set up virtqueue notify */
> -    r = k->set_host_notifier(qbus->parent, 0, true);
> +    r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), 0, true);
> +    if (r == -ENOSYS) {
> +        r = k->set_host_notifier(qbus->parent, 0, true);
> +    }
>      if (r != 0) {
>          fprintf(stderr, "virtio-blk failed to set host notifier (%d)\n", r);
>          goto fail_host_notifier;
> @@ -193,6 +197,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
>      BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s->vdev)));
>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>      VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
> +    int r;
>
>      if (!vblk->dataplane_started || s->stopping) {
>          return;
> @@ -217,7 +222,10 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
>
>      aio_context_release(s->ctx);
>
> -    k->set_host_notifier(qbus->parent, 0, false);
> +    r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), 0, false);
> +    if (r == -ENOSYS) {
> +        k->set_host_notifier(qbus->parent, 0, false);
> +    }
>
>      /* Clean up guest notifier (irq) */
>      k->set_guest_notifiers(qbus->parent, 1, false);
> diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
> index 1a49f1e..b9a5716 100644
> --- a/hw/scsi/virtio-scsi-dataplane.c
> +++ b/hw/scsi/virtio-scsi-dataplane.c
> @@ -31,7 +31,8 @@ void virtio_scsi_set_iothread(VirtIOSCSI *s, IOThread *iothread)
>      s->ctx = iothread_get_aio_context(vs->conf.iothread);
>
>      /* Don't try if transport does not support notifiers. */
> -    if (!k->set_guest_notifiers || !k->set_host_notifier) {
> +    if (!k->set_guest_notifiers ||
> +        (!k->set_host_notifier && !k->ioeventfd_started)) {
>          fprintf(stderr, "virtio-scsi: Failed to set iothread "
>                     "(transport does not support notifiers)");
>          exit(1);
> @@ -73,7 +74,10 @@ static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue *vq, int n,
>      int rc;
>
>      /* Set up virtqueue notify */
> -    rc = k->set_host_notifier(qbus->parent, n, true);
> +    rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), n, true);
> +    if (rc == -ENOSYS) {
> +        rc = k->set_host_notifier(qbus->parent, n, true);
> +    }
>      if (rc != 0) {
>          fprintf(stderr, "virtio-scsi: Failed to set host notifier (%d)\n",
>                  rc);
> @@ -159,7 +163,10 @@ fail_vrings:
>      virtio_scsi_clear_aio(s);
>      aio_context_release(s->ctx);
>      for (i = 0; i < vs->conf.num_queues + 2; i++) {
> -        k->set_host_notifier(qbus->parent, i, false);
> +        rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
> +        if (rc == -ENOSYS) {
> +            k->set_host_notifier(qbus->parent, i, false);
> +        }
>      }
>      k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, false);
>  fail_guest_notifiers:
> @@ -174,7 +181,7 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
>      BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>      VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
> -    int i;
> +    int i, rc;
>
>      if (!s->dataplane_started || s->dataplane_stopping) {
>          return;
> @@ -198,7 +205,10 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
>      aio_context_release(s->ctx);
>
>      for (i = 0; i < vs->conf.num_queues + 2; i++) {
> -        k->set_host_notifier(qbus->parent, i, false);
> +        rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
> +        if (rc == -ENOSYS) {
> +            k->set_host_notifier(qbus->parent, i, false);
> +        }
>      }
>
>      /* Clean up guest notifier (irq) */
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 81cc5b0..bce1b6e 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1110,14 +1110,18 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
>      VirtioBusState *vbus = VIRTIO_BUS(qbus);
>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
>      int i, r, e;
> -    if (!k->set_host_notifier) {
> +    if (!k->set_host_notifier || !k->ioeventfd_started) {
>          fprintf(stderr, "binding does not support host notifiers\n");
>          r = -ENOSYS;
>          goto fail;
>      }
>
>      for (i = 0; i < hdev->nvqs; ++i) {
> -        r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
> +        r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i,
> +                                         true);
> +        if (r == -ENOSYS) {
> +            r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
> +        }
>          if (r < 0) {
>              fprintf(stderr, "vhost VQ %d notifier binding failed: %d\n", i, -r);
>              goto fail_vq;
> @@ -1127,7 +1131,11 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
>      return 0;
>  fail_vq:
>      while (--i >= 0) {
> -        e = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
> +        e = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i,
> +                                         false);
> +        if (e == -ENOSYS) {
> +            e = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
> +        }
>          if (e < 0) {
>              fprintf(stderr, "vhost VQ %d notifier cleanup error: %d\n", i, -r);
>              fflush(stderr);
> @@ -1151,7 +1159,11 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
>      int i, r;
>
>      for (i = 0; i < hdev->nvqs; ++i) {
> -        r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
> +        r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i,
> +                                         false);
> +        if (r == -ENOSYS) {
> +            r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
> +        }
>          if (r < 0) {
>              fprintf(stderr, "vhost VQ %d notifier cleanup failed: %d\n", i, -r);
>              fflush(stderr);
> --
> MST
>
>
Cornelia Huck - June 29, 2016, 11:48 a.m.
On Wed, 29 Jun 2016 13:37:15 +0200
Marc-André Lureau <marcandre.lureau@gmail.com> wrote:

> Hi
> 
> On Fri, Jun 24, 2016 at 7:55 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > From: Cornelia Huck <cornelia.huck@de.ibm.com>
> >
> > Have vhost and dataplane use the new api for transports that
> > have been converted.
> >
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > Reviewed-by: Fam Zheng <famz@redhat.com>
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> 
> This patch and further break vhost-user-test:
> 
> QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64  tests/vhost-user-test
> /x86_64/vhost-user/read-guest-mem: binding does not support host notifiers
> qemu-system-x86_64: unable to start vhost net: 38: falling back on
> userspace virtio
> **
> ERROR:tests/vhost-user-test.c:162:wait_for_fds: assertion failed: (s->fds_num)
> Aborted (core dumped)

Yes, there's an || that needs to be a && (too late to fix), and the
mechanism does not work for vhost (currently working on a fix; our
fallback is too good since it only prints a message but otherwise
works).

Patch

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 2073f9a..fdf5fd1 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -79,7 +79,8 @@  void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
     }
 
     /* Don't try if transport does not support notifiers. */
-    if (!k->set_guest_notifiers || !k->set_host_notifier) {
+    if (!k->set_guest_notifiers ||
+        (!k->set_host_notifier && !k->ioeventfd_started)) {
         error_setg(errp,
                    "device is incompatible with dataplane "
                    "(transport does not support notifiers)");
@@ -157,7 +158,10 @@  void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
     s->guest_notifier = virtio_queue_get_guest_notifier(s->vq);
 
     /* Set up virtqueue notify */
-    r = k->set_host_notifier(qbus->parent, 0, true);
+    r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), 0, true);
+    if (r == -ENOSYS) {
+        r = k->set_host_notifier(qbus->parent, 0, true);
+    }
     if (r != 0) {
         fprintf(stderr, "virtio-blk failed to set host notifier (%d)\n", r);
         goto fail_host_notifier;
@@ -193,6 +197,7 @@  void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
     BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s->vdev)));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
+    int r;
 
     if (!vblk->dataplane_started || s->stopping) {
         return;
@@ -217,7 +222,10 @@  void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
 
     aio_context_release(s->ctx);
 
-    k->set_host_notifier(qbus->parent, 0, false);
+    r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), 0, false);
+    if (r == -ENOSYS) {
+        k->set_host_notifier(qbus->parent, 0, false);
+    }
 
     /* Clean up guest notifier (irq) */
     k->set_guest_notifiers(qbus->parent, 1, false);
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index 1a49f1e..b9a5716 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -31,7 +31,8 @@  void virtio_scsi_set_iothread(VirtIOSCSI *s, IOThread *iothread)
     s->ctx = iothread_get_aio_context(vs->conf.iothread);
 
     /* Don't try if transport does not support notifiers. */
-    if (!k->set_guest_notifiers || !k->set_host_notifier) {
+    if (!k->set_guest_notifiers ||
+        (!k->set_host_notifier && !k->ioeventfd_started)) {
         fprintf(stderr, "virtio-scsi: Failed to set iothread "
                    "(transport does not support notifiers)");
         exit(1);
@@ -73,7 +74,10 @@  static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue *vq, int n,
     int rc;
 
     /* Set up virtqueue notify */
-    rc = k->set_host_notifier(qbus->parent, n, true);
+    rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), n, true);
+    if (rc == -ENOSYS) {
+        rc = k->set_host_notifier(qbus->parent, n, true);
+    }
     if (rc != 0) {
         fprintf(stderr, "virtio-scsi: Failed to set host notifier (%d)\n",
                 rc);
@@ -159,7 +163,10 @@  fail_vrings:
     virtio_scsi_clear_aio(s);
     aio_context_release(s->ctx);
     for (i = 0; i < vs->conf.num_queues + 2; i++) {
-        k->set_host_notifier(qbus->parent, i, false);
+        rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
+        if (rc == -ENOSYS) {
+            k->set_host_notifier(qbus->parent, i, false);
+        }
     }
     k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, false);
 fail_guest_notifiers:
@@ -174,7 +181,7 @@  void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
     BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
-    int i;
+    int i, rc;
 
     if (!s->dataplane_started || s->dataplane_stopping) {
         return;
@@ -198,7 +205,10 @@  void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
     aio_context_release(s->ctx);
 
     for (i = 0; i < vs->conf.num_queues + 2; i++) {
-        k->set_host_notifier(qbus->parent, i, false);
+        rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
+        if (rc == -ENOSYS) {
+            k->set_host_notifier(qbus->parent, i, false);
+        }
     }
 
     /* Clean up guest notifier (irq) */
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 81cc5b0..bce1b6e 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1110,14 +1110,18 @@  int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
     VirtioBusState *vbus = VIRTIO_BUS(qbus);
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
     int i, r, e;
-    if (!k->set_host_notifier) {
+    if (!k->set_host_notifier || !k->ioeventfd_started) {
         fprintf(stderr, "binding does not support host notifiers\n");
         r = -ENOSYS;
         goto fail;
     }
 
     for (i = 0; i < hdev->nvqs; ++i) {
-        r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
+        r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i,
+                                         true);
+        if (r == -ENOSYS) {
+            r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
+        }
         if (r < 0) {
             fprintf(stderr, "vhost VQ %d notifier binding failed: %d\n", i, -r);
             goto fail_vq;
@@ -1127,7 +1131,11 @@  int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
     return 0;
 fail_vq:
     while (--i >= 0) {
-        e = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
+        e = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i,
+                                         false);
+        if (e == -ENOSYS) {
+            e = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
+        }
         if (e < 0) {
             fprintf(stderr, "vhost VQ %d notifier cleanup error: %d\n", i, -r);
             fflush(stderr);
@@ -1151,7 +1159,11 @@  void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
     int i, r;
 
     for (i = 0; i < hdev->nvqs; ++i) {
-        r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
+        r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i,
+                                         false);
+        if (r == -ENOSYS) {
+            r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
+        }
         if (r < 0) {
             fprintf(stderr, "vhost VQ %d notifier cleanup failed: %d\n", i, -r);
             fflush(stderr);