[virt-tools-list] [virt-viewer] [PATCH] session-spice: Only update displays geometry if the agent is connected

[Pavel Grunt]

On Wed, 2015-06-03 at 00:20 +0200, Fabiano Fidêncio wrote:
> On Wed, Jun 3, 2015 at 12:17 AM, Fabiano Fidêncio <fabiano at fidencio.org> 
> wrote:
> > On Wed, Jun 3, 2015 at 12:03 AM, Jonathon Jongsma <jjongsma at redhat.com> 
> > wrote:
> > > On Tue, 2015-06-02 at 19:07 +0200, Pavel Grunt wrote:
> > > > Hi, it makes sense.
> > > > 
> > > > What about moving the check to 
> > > > virt_viewer_session_spice_apply_monitor_geometry()?
> > > 
> > > No, I don't think we can move it there. Applying the monitor geometry
> > > does two things:
> > > 
> > > - it allows spice-gtk to update its the internal state so that it can
> > > stay in sync with the current display configuration
> > > - it sends down an update to the guest (if the agent is connected)
> > > 
> > > Fabiano's patch should be OK because this particular case is a "fake"
> > > display update intended only to send our current display configuration
> > > down to a newly-connected agent. But if we move this logic to
> > > _apply_monitor_geometry(), we will avoid updating spice-gtk whenever the
> > > agent is not connected, even if the client's display configuration *was*
> > > actually changed. I suspect this will cause some issues because
> > > spice-gtk will have an out-of-sync internal state.
> > 
> > Not exactly related to the patch (and not even sure if it would be a
> > valid clean up), but in
> > virt_viewer_session_spice_apply_monitor_geometry() we could replace
> > spice_main_set_display() by spice_main_update_display() and update the
> > display timer based on whether the agent is running or not.
> > 
> > What do you guys think about this?
> > 
> 
> Hmm. Actually, I don't see any gain on this :-p
> 
> Back to the patch, may I apply the patch as it is?
> 
Yes, ack from me.
Pavel

> > > 
> > > Jonathon
> > > 
> > > 
> > > > 
> > > > Pavel
> > > > 
> > > > On Tue, 2015-06-02 at 17:35 +0200, Fabiano Fidêncio wrote:
> > > > > ---
> > > > >  src/virt-viewer-session-spice.c | 11 ++++++++---
> > > > >  1 file changed, 8 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session
> > > > > -spice.c
> > > > > index b69faa6..f763975 100644
> > > > > --- a/src/virt-viewer-session-spice.c
> > > > > +++ b/src/virt-viewer-session-spice.c
> > > > > @@ -674,12 +674,17 @@ agent_connected_changed(SpiceChannel *cmain 
> > > > > G_GNUC_UNUSED,
> > > > >                          GParamSpec *pspec G_GNUC_UNUSED,
> > > > >                          VirtViewerSessionSpice *self)
> > > > >  {
> > > > > +    gboolean agent_connected;
> > > > > +
> > > > >      // this will force refresh of application menu
> > > > >      g_signal_emit_by_name(self, "session-display-updated");
> > > > > 
> > > > > -    /* this will force update displays geometry when the agent has 
> > > > > connected
> > > > > -     * after the application (eg: rebooting the guest) */
> > > > > -   
> > > > >  virt_viewer_session_update_displays_geometry(VIRT_VIEWER_SESSION(self
> > > > > ));
> > > > > +    g_object_get(cmain, "agent-connected", &agent_connected, NULL);
> > > > > +    if (agent_connected) {
> > > > > +        /* this will force update displays geometry when the agent 
> > > > > has connected
> > > > > +         * after the application (eg: rebooting the guest) */
> > > > > +       
> > > > >  virt_viewer_session_update_displays_geometry(VIRT_VIEWER_SESSION(self
> > > > > ));
> > > > > +    }
> > > > >  }
> > > > > 
> > > > >  static void
> > > > 
> > > > _______________________________________________
> > > > virt-tools-list mailing list
> > > > virt-tools-list at redhat.com
> > > > https://www.redhat.com/mailman/listinfo/virt-tools-list
> > > 
> > > 
> > > _______________________________________________
> > > virt-tools-list mailing list
> > > virt-tools-list at redhat.com
> > > https://www.redhat.com/mailman/listinfo/virt-tools-list
> > 
> > 
> > Best Regards,
> > --
> > Fabiano Fidêncio
> 
> 
> Best Regards,