On 25.01.24 19:18, Hanna Czenczek wrote:
On 25.01.24 19:03, Stefan Hajnoczi wrote:
On Wed, Jan 24, 2024 at 06:38:30PM +0100, Hanna Czenczek wrote:
[...]
@@ -3563,6 +3574,13 @@ void
virtio_queue_aio_attach_host_notifier(VirtQueue *vq, AioContext *ctx)
aio_set_event_notifier_poll(ctx, &vq->host_notifier,
virtio_queue_host_notifier_aio_poll_begin,
virtio_queue_host_notifier_aio_poll_end);
+
+ /*
+ * We will have ignored notifications about new requests from
the guest
+ * during the drain, so "kick" the virt queue to process those
requests
+ * now.
+ */
+ virtio_queue_notify(vq->vdev, vq->queue_index);
event_notifier_set(&vq->host_notifier) is easier to understand because
it doesn't contain a non-host_notifier code path that we must not take.
Is there a reason why you used virtio_queue_notify() instead?
Not a good one anyway!
virtio_queue_notify() is just what seemed obvious to me (i.e. to
notify the virtqueue). Before removal of the AioContext lock, calling
handle_output seemed safe. But, yes, there was the discussion on the
RFC that it really isn’t. I didn’t consider that means we must rely
on virtio_queue_notify() calling event_notifier_set(), so we may as
well call it explicitly here.
I’ll fix it, thanks for pointing it out!
(I think together with this change, I’ll also remove the
event_notifier_set() call from virtio_blk_data_plane_start(). It’d
obviously be a duplicate, and removing it shows why
virtio_queue_aio_attach_host_notifier() should always kick the queue.)
