Re: [Qemu-devel] [PATCH] virtio: Register host notifier handler as external

[Fam Zheng]

On Thu, 04/21 15:20, Michael S. Tsirkin wrote:
> On Thu, Apr 21, 2016 at 10:04:28AM +0800, Fam Zheng wrote:
> > This ensures the bdrv_drained_begin() in block layer is effective and
> > fixes launchpad bug #1570134.
> > 
> > Signed-off-by: Fam Zheng <address@hidden>
> > ---
> >  hw/virtio/virtio.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index f745c4a..002c2c6 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -1829,10 +1829,11 @@ void 
> > virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
> >                                                 bool set_handler)
> >  {
> >      if (assign && set_handler) {
> > -        event_notifier_set_handler(&vq->host_notifier,
> > -                                   virtio_queue_host_notifier_read);
> > +        aio_set_event_notifier(qemu_get_aio_context(), &vq->host_notifier,
> > +                               true, virtio_queue_host_notifier_read);
> >      } else {
> > -        event_notifier_set_handler(&vq->host_notifier, NULL);
> > +        aio_set_event_notifier(qemu_get_aio_context(), &vq->host_notifier,
> > +                               true, NULL);
> >      }
> >      if (!assign) {
> >          /* Test and clear notifier before after disabling event,
> 
> Is this the right thing to do in all cases?
> E.g. it doesn't look like event_notifier_set_handler previously called
> aio_set_event_notifier on win32. Was there a reason for this?

It's a good catch. I'm not sure whether/how win32 goes with ioeventfd, but it
seems all the existing host notifier handling (fd handler) in vhost and data
plane mismatch with the event notifier implementation (WaitObject) in win32.

This is a subject understood best by Paolo but since he's on PTO I'll see if I
can figure out the answer by testing this code with wine.

Fam