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

[Fabiano Fidêncio]

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?

>>
>> 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,
-- 
Fabiano Fidêncio