Re: [Qemu-devel] [PATCH 6/9] terminal3270: do not use backend timer sources

[Peter Xu]

On Tue, Aug 28, 2018 at 11:09:28AM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Aug 28, 2018 at 5:52 AM Peter Xu <address@hidden> wrote:
> >
> > On Tue, Aug 28, 2018 at 12:23:19AM +0200, Marc-André Lureau wrote:
> > > terminal3270 is uses the front-end side of the chardev. It shouldn't
> > > create sources from backend side context. Fwiw, send_timing_mark_cb
> > > calls qemu_chr_fe_write_all() which should be thread safe.
> > >
> > > This partially reverts changes from commit
> > > 2c716ba1506769c9be2caa02f0f6d6e7c00f4304.
> >
> > I don't fully understand too on why "It shouldn't create sources from
> > backend side context".  Could you explain a bit?
> 
> A frontend shouldn't use a backend function, and doesn't have to use
> the backend gmaincontext.
> 
> >
> > If you don't want the backend gcontext to be exposed to the frontend,
> > IMHO the simplest thing is just to use the NULL gcontext in
> > qemu_chr_timeout_add_ms() which is a one-liner change. I don't see
> > much point on re-using the GSource tags since IMHO GSource pointer is
> > always superior to the old tag system.
> 
> Using the tag or the GSource * doesn't change much afaik.

I did some convertions in the past year to convert tag -> GSource and
that shows some differences when we want non-default gcontexts...
Otherwise I won't bother. :)

> Not only
> this is not a hot path, but glib gmain code does create the tag on
> attach anyway. You may be marginally faster on remove/detach by
> avoiding one lookup.
> 
> (btw, I think both tag and GSource* are as old)

I am not strong on this, but my rule is that if I don't have explicit
reason to change a code I keep it there.  Here I don't see an obvious
reason to switch back to the tag things (not to mention that I still
think GSource is better, always).

Thanks,

-- 
Peter Xu