<div dir="ltr"><div><div>Hi </div>Using a timer for monitor configuration to take place will lead to all kind of issues. (and 2 second is to short for wan, to slow for local, somehow short even for lan) </div>Instead there should probably be a more robust solution involving the agent to monitor the initial configuration taking place, while the client wouldn't send further monitor configuration. The agent wouldn't have a timer either, it would either succeed or fail to manage this initial configuration and resume client auto-conf. nack from me. </div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Apr 21, 2015 at 10:34 PM, Jonathon Jongsma <<a href="mailto:jjongsma@redhat.com" target="_blank">jjongsma@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">When doing fullscreen auto-conf, we send down a new configuration to the server before the display chanels are connected. Due to race conditions in the server, we sometimes get an initial monitors-config message with the guest's old display state. If the guest happened to have more displays enabled than we need for auto-conf, those extra displays windows will remain open, and the auto-conf will not succeed. To avoid this, we delay creating any windows and displays until the guest responds with a monitor config equal to the one we sent down for auto-conf. This allows the server some time to "settle", and prevents us from opening more windows on the client than we want. If for some reason the guest cannot be configured properly within a certain time frame, the client will go ahead and open displays to match the current state of the server. Resolves: rhbz#1200750 --- src/virt-viewer-session-spice.c | 121 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 118 insertions(+), 3 deletions(-) diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c index 4620bba..d0a7ab5 100644 --- a/src/virt-viewer-session-spice.c +++ b/src/virt-viewer-session-spice.c @@ -66,6 +66,7 @@ struct _VirtViewerSessionSpicePrivate { AutoConfState auto_conf_state; GList *display_channels; GArray *fullscreen_config; + guint auto_conf_timer; }; #define VIRT_VIEWER_SESSION_SPICE_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE((o), VIRT_VIEWER_TYPE_SESSION_SPICE, VirtViewerSessionSpicePrivate)) @@ -704,9 +705,8 @@ destroy_display(gpointer data) } static void -virt_viewer_session_spice_display_monitors(SpiceChannel *channel, - GParamSpec *pspec G_GNUC_UNUSED, - VirtViewerSessionSpice *self) +virt_viewer_session_spice_process_monitors_for_display_channel(VirtViewerSessionSpice *self, + SpiceChannel *channel) { GArray *monitors = NULL; GPtrArray *displays = NULL; @@ -759,6 +759,105 @@ virt_viewer_session_spice_display_monitors(SpiceChannel *channel, } +static gboolean +virt_viewer_session_spice_displays_match_initial_config(VirtViewerSessionSpice *self) +{ + cairo_region_t *region1, *region2; + GList *l; + int i; + GArray *rects = g_array_new(FALSE, TRUE, sizeof(GdkRectangle)); + VirtViewerApp *app = virt_viewer_session_get_app(VIRT_VIEWER_SESSION(self)); + GList *initial_displays = virt_viewer_app_get_initial_displays(app); + gboolean match = FALSE; + + if (!virt_viewer_app_get_fullscreen(app)) + goto cleanup; + + for (l = self->priv->display_channels; l != NULL; l = l->next) { + GArray *monitors = NULL; + g_object_get(l->data, + "monitors", &monitors, + NULL); + + if (!monitors) + continue; + + for (i = 0; i < monitors->len; i++) { + SpiceDisplayMonitorConfig *monitor = &g_array_index(monitors, SpiceDisplayMonitorConfig, i); + + if (monitor->width == 0 || monitor->height == 0) + continue; + + GdkRectangle rect = { + .x = monitor->x, + .y = monitor->y, + .width = monitor->width, + .height = monitor->height + }; + g_array_append_val(rects, rect); + } + } + + if (rects->len != g_list_length(initial_displays)) + goto cleanup; + + region1 = cairo_region_create_rectangles((cairo_rectangle_int_t*)rects->data, + rects->len); + region2 = cairo_region_create_rectangles((cairo_rectangle_int_t*)self->priv->fullscreen_config->data, + self->priv->fullscreen_config->len); + + match = cairo_region_equal(region1, region2); + + cairo_region_destroy(region1); + cairo_region_destroy(region2); + +cleanup: + g_list_free(initial_displays); + g_array_unref(rects); + + return match; +} + +static void +virt_viewer_session_spice_auto_conf_complete(VirtViewerSessionSpice *self) +{ + GList *l; + + if (self->priv->auto_conf_timer) { + g_source_remove(self->priv->auto_conf_timer); + self->priv->auto_conf_timer = 0; + } + + self->priv->auto_conf_state = AUTO_CONF_STATE_COMPLETE; + + /* now process all pending display channels, creating displays, etc */ + for (l = self->priv->display_channels; l != NULL; l = l->next) + virt_viewer_session_spice_process_monitors_for_display_channel(self, l->data); +} + +static void +virt_viewer_session_spice_display_monitors(SpiceChannel *channel, + GParamSpec *pspec G_GNUC_UNUSED, + VirtViewerSessionSpice *self) +{ + if (self->priv->auto_conf_state == AUTO_CONF_STATE_SENT) { + + /* At startup, the server can sometimes send up old monitor + * configurations even if we've sent the initial auto conf. This will + * generally be followed by another monitor update with the correct + * configuration. So if we've sent our auto-conf configuration, check + * whether this is the correct config and process all pending updates. + * If not, wait for the next update. */ + if (virt_viewer_session_spice_displays_match_initial_config(self)) + virt_viewer_session_spice_auto_conf_complete(self); + else + g_debug("Displays don't match initial config. Waiting for next monitor config update"); + return; + } + + virt_viewer_session_spice_process_monitors_for_display_channel(self, channel); +} + static void virt_viewer_session_spice_channel_new(SpiceSession *s, SpiceChannel *channel, @@ -832,6 +931,16 @@ property_notify_do_auto_conf(GObject *gobject G_GNUC_UNUSED, } static gboolean +timeout_complete_auto_conf(gpointer data) +{ + VirtViewerSessionSpice *self = VIRT_VIEWER_SESSION_SPICE(data); + g_debug("Guest wasn't configured in time, completing auto-conf"); + virt_viewer_session_spice_auto_conf_complete(self); + + return FALSE; +} + +static gboolean virt_viewer_session_spice_fullscreen_auto_conf(VirtViewerSessionSpice *self) { GdkScreen *screen = gdk_screen_get_default(); @@ -901,6 +1010,12 @@ virt_viewer_session_spice_fullscreen_auto_conf(VirtViewerSessionSpice *self) spice_main_send_monitor_config(cmain); self->priv->auto_conf_state = AUTO_CONF_STATE_SENT; + /* Wait for the guest to respond with the exact configuration we just sent + * down. If the configuration doesn't happen before the timeout, just show + * the current displays */ + self->priv->auto_conf_timer = + g_timeout_add_seconds(2, timeout_complete_auto_conf, self); + self->priv->fullscreen_config = config; return TRUE; } -- 2.1.0 _______________________________________________ virt-tools-list mailing list <a href="mailto:virt-tools-list@redhat.com">virt-tools-list@redhat.com</a> <a href="https://www.redhat.com/mailman/listinfo/virt-tools-list" target="_blank">https://www.redhat.com/mailman/listinfo/virt-tools-list</a> </blockquote></div> -- <div class="gmail_signature">Marc-André Lureau</div> </div>