Re: [Xen-devel] [PATCH 2/3] xen-bus: allow AioContext to be specified for each event channel
> -Original Message- > From: Anthony PERARD [mailto:anthony.per...@citrix.com] > Sent: 10 April 2019 16:23 > To: Paul Durrant > Cc: qemu-de...@nongnu.org; qemu-bl...@nongnu.org; > xen-devel@lists.xenproject.org; Stefano Stabellini > ; Stefan Hajnoczi ; Kevin Wolf > ; Max > Reitz > 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.per...@citrix.com] > > > Sent: 10 April 2019 13:57 > > > To: Paul Durrant > > > Cc: qemu-de...@nongnu.org; qemu-bl...@nongnu.org; > > > xen-devel@lists.xenproject.org; Stefano > Stabellini > > > ; Stefan Hajnoczi ; Kevin > > > Wolf ; > Max > > > Reitz > > > 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 > > > > --- > > > > @@ -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 > > Thanks, > > -- > Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/3] xen-bus: allow AioContext to be specified for each event channel
> -Original Message- > From: Anthony PERARD [mailto:anthony.per...@citrix.com] > Sent: 10 April 2019 13:57 > To: Paul Durrant > Cc: qemu-de...@nongnu.org; qemu-bl...@nongnu.org; > xen-devel@lists.xenproject.org; Stefano Stabellini > ; Stefan Hajnoczi ; Kevin Wolf > ; Max > Reitz > 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 > > --- > > @@ -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 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [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.per...@citrix.com] > > Sent: 10 April 2019 13:57 > > To: Paul Durrant > > Cc: qemu-de...@nongnu.org; qemu-bl...@nongnu.org; > > xen-devel@lists.xenproject.org; Stefano Stabellini > > ; Stefan Hajnoczi ; Kevin Wolf > > ; Max > > Reitz > > 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 > > > --- > > > @@ -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. Reviewed-by: Anthony PERARD Thanks, -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [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 > --- > @@ -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? -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
