[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 8:01 AM, Pavel Grunt <pgrunt at redhat.com> wrote:
> 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
>

Okay, pushed!

Best Regards,
-- 
Fabiano Fidêncio