Re: [PATCH RFC 08/15] hw/virtio/vhost-shadow-virtqueue: used handler

On Fri, Jul 24, 2026 at 4:29 AM Akihiko Odaki
<[email protected]> wrote:
>
>
> The error of the handler that may have left elem stale is ignored here.
>
> Regards,
> Akihiko Odaki

I have now added in a check that returns on error without setting the guest
notifier.  So essentially the call is ignored.  Maybe this could be expanded
on to handle errors more gracefully depending on the error code.

On Tue, Jul 28, 2026 at 8:06 AM Stefan Hajnoczi <[email protected]> wrote:
>
> >
> > +/**
> > + * Callback to handle a used buffer
> > + *
> > + * @svq: Shadow virtqueue
> > + * @elem: Element placed in the queue by the device
> > + * @vq_callback_opaque: Used to pass arguments to callback
> > + *
> > + * Returns 0 if the vq is running as expected
>
> The doc comment should mention what non-0 look like. Are errors reported
> as a negative errno?
>

Yes, a negative errno is expected on failure.  I have added clarification to the
comment.

On Mon, Aug 3, 2026 at 5:42 AM Hanna Czenczek <[email protected]> wrote:
>
> On 24.07.26 00:30, Connor Kite wrote:
> > Adds a handler callback to shadow virtqueues in order to enable
> > non-default processing of used vring elements from the device.
> >
> > Signed-off-by: Connor Kite <[email protected]>
> > ---
> >   hw/virtio/vhost-shadow-virtqueue.c |  4 ++++
> >   hw/virtio/vhost-shadow-virtqueue.h | 14 ++++++++++++++
> >   2 files changed, 18 insertions(+)
>
> The avail handler replaces `vhost_svq_add_element()`. I find it a bit
> unintuitive that this handler does not replace `virtqueue_fill()`, i.e.
> that it is not a “mirror” of the avail handler; that the avail handler
> actually handles avail elements, whereas the used handler is “just” a
> callback on each used element. Was that intentional? (If so, I think the
> comment should be explicit about this choice.)
>
> Hanna
>

I implemented it as a more narrowly-focused callback since virtqueue_fill
needs to be called regardless.  If I instead called virtqueue_fill
from the handler,
I would need to pass in the elem index and length via svq->ops_opaque,
which is a bit awkward as vhost_user defines ops_opaque when registering
the callback.  That's certainly not impossible to do, but this way felt cleaner.
For now, I will add a comment explaining that the callback should not be used
to add elements to the underlying vq's used ring.