[Spice-devel] [PATCH spice-server 0/4] Refactor RedChannel ClientCbs

[Christophe Fergeau]

On Wed, Aug 30, 2017 at 11:06:41AM +0200, Christophe Fergeau wrote:
> On Wed, Aug 30, 2017 at 03:31:46AM -0400, Frediano Ziglio wrote:
> > > This is an alternate proposal to the patch Frediano sent recently which
> > > includes the data argument in the ClientCbs struct and removes the
> > > g_object_get|set_data() calls.
> > > 
> > > This series also removes the GObject data stuff, but also does some
> > > deeper refactoring.
> > > 
> > 
> > 
> > Honestly does not seem that great.
> > 
> > It assumes CursorChannel and DisplayChannel will always have an
> > associated QXL device. There are different patches that remove this
> > needs for CursorChannel.
> 
> It seems this comment mostly applies to patch 2/4 which removes
> dispatcher = g_object_get_data(.., "dispatcher"); in favour of getting
> it from CommonGraphicsChannel::qxl? Or do you also object to moving from
> ClientCbs to virtual functions?

Ah you answered that already in your next email:
> The current (master) design is flexible. It allows to write
> channels that can run in the main thread or a separate one.
> Currently (master) CursorChannel and DisplayChannel run on
> separate threads (the reason is that image compression could
> take time and is not a good idea to suspend the main thread
> that long) but this is not in these channel implementation
> but on the RedQXL/RedWorker implementation (RedQxl and
> RedWorker are close friends basically implementing this
> thread separation, RedQxl implements the marshalling part
> of the dispatcher while RedWorker implements the unmarshalling
> one).
> 
> I think the possibility to have a single implementation
> that could run in either the main thread or in another is
> a good feature. If in the future one channel like SpiceVMC
> get expensive to run inside the main thread we could move
> easily or decide where to run dynamically.

Christophe