On Tue, 7 Apr 2015 12:45:56 +0200
"Michael S. Tsirkin" <m...@redhat.com> wrote:
> generalize and move these from vhost to virtio.
>
> Signed-off-by: Michael S. Tsirkin <m...@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);
