Re: [Qemu-block] [PATCH 2/3] xen-bus: allow AioContext to be specified for each event channel

[Paul Durrant]

> -----Original Message-----
> From: Anthony PERARD [mailto:address@hidden
> Sent: 10 April 2019 13:57
> To: Paul Durrant <address@hidden>
> Cc: address@hidden; address@hidden; address@hidden; Stefano Stabellini
> <address@hidden>; Stefan Hajnoczi <address@hidden>; Kevin Wolf 
> <address@hidden>; Max
> Reitz <address@hidden>
> Subject: Re: [PATCH 2/3] xen-bus: allow AioContext to be specified for each 
> event channel
> 
> On Mon, Apr 08, 2019 at 04:16:16PM +0100, Paul Durrant wrote:
> > This patch adds an AioContext parameter to xen_device_bind_event_channel()
> > and then uses aio_set_fd_handler() to set the callback rather than
> > qemu_set_fd_handler().
> >
> > Signed-off-by: Paul Durrant <address@hidden>
> > ---
> > @@ -943,6 +944,7 @@ static void xen_device_event(void *opaque)
> >  }
> >
> >  XenEventChannel *xen_device_bind_event_channel(XenDevice *xendev,
> > +                                               AioContext *ctx,
> >                                                 unsigned int port,
> >                                                 XenEventHandler handler,
> >                                                 void *opaque, Error **errp)
> > @@ -968,8 +970,9 @@ XenEventChannel 
> > *xen_device_bind_event_channel(XenDevice *xendev,
> >      channel->handler = handler;
> >      channel->opaque = opaque;
> >
> > -    qemu_set_fd_handler(xenevtchn_fd(channel->xeh), xen_device_event, NULL,
> > -                        channel);
> > +    channel->ctx = ctx;
> > +    aio_set_fd_handler(channel->ctx, xenevtchn_fd(channel->xeh), false,
> > +                       xen_device_event, NULL, NULL, channel);
> 
> I wonder if the `'is_external' parameter of aio_set_fd_handler shoud be
> `true' here, instead. That flag seems to be used when making a snapshot
> of a blockdev, for example.
> 
> That was introduced by:
> dca21ef23ba48f6f1428c59f295a857e5dc203c8^..c07bc2c1658fffeee08eb46402b2f66d55b07586
> 
> What do you think?

Interesting. I admit I was merely transcribing what qemu_set_fd_handler() 
passes without really looking into the values. Looking at the arguments that 
virtio-blk passes to aio_set_event_notifier() though, and what 'is_external' 
means, it would appear that setting it to true is probably the right thing to 
do. Do you want me to send a v2 of the series or can you fix it up?

  Cheers,

    Paul

> 
> 
> --
> Anthony PERARD