Re: [Qemu-devel] [PATCH 03/14] qio: introduce qio_channel_add_watch_full()

[Peter Xu]

On Wed, Feb 28, 2018 at 12:47:43PM +0000, Daniel P. Berrangé wrote:
> On Wed, Feb 28, 2018 at 08:44:24PM +0800, Peter Xu wrote:
> > On Wed, Feb 28, 2018 at 09:08:45AM +0000, Daniel P. Berrangé wrote:
> > > On Wed, Feb 28, 2018 at 01:06:22PM +0800, Peter Xu wrote:
> > > > It's a more powerful version of qio_channel_add_watch(), which supports
> > > > non-default gcontext.  It's stripped from the old one, then we have
> > > > g_source_get_id() to fetch the tag ID to keep the old interface.
> > > > 
> > > > Note that the new API will return a gsource, meanwhile keep a reference
> > > > of it so that callers need to unref them explicitly.
> > > 
> > > I don't really like this. Having qio_channel_add_watch and
> > > qio_channel_add_watch_full with differing return values is
> > > really very surprising. They should be functionally identical,
> > > except for the extra context arg.
> > 
> > Yeah it's not nice, but I do need the GSource and the tag ID does not
> > help in the series.
> > 
> > An alternative would be that I modify qio_channel_add_watch() to
> > return GSource too.  Is there an third choice that you could suggest?
> 
> Given you have the id + GMainContext you can just acquire the GSource,
> if needed, using g_main_context_find_source_by_id.

I always feel unsafe to play around with tag IDs since the IDs can
change after GSource removed and new GSource added, and also the
result of the call will depend on a correct pairing of context (so if
the context is incorrect, instead of failure, we possibly got
everything screwed up while we never know we failed...).

But indeed for this one it seems pretty safe if I call
g_main_context_find_source_by_id() right after I call
qio_channel_add_watch_full() to fetch the GSource.  If you agree, I
can use this approach in my next post.  Thanks,

-- 
Peter Xu