[virt-tools-list] [virt-viewer RFC] Hide 'un-ready' displays after a short timeout

[Jonathon Jongsma]

Fair enough.  I think I probably agree with you, but wanted to try it out and ask opinions. Another mitigating factor is that this doesn't apply to all sparse monitor configurations.  Only to those that are configured with the guest display config tools rather than by e.g. disabling the display in virt-viewer.  So that further reduces the likelihood that somebody will experience this.

Jonathon


----- Original Message -----
> From: "Marc-André Lureau" <mlureau at redhat.com>
> To: "Jonathon Jongsma" <jjongsma at redhat.com>
> Cc: virt-tools-list at redhat.com
> Sent: Thursday, November 21, 2013 3:05:43 AM
> Subject: Re: [virt-tools-list] [virt-viewer RFC] Hide 'un-ready' displays after	a short timeout
> 
> Hi,
> 
> Tbh, I think the current behaviour is better than hiding and showing window
> with arbitrary timings.
> 
> I understand that #868970 is a bug, and there is no simple solution to it.
> 
> However, I wonder how useful and common is a "sparse" monitor configuration,
> and thus I'd minor #868970 severity and keep current behaviour.
> 
> ----- Original Message -----
> > When a user disables a display using the display configuration tools in the
> > guest, the window simply turns black and displays a "Waiting for display
> > N..."
> > message.  If the user then resizes one of the windows, it will re-enable
> > this
> > disabled window (rhbz#868970).
> > 
> > There is not a good way to determine for certain whether the display is
> > 'permanently' disabled or whether it's a transient situation (e.g. during
> > display reconfigure, restarting the guest). This patch adds a workaround
> > that
> > sets up a timeout to hide and formally disable the display after a short
> > timeout
> > (currently 5 seconds).
> > 
> > In practice, this does significantly reduce the impact of rhbz#868970
> > (though
> > it's not a proper solution -- that would require significantly more
> > invasive
> > changes, including a new agent capability). Unfortunately, this patch has a
> > slightly negative impact on the user experience of rebooting a guest. For
> > Windows guests, the impact is minor; for linux guests it's slightly larger:
> > 
> > When rebooting a linux guest with 2 displays, the non-primary display will
> > become 'un-ready' during shutdown, and the window will be hidden after 5
> > seconds. From the guest's point-of-view, this is like disconnecting the
> > physical
> > monitor, so upon restart there will only be a single display.
> > 
> > When rebooting a Windows guest with 2 displays, the non-primary display
> > will
> > also be hidden during shutdown.  But multiple monitors work differently in
> > windows guests than linux guests (e.g. multiple QXL devices vs multiple
> > displays
> > per QXL device). Because of this, from the guest's point-of-view, there are
> > still 2 physical monitors attached. So after reboot, the second display
> > will
> > be
> > re-enabled and virt-viewer will show the second display.
> > ---
> > 
> > There are some drawbacks to this workaround, so I'd appreciate some
> > comments
> > on
> > the general approach in addition to reviewing the code itself.
> > 
> >  src/virt-viewer-window.c | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> > index af4d0bc..0ad6766 100644
> > --- a/src/virt-viewer-window.c
> > +++ b/src/virt-viewer-window.c
> > @@ -104,6 +104,7 @@ struct _VirtViewerWindowPrivate {
> >      gboolean auto_resize;
> >      gboolean fullscreen;
> >      gchar *subtitle;
> > +    guint source_delayed_hide;
> >  };
> >  
> >  static void
> > @@ -1205,6 +1206,17 @@
> > virt_viewer_window_set_usb_options_sensitive(VirtViewerWindow *self,
> > gboolean se
> >      gtk_widget_set_visible(priv->toolbar_usb_device_selection, sensitive);
> >  }
> >  
> > +static gboolean
> > +delayed_hide_window(gpointer user_data)
> > +{
> > +    VirtViewerWindow *self = VIRT_VIEWER_WINDOW(user_data);
> > +
> > +    virt_viewer_window_hide(self);
> > +    self->priv->source_delayed_hide = 0;
> > +
> > +    return FALSE;
> > +}
> > +
> >  static void
> >  display_show_hint(VirtViewerDisplay *display,
> >                    GParamSpec *pspec G_GNUC_UNUSED,
> > @@ -1219,6 +1231,21 @@ display_show_hint(VirtViewerDisplay *display,
> >      gtk_widget_set_sensitive(GTK_WIDGET(gtk_builder_get_object(self->priv->builder,
> >      "menu-send")), hint);
> >      gtk_widget_set_sensitive(GTK_WIDGET(gtk_builder_get_object(self->priv->builder,
> >      "menu-file-screenshot")), hint);
> >      gtk_widget_set_sensitive(self->priv->toolbar_send_key, hint);
> > +
> > +    /* schedule the window to be hidden after a short timeout when it
> > become
> > +     * 'un-ready', e.g. when a display is disabled via the guest's display
> > +     * configuration tools */
> > +    if (!hint) {
> > +        if (!self->priv->source_delayed_hide) {
> > +            self->priv->source_delayed_hide = g_timeout_add_seconds(5,
> > delayed_hide_window, self);
> > +        }
> > +    } else {
> > +        if (self->priv->source_delayed_hide) {
> > +            g_source_remove(self->priv->source_delayed_hide);
> > +            self->priv->source_delayed_hide = 0;
> > +        }
> > +    }
> > +
> >  }
> >  static gboolean
> >  window_key_pressed (GtkWidget *widget G_GNUC_UNUSED,
> > --
> > 1.8.3.1
> > 
> > _______________________________________________
> > virt-tools-list mailing list
> > virt-tools-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/virt-tools-list
> > 
>