On Wed, Feb 28, 2018 at 09:25:11AM +0000, Daniel P. Berrangé wrote: > On Wed, Feb 28, 2018 at 01:06:26PM +0800, Peter Xu wrote: > > TCP chardevs can be using QIO network listeners working in the > > background when in listening mode. However the network listeners are > > always running in main context. This can race with chardevs that are > > running in non-main contexts. > > > > To solve this: firstly introduce qio_net_listener_set_context() to allow > > caller to set gcontext for network listeners. Then call it in > > tcp_chr_update_read_handler(), with the newly cached gcontext. > > > > It's fairly straightforward after we have introduced some net listener > > helper functions - basically we unregister the GSources and add them > > back with the correct context. > > > > Signed-off-by: Peter Xu <pet...@redhat.com> > > --- > > chardev/char-socket.c | 9 +++++++++ > > include/io/net-listener.h | 12 ++++++++++++ > > io/net-listener.c | 7 +++++++ > > 3 files changed, 28 insertions(+) > > > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c > > index 43a2cc2c1c..8f0935cd15 100644 > > --- a/chardev/char-socket.c > > +++ b/chardev/char-socket.c > > @@ -559,6 +559,15 @@ static void tcp_chr_update_read_handler(Chardev *chr) > > { > > SocketChardev *s = SOCKET_CHARDEV(chr); > > > > + if (s->listener) { > > + /* > > + * It's possible that chardev context is changed in > > + * qemu_chr_be_update_read_handlers(). Reset it for QIO net > > + * listener if there is. > > + */ > > + qio_net_listener_set_context(s->listener, chr->gcontext); > > + } > > + > > if (!s->connected) { > > return; > > } > > diff --git a/include/io/net-listener.h b/include/io/net-listener.h > > index 566be283b3..39dede9d6f 100644 > > --- a/include/io/net-listener.h > > +++ b/include/io/net-listener.h > > @@ -106,6 +106,18 @@ int qio_net_listener_open_sync(QIONetListener > > *listener, > > SocketAddress *addr, > > Error **errp); > > > > +/** > > + * qio_net_listener_set_context: > > + * @listener: the net listener object > > + * @context: the context that we'd like to bind the sources to > > + * > > + * This helper does not do anything but moves existing net listener > > + * sources from the old one to the new one. It can be seen as a > > + * no-operation if there is no listening source at all. > > + */ > > +void qio_net_listener_set_context(QIONetListener *listener, > > + GMainContext *context); > > I don't think this is a good design. The GMainContext should be provided > to the qio_net_listener_set_client_func method immediately, not updated > after the fact
After the patches, this is qio_net_listener_set_client_func_full(): void qio_net_listener_set_client_func_full(QIONetListener *listener, QIONetListenerClientFunc func, gpointer data, GDestroyNotify notify, GMainContext *context) { if (listener->io_notify) { listener->io_notify(listener->io_data); } listener->io_func = func; listener->io_data = data; listener->io_notify = notify; qio_net_listener_sources_clear(listener); qio_net_listener_sources_update(listener, context); } This is qio_net_listener_set_context(): void qio_net_listener_set_context(QIONetListener *listener, GMainContext *context) { qio_net_listener_sources_clear(listener); qio_net_listener_sources_update(listener, context); } So... Basically qio_net_listener_set_context() is just a simplified version of qio_net_listener_set_client_func_full(), but without passing in the funcs again. So do you mean that I can just avoid introducing this function and call qio_net_listener_set_client_func_full() directly? Thanks, -- Peter Xu