Re: [PATCH 5/7] vdpa: Make SVQ vring unmapping return void

[Jason Wang]

在 2022/8/20 00:53, Eugenio Pérez 写道:
> Nothing actually reads the return value, but an error in cleaning some
> entries could cause device stop to abort, making a restart impossible.
> Better ignore explicitely the return value.
>
> Reported-by: Lei Yang <leiyang@redhat.com>
> Fixes: 34e3c94eda ("vdpa: Add custom IOTLB translations to SVQ")
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>


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


> ---
>   hw/virtio/vhost-vdpa.c | 32 ++++++++++----------------------
>   1 file changed, 10 insertions(+), 22 deletions(-)
>
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 943799c17c..07d00f5284 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -884,7 +884,7 @@ static int vhost_vdpa_svq_set_fds(struct vhost_dev *dev,
>   /**
>    * Unmap a SVQ area in the device
>    */
> -static bool vhost_vdpa_svq_unmap_ring(struct vhost_vdpa *v,
> +static void vhost_vdpa_svq_unmap_ring(struct vhost_vdpa *v,
>                                         const DMAMap *needle)
>   {
>       const DMAMap *result = vhost_iova_tree_find_iova(v->iova_tree, needle);
> @@ -893,37 +893,32 @@ static bool vhost_vdpa_svq_unmap_ring(struct vhost_vdpa 
> *v,
>   
>       if (unlikely(!result)) {
>           error_report("Unable to find SVQ address to unmap");
> -        return false;
> +        return;
>       }
>   
>       size = ROUND_UP(result->size, qemu_real_host_page_size());
>       r = vhost_vdpa_dma_unmap(v, result->iova, size);
>       if (unlikely(r < 0)) {
> -        return false;
> +        return;
>       }
>   
>       vhost_iova_tree_remove(v->iova_tree, result);
> -    return 0;
>   }
>   
> -static bool vhost_vdpa_svq_unmap_rings(struct vhost_dev *dev,
> +static void vhost_vdpa_svq_unmap_rings(struct vhost_dev *dev,
>                                          const VhostShadowVirtqueue *svq)
>   {
>       DMAMap needle = {};
>       struct vhost_vdpa *v = dev->opaque;
>       struct vhost_vring_addr svq_addr;
> -    bool ok;
>   
>       vhost_svq_get_vring_addr(svq, &svq_addr);
>   
>       needle.translated_addr = svq_addr.desc_user_addr;
> -    ok = vhost_vdpa_svq_unmap_ring(v, &needle);
> -    if (unlikely(!ok)) {
> -        return false;
> -    }
> +    vhost_vdpa_svq_unmap_ring(v, &needle);
>   
>       needle.translated_addr = svq_addr.used_user_addr;
> -    return vhost_vdpa_svq_unmap_ring(v, &needle);
> +    vhost_vdpa_svq_unmap_ring(v, &needle);
>   }
>   
>   /**
> @@ -1094,26 +1089,22 @@ err:
>       return false;
>   }
>   
> -static bool vhost_vdpa_svqs_stop(struct vhost_dev *dev)
> +static void vhost_vdpa_svqs_stop(struct vhost_dev *dev)
>   {
>       struct vhost_vdpa *v = dev->opaque;
>   
>       if (!v->shadow_vqs) {
> -        return true;
> +        return;
>       }
>   
>       for (unsigned i = 0; i < v->shadow_vqs->len; ++i) {
>           VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i);
> -        bool ok = vhost_vdpa_svq_unmap_rings(dev, svq);
> -        if (unlikely(!ok)) {
> -            return false;
> -        }
> +        vhost_vdpa_svq_unmap_rings(dev, svq);
>       }
>   
>       if (v->migration_blocker) {
>           migrate_del_blocker(v->migration_blocker);
>       }
> -    return true;
>   }
>   
>   static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
> @@ -1130,10 +1121,7 @@ static int vhost_vdpa_dev_start(struct vhost_dev *dev, 
> bool started)
>           }
>           vhost_vdpa_set_vring_ready(dev);
>       } else {
> -        ok = vhost_vdpa_svqs_stop(dev);
> -        if (unlikely(!ok)) {
> -            return -1;
> -        }
> +        vhost_vdpa_svqs_stop(dev);
>           vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
>       }
>