Re: [PATCH RFC v2 04/13] hw/virtio/vhost-shadow-virtqueue: used callback

On Wed, Aug 19, 2026 at 10:15 PM Akihiko Odaki
<[email protected]> wrote:
>
>
> Skipping event_notifier_set() only suppresses this guest interrupt, and
> disabling SVQ notifications only suppresses backend call notifications;
> it does not stop backend ring processing. The error should be propagated
> with virtio_error().
>
> I looked into the virtqueue_fill() and virtqueue_flush() calls in the
> existing error path. The virtqueue_fill() call detaches the element and
> publishes it to the guest. Detaching the element is necessary and should
> be done in the new error path too. On the other hand, publishing the
> failed element is wrong and should not be done.
>
> virtqueue_flush(vq, i) is still required to publish and account for the
> preceding successfully filled elements.
>
> Regards,
> Akihiko Odaki

Thank you for the explanation!  I was not aware of virtio_error.  This
error path will now follow the following order:

    if (r < 0) {
        virtio_error(svq->vdev, "Vhost shadow virtqueue error while
handling used element.\n");
        virtqueue_detach_element(vq, elem, 0);
        g_free(elem);
        virtqueue_flush(vq, i);
        return;
    }

One thing still seems a little strange here about how vq->inuse is
modified by these operations.  The count provided to virtqueue_flush
in the svq code is the number of elements that have been added.
However, depending on which type of flush occurs, the value of
vq->inuse is decreased by either this count (virtqueue_split_flush) or
the number of descriptors in the flushed elements
(virtqueue_packed_flush, virtqueue_ordered_flush).  Also, the
virtqueue_detach_element always decrements vq->inuse by the number of
descriptors.

I'm possibly just misunderstanding something, but this seems like
vq->inuse could potentially end up in an incorrect state.  Maybe that
doesn't matter in this case once virtio_error gets called...