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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@xxxxxxxxxx]
> Sent: 10 April 2019 16:23
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: qemu-devel@xxxxxxxxxx; qemu-block@xxxxxxxxxx; 
> xen-devel@xxxxxxxxxxxxxxxxxxxx; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; Stefan Hajnoczi <stefanha@xxxxxxxxxx>; Kevin Wolf 
> <kwolf@xxxxxxxxxx>; Max
> Reitz <mreitz@xxxxxxxxxx>
> Subject: Re: [PATCH 2/3] xen-bus: allow AioContext to be specified for each 
> event channel
> 
> On Wed, Apr 10, 2019 at 04:20:05PM +0100, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Anthony PERARD [mailto:anthony.perard@xxxxxxxxxx]
> > > Sent: 10 April 2019 13:57
> > > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> > > Cc: qemu-devel@xxxxxxxxxx; qemu-block@xxxxxxxxxx; 
> > > xen-devel@xxxxxxxxxxxxxxxxxxxx; Stefano
> Stabellini
> > > <sstabellini@xxxxxxxxxx>; Stefan Hajnoczi <stefanha@xxxxxxxxxx>; Kevin 
> > > Wolf <kwolf@xxxxxxxxxx>;
> Max
> > > Reitz <mreitz@xxxxxxxxxx>
> > > 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 <paul.durrant@xxxxxxxxxx>
> > > > ---
> > > > @@ -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?
> 
> I'll fix that up.

Thanks,

  Paul

> 
> Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> 
> Thanks,
> 
> --
> Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel