Re: [PATCH v6 03/10] vhost: Allocate SVQ device file descriptors at device start

[Jason Wang]

On Wed, Nov 9, 2022 at 1:08 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> The next patches will start control SVQ if possible. However, we don't
> know if that will be possible at qemu boot anymore.
>
> Delay device file descriptors until we know it at device start.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

> ---
>  hw/virtio/vhost-shadow-virtqueue.c | 31 ++------------------------
>  hw/virtio/vhost-vdpa.c             | 35 ++++++++++++++++++++++++------
>  2 files changed, 30 insertions(+), 36 deletions(-)
>
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c 
> b/hw/virtio/vhost-shadow-virtqueue.c
> index 264ddc166d..3b05bab44d 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -715,43 +715,18 @@ void vhost_svq_stop(VhostShadowVirtqueue *svq)
>   * @iova_tree: Tree to perform descriptors translations
>   * @ops: SVQ owner callbacks
>   * @ops_opaque: ops opaque pointer
> - *
> - * Returns the new virtqueue or NULL.
> - *
> - * In case of error, reason is reported through error_report.
>   */
>  VhostShadowVirtqueue *vhost_svq_new(VhostIOVATree *iova_tree,
>                                      const VhostShadowVirtqueueOps *ops,
>                                      void *ops_opaque)
>  {
> -    g_autofree VhostShadowVirtqueue *svq = g_new0(VhostShadowVirtqueue, 1);
> -    int r;
> -
> -    r = event_notifier_init(&svq->hdev_kick, 0);
> -    if (r != 0) {
> -        error_report("Couldn't create kick event notifier: %s (%d)",
> -                     g_strerror(errno), errno);
> -        goto err_init_hdev_kick;
> -    }
> -
> -    r = event_notifier_init(&svq->hdev_call, 0);
> -    if (r != 0) {
> -        error_report("Couldn't create call event notifier: %s (%d)",
> -                     g_strerror(errno), errno);
> -        goto err_init_hdev_call;
> -    }
> +    VhostShadowVirtqueue *svq = g_new0(VhostShadowVirtqueue, 1);
>
>      event_notifier_init_fd(&svq->svq_kick, VHOST_FILE_UNBIND);
>      svq->iova_tree = iova_tree;
>      svq->ops = ops;
>      svq->ops_opaque = ops_opaque;
> -    return g_steal_pointer(&svq);
> -
> -err_init_hdev_call:
> -    event_notifier_cleanup(&svq->hdev_kick);
> -
> -err_init_hdev_kick:
> -    return NULL;
> +    return svq;
>  }
>
>  /**
> @@ -763,7 +738,5 @@ void vhost_svq_free(gpointer pvq)
>  {
>      VhostShadowVirtqueue *vq = pvq;
>      vhost_svq_stop(vq);
> -    event_notifier_cleanup(&vq->hdev_kick);
> -    event_notifier_cleanup(&vq->hdev_call);
>      g_free(vq);
>  }
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 7f0ff4df5b..3df2775760 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -428,15 +428,11 @@ static int vhost_vdpa_init_svq(struct vhost_dev *hdev, 
> struct vhost_vdpa *v,
>
>      shadow_vqs = g_ptr_array_new_full(hdev->nvqs, vhost_svq_free);
>      for (unsigned n = 0; n < hdev->nvqs; ++n) {
> -        g_autoptr(VhostShadowVirtqueue) svq;
> +        VhostShadowVirtqueue *svq;
>
>          svq = vhost_svq_new(v->iova_tree, v->shadow_vq_ops,
>                              v->shadow_vq_ops_opaque);
> -        if (unlikely(!svq)) {
> -            error_setg(errp, "Cannot create svq %u", n);
> -            return -1;
> -        }
> -        g_ptr_array_add(shadow_vqs, g_steal_pointer(&svq));
> +        g_ptr_array_add(shadow_vqs, svq);
>      }
>
>      v->shadow_vqs = g_steal_pointer(&shadow_vqs);
> @@ -864,11 +860,23 @@ static int vhost_vdpa_svq_set_fds(struct vhost_dev *dev,
>      const EventNotifier *event_notifier = &svq->hdev_kick;
>      int r;
>
> +    r = event_notifier_init(&svq->hdev_kick, 0);
> +    if (r != 0) {
> +        error_setg_errno(errp, -r, "Couldn't create kick event notifier");
> +        goto err_init_hdev_kick;
> +    }
> +
> +    r = event_notifier_init(&svq->hdev_call, 0);
> +    if (r != 0) {
> +        error_setg_errno(errp, -r, "Couldn't create call event notifier");
> +        goto err_init_hdev_call;
> +    }
> +
>      file.fd = event_notifier_get_fd(event_notifier);
>      r = vhost_vdpa_set_vring_dev_kick(dev, &file);
>      if (unlikely(r != 0)) {
>          error_setg_errno(errp, -r, "Can't set device kick fd");
> -        return r;
> +        goto err_init_set_dev_fd;
>      }
>
>      event_notifier = &svq->hdev_call;
> @@ -876,8 +884,18 @@ static int vhost_vdpa_svq_set_fds(struct vhost_dev *dev,
>      r = vhost_vdpa_set_vring_dev_call(dev, &file);
>      if (unlikely(r != 0)) {
>          error_setg_errno(errp, -r, "Can't set device call fd");
> +        goto err_init_set_dev_fd;
>      }
>
> +    return 0;
> +
> +err_init_set_dev_fd:
> +    event_notifier_set_handler(&svq->hdev_call, NULL);
> +
> +err_init_hdev_call:
> +    event_notifier_cleanup(&svq->hdev_kick);
> +
> +err_init_hdev_kick:
>      return r;
>  }
>
> @@ -1089,6 +1107,9 @@ static void vhost_vdpa_svqs_stop(struct vhost_dev *dev)
>      for (unsigned i = 0; i < v->shadow_vqs->len; ++i) {
>          VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i);
>          vhost_vdpa_svq_unmap_rings(dev, svq);
> +
> +        event_notifier_cleanup(&svq->hdev_kick);
> +        event_notifier_cleanup(&svq->hdev_call);
>      }
>  }
>
> --
> 2.31.1
>