Re: [PATCH v7 01/12] vhost: stop transfer elem ownership in vhost_handle_guest_kick

[Jason Wang]

On Fri, Aug 5, 2022 at 2:29 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> It was easier to allow vhost_svq_add to handle the memory. Now that we
> will allow qemu to add elements to a SVQ without the guest's knowledge,
> it's better to handle it in the caller.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

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

> ---
>  hw/virtio/vhost-shadow-virtqueue.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c 
> b/hw/virtio/vhost-shadow-virtqueue.c
> index e4956728dd..ffd2b2c972 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -233,9 +233,6 @@ static void vhost_svq_kick(VhostShadowVirtqueue *svq)
>  /**
>   * Add an element to a SVQ.
>   *
> - * The caller must check that there is enough slots for the new element. It
> - * takes ownership of the element: In case of failure not ENOSPC, it is free.
> - *
>   * Return -EINVAL if element is invalid, -ENOSPC if dev queue is full
>   */
>  int vhost_svq_add(VhostShadowVirtqueue *svq, const struct iovec *out_sg,
> @@ -252,7 +249,6 @@ int vhost_svq_add(VhostShadowVirtqueue *svq, const struct 
> iovec *out_sg,
>
>      ok = vhost_svq_add_split(svq, out_sg, out_num, in_sg, in_num, 
> &qemu_head);
>      if (unlikely(!ok)) {
> -        g_free(elem);
>          return -EINVAL;
>      }
>
> @@ -293,7 +289,7 @@ static void vhost_handle_guest_kick(VhostShadowVirtqueue 
> *svq)
>          virtio_queue_set_notification(svq->vq, false);
>
>          while (true) {
> -            VirtQueueElement *elem;
> +            g_autofree VirtQueueElement *elem;
>              int r;
>
>              if (svq->next_guest_avail_elem) {
> @@ -324,12 +320,14 @@ static void 
> vhost_handle_guest_kick(VhostShadowVirtqueue *svq)
>                       * queue the current guest descriptor and ignore kicks
>                       * until some elements are used.
>                       */
> -                    svq->next_guest_avail_elem = elem;
> +                    svq->next_guest_avail_elem = g_steal_pointer(&elem);
>                  }
>
>                  /* VQ is full or broken, just return and ignore kicks */
>                  return;
>              }
> +            /* elem belongs to SVQ or external caller now */
> +            elem = NULL;
>          }
>
>          virtio_queue_set_notification(svq->vq, true);
> --
> 2.31.1
>