[virt-tools-list] [PATCH virt-viewer v2 4/7] Fullscreen auto-conf: wait for server to be configured
Marc-André Lureau
marcandre.lureau at gmail.com
Wed Apr 22 15:11:08 UTC 2015
Hi
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)
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.
On Tue, Apr 21, 2015 at 10:34 PM, Jonathon Jongsma <jjongsma at redhat.com>
wrote:
> 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
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
>
--
Marc-André Lureau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20150422/21648d53/attachment.htm>
More information about the virt-tools-list
mailing list