On Wed, Aug 13, 2025 at 12:56 PM Vladimir Sementsov-Ogievskiy
<[email protected]> wrote:
>
> This helps to simplify failure paths of vhost_virtqueue_start()
> a lot.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
> ---
> hw/virtio/vhost.c | 23 +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 1e14987cd5..1fdc1937b6 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -489,6 +489,10 @@ static void vhost_memory_unmap(struct vhost_dev *dev,
> void *buffer,
> hwaddr len, int is_write,
> hwaddr access_len)
> {
> + if (!buffer) {
> + return;
> + }
> +
> if (!vhost_dev_has_iommu(dev)) {
> cpu_physical_memory_unmap(buffer, len, is_write, access_len);
> }
> @@ -1313,33 +1317,33 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> vq->desc = vhost_memory_map(dev, a, l, false);
> if (!vq->desc) {
> r = -ENOMEM;
> - goto fail_alloc_desc;
> + goto fail;
> }
> vq->avail_size = l = virtio_queue_get_avail_size(vdev, idx);
> vq->avail_phys = a = virtio_queue_get_avail_addr(vdev, idx);
> vq->avail = vhost_memory_map(dev, a, l, false);
> if (!vq->avail) {
> r = -ENOMEM;
> - goto fail_alloc_avail;
> + goto fail;
> }
> vq->used_size = l = virtio_queue_get_used_size(vdev, idx);
> vq->used_phys = a = virtio_queue_get_used_addr(vdev, idx);
> vq->used = vhost_memory_map(dev, a, l, true);
> if (!vq->used) {
> r = -ENOMEM;
> - goto fail_alloc_used;
> + goto fail;
> }
>
> r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
> if (r < 0) {
> - goto fail_alloc;
> + goto fail;
> }
>
> file.fd = event_notifier_get_fd(virtio_queue_get_host_notifier(vvq));
> r = dev->vhost_ops->vhost_set_vring_kick(dev, &file);
> if (r) {
> VHOST_OPS_DEBUG(r, "vhost_set_vring_kick failed");
> - goto fail_kick;
> + goto fail;
> }
>
> /* Clear and discard previous events if any. */
> @@ -1359,24 +1363,19 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> file.fd = -1;
> r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
> if (r) {
> - goto fail_vector;
> + goto fail;
> }
> }
>
> return 0;
>
> -fail_vector:
> -fail_kick:
> -fail_alloc:
> +fail:
> vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
> 0, 0);
> -fail_alloc_used:
> vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev,
> idx),
> 0, 0);
> -fail_alloc_avail:
> vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
> 0, 0);
> -fail_alloc_desc:
> return r;
This assumes that vq->{used, avail, desc} will be nulled out. I’m not
totally convinced that will be the case when a device is started and
stopped, or at least I don’t see the unmap path doing it.
> }
>
> --
> 2.48.1
>
>
