Re: [Qemu-devel] [PATCH 3/3] iothread: delay the context release to finalize

[Fam Zheng <famz@redhat.com>]

On Mon, 09/25 13:50, Peter Xu wrote:
> On Mon, Sep 25, 2017 at 01:30:02PM +0800, Fam Zheng wrote:
> > On Mon, 09/25 13:23, Peter Xu wrote:
> > > On Fri, Sep 22, 2017 at 09:09:22PM +0800, Fam Zheng wrote:
> > > > On Fri, 09/22 16:56, Peter Xu wrote:
> > > > > When gcontext is used with iothread, the context will be destroyed
> > > > > during iothread_stop().  That's not good since sometimes we would like
> > > > > to keep the resources until iothread is destroyed, but we may want to
> > > > > stop the thread before that point.
> > > > 
> > > > Would be nice if you can also mention the glib bug that "required" this in the
> > > > commit message.
> > > 
> > > I can add it, but I am not sure it's very closely related (and I'm
> > > afraid that may confuse more people).  Say, even without that bug, I
> > > would still think it not a good idea to free the context in the loop,
> > > especially considering that we have the finalize function there.  Thanks,
> > 
> > It's interesting to know if or not your future change will break without this
> > patch, this is especially useful for backport.
> 
> I haven't tried to run with iothread and without this patch, but I
> think it should fail, so this patch should be needed.
> 
> The point is that we should not destroy the context before explicitly
> calling remove_fd_in_watch() if the context is running chardevs.
> Without this patch, this rule does not satisfy.  And IIUC this rule
> comes from the glib bug.
> 
> Anyway, I'll mention it in commit message to clarify.

OK, thanks for the explanations! My r-b still stands with the amended commit
log.

Fam