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

On 2026/08/20 10:06, Connor Kite wrote:
> On Tue, Aug 18, 2026 at 11:23 PM Akihiko Odaki
> <[email protected]> wrote:
>>
>>> +
>>> +                if (r < 0) {
>>> +                    /* VQ or handler is broken. Do not set guest notifier */
>>
>> This comment is extraneous. It is obvious that event_notifier_set()
>> below is not called, and it is same with the existing error path below.
>>
>> What matters here is the difference with the existing error path. Why
>> doesn't it call virtqueue_fill() nor virtqueue_flush()? They need to be
>> addressed here.
>>
>> Regards,
>> Akihiko Odaki
>>
> 
> Here, apart from not calling event_notifier_set (not notifying the
> guest), we are also not calling vhost_svq_enable_notification, so the
> device is directed not to send any more call notifications.  If the
> guest isn't receiving notifications, and the device isn't sending
> them, is there a purpose to flushing the current element?  With the
> current implementation, I'm not sure there is a path to re-enable
> calls on either the front-end or back-end, but maybe a case could be
> made that it's good hygiene to flush the known bad element?  For now,
> I will add the calls to add and flush the element

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