diff mbox

[1/3] virtio: interface to enable/disable host notifiers

Message ID 1428403546-31914-2-git-send-email-mst@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin April 7, 2015, 10:45 a.m. UTC 
generalize and move these from vhost to virtio.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/virtio/virtio.h |  3 +++
 hw/virtio/vhost.c          | 45 ++--------------------------------------
 hw/virtio/virtio.c         | 51 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 43 deletions(-)

Comments

Cornelia Huck April 7, 2015, 11:32 a.m. UTC | #1 
On Tue, 7 Apr 2015 12:45:56 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> generalize and move these from vhost to virtio.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  include/hw/virtio/virtio.h |  3 +++
>  hw/virtio/vhost.c          | 45 ++--------------------------------------
>  hw/virtio/virtio.c         | 51 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 56 insertions(+), 43 deletions(-)
> 

> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 17c1260..e7ee069 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1283,6 +1283,57 @@ void virtio_device_set_child_bus_name(VirtIODevice *vdev, char *bus_name)
>      vdev->bus_name = g_strdup(bus_name);
>  }
> 
> +int virtio_enable_host_notifiers(VirtIODevice *vdev, int startvq, int nvqs)

I realize that you just move the function, but would it make sense to
verify that startvq + nvqs is actually still a valid queue?

> +{
> +    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
> +    VirtioBusState *vbus = VIRTIO_BUS(qbus);
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
> +    int i, r;
> +    if (!k->set_host_notifier) {
> +        fprintf(stderr, "binding does not support host notifiers\n");
> +        r = -ENOSYS;
> +        goto fail;
> +    }
> +
> +    for (i = 0; i < nvqs; ++i) {
> +        r = k->set_host_notifier(qbus->parent, startvq + i, true);
> +        if (r < 0) {
> +            fprintf(stderr, "vhost VQ %d notifier binding failed: %d\n", i, -r);

I think you'll want to get rid of the vhost in the message here. And
the number of the failing virtqueue is startvq+i, no?

Also: would it make sense to convert this to the error reporting
infrastructure?

> +            goto fail_vq;
> +        }
> +    }
> +
> +    return 0;
> +fail_vq:
> +    while (--i >= 0) {
> +        r = k->set_host_notifier(qbus->parent, startvq + i, false);
> +        if (r < 0) {
> +            fprintf(stderr, "vhost VQ %d notifier cleanup error: %d\n", i, -r);
> +            fflush(stderr);
> +        }
> +        assert (r >= 0);
> +    }
> +fail:
> +    return r;
> +}
> +
> +void virtio_disable_host_notifiers(VirtIODevice *vdev, int startvq, int nvqs)
> +{
> +    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
> +    VirtioBusState *vbus = VIRTIO_BUS(qbus);
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
> +    int i, r;
> +
> +    for (i = 0; i < nvqs; ++i) {
> +        r = k->set_host_notifier(qbus->parent, startvq + i, false);

Should a generic function maybe check for ->set_host_notifier and bail
out early if it doesn't exist? Probably depends on how the callers want
to handle non-existence of host notifiers.

> +        if (r < 0) {
> +            fprintf(stderr, "vhost VQ %d notifier cleanup failed: %d\n", i, -r);
> +            fflush(stderr);
> +        }
> +        assert (r >= 0);
> +    }
> +}
> +
>  static void virtio_device_realize(DeviceState *dev, Error **errp)
>  {
>      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
diff mbox

Patch

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index d95f8b6..e1cff22 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -219,6 +219,9 @@  void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
 void virtio_queue_notify_vq(VirtQueue *vq);
 void virtio_irq(VirtQueue *vq);
 
+int virtio_enable_host_notifiers(VirtIODevice *vdev, int startvq, int nvqs);
+void virtio_disable_host_notifiers(VirtIODevice *vdev, int startvq, int nvqs);
+
 static inline void virtio_add_feature(uint32_t *features, unsigned int fbit)
 {
     assert(fbit < 32);
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 5a12861..8dd2f59 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -918,36 +918,7 @@  bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev)
  */
 int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
 {
-    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
-    VirtioBusState *vbus = VIRTIO_BUS(qbus);
-    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
-    int i, r;
-    if (!k->set_host_notifier) {
-        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);
-        if (r < 0) {
-            fprintf(stderr, "vhost VQ %d notifier binding failed: %d\n", i, -r);
-            goto fail_vq;
-        }
-    }
-
-    return 0;
-fail_vq:
-    while (--i >= 0) {
-        r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
-        if (r < 0) {
-            fprintf(stderr, "vhost VQ %d notifier cleanup error: %d\n", i, -r);
-            fflush(stderr);
-        }
-        assert (r >= 0);
-    }
-fail:
-    return r;
+    return virtio_enable_host_notifiers(vdev, hdev->vq_index, hdev->nvqs);
 }
 
 /* Stop processing guest IO notifications in vhost.
@@ -957,19 +928,7 @@  fail:
  */
 void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
 {
-    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
-    VirtioBusState *vbus = VIRTIO_BUS(qbus);
-    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
-    int i, r;
-
-    for (i = 0; i < hdev->nvqs; ++i) {
-        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);
-        }
-        assert (r >= 0);
-    }
+    virtio_disable_host_notifiers(vdev, hdev->vq_index, hdev->nvqs);
 }
 
 /* Test and clear event pending status.
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 17c1260..e7ee069 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1283,6 +1283,57 @@  void virtio_device_set_child_bus_name(VirtIODevice *vdev, char *bus_name)
     vdev->bus_name = g_strdup(bus_name);
 }
 
+int virtio_enable_host_notifiers(VirtIODevice *vdev, int startvq, int nvqs)
+{
+    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+    VirtioBusState *vbus = VIRTIO_BUS(qbus);
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
+    int i, r;
+    if (!k->set_host_notifier) {
+        fprintf(stderr, "binding does not support host notifiers\n");
+        r = -ENOSYS;
+        goto fail;
+    }
+
+    for (i = 0; i < nvqs; ++i) {
+        r = k->set_host_notifier(qbus->parent, startvq + i, true);
+        if (r < 0) {
+            fprintf(stderr, "vhost VQ %d notifier binding failed: %d\n", i, -r);
+            goto fail_vq;
+        }
+    }
+
+    return 0;
+fail_vq:
+    while (--i >= 0) {
+        r = k->set_host_notifier(qbus->parent, startvq + i, false);
+        if (r < 0) {
+            fprintf(stderr, "vhost VQ %d notifier cleanup error: %d\n", i, -r);
+            fflush(stderr);
+        }
+        assert (r >= 0);
+    }
+fail:
+    return r;
+}
+
+void virtio_disable_host_notifiers(VirtIODevice *vdev, int startvq, int nvqs)
+{
+    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+    VirtioBusState *vbus = VIRTIO_BUS(qbus);
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
+    int i, r;
+
+    for (i = 0; i < nvqs; ++i) {
+        r = k->set_host_notifier(qbus->parent, startvq + i, false);
+        if (r < 0) {
+            fprintf(stderr, "vhost VQ %d notifier cleanup failed: %d\n", i, -r);
+            fflush(stderr);
+        }
+        assert (r >= 0);
+    }
+}
+
 static void virtio_device_realize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);