On Wed, 2014-09-24 at 12:07 +0200, Christophe Fergeau wrote: > Hey, > > On Tue, Sep 23, 2014 at 04:57:41PM -0500, Jonathon Jongsma wrote: > > When using the fullscreen display mapping configuration file, extra > > monitors could end up enabled by mistake. This was because > > virt_viewer_app_get_initial_monitor_for_display would end up returning > > Nmonitor = Ndisplay when the display map hash lookup failed. In > > reality, when a display map is specified, but the hash lookup fails, > > the display should not be enabled. This function now returns -1 to > > distinguish this case, and the display is not enabled when this value is > > returned. > > --- > > > > The implementation was apparently incomplete. This patch attempts to fix an > > issue that was found during testing: > > see https://bugzilla.redhat.com/show_bug.cgi?id=1129477#c9 > > > > src/virt-viewer-app.c | 22 ++++++++++++---------- > > src/virt-viewer-session-spice.c | 3 +++ > > 2 files changed, 15 insertions(+), 10 deletions(-) > > > > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c > > index a890bf6..12d01eb 100644 > > --- a/src/virt-viewer-app.c > > +++ b/src/virt-viewer-app.c > > @@ -314,8 +314,11 @@ gint virt_viewer_app_get_initial_monitor_for_display(VirtViewerApp* self, gint d > > > > if (self->priv->initial_display_map) { > > gpointer value = NULL; > > - if (g_hash_table_lookup_extended(self->priv->initial_display_map, GINT_TO_POINTER(display), NULL, &value)) > > + if (g_hash_table_lookup_extended(self->priv->initial_display_map, GINT_TO_POINTER(display), NULL, &value)) { > > monitor = GPOINTER_TO_INT(value); > > + } else { > > + monitor = -1; > > + } > > } > > My understanding is that self->priv->initial_display_map is only set > when a display configuration file is in used? Yes, that's true > > > > > return monitor; > > @@ -326,13 +329,14 @@ app_window_try_fullscreen(VirtViewerApp *self G_GNUC_UNUSED, > > VirtViewerWindow *win, gint nth) > > { > > GdkScreen *screen = gdk_screen_get_default(); > > - > > - if (nth >= gdk_screen_get_n_monitors(screen)) { > > - g_debug("skipping display %d", nth); > > + gint monitor = virt_viewer_app_get_initial_monitor_for_display(self, nth); > > + if (monitor == -1 || monitor >= gdk_screen_get_n_monitors(screen)) { > > + g_debug("skipping fullscreen for display %d", nth); > > + virt_viewer_window_hide(win); > > return; > > } > > > > - virt_viewer_window_enter_fullscreen(win, nth); > > + virt_viewer_window_enter_fullscreen(win, monitor); > > } > > > > > > @@ -449,8 +453,7 @@ void virt_viewer_app_set_uuid_string(VirtViewerApp *self, const gchar *uuid_stri > > > > g_hash_table_iter_init(&iter, self->priv->windows); > > while (g_hash_table_iter_next(&iter, NULL, &value)) { > > - gint monitor = virt_viewer_app_get_initial_monitor_for_display(self, i); > > - app_window_try_fullscreen(self, VIRT_VIEWER_WINDOW(value), monitor); > > + app_window_try_fullscreen(self, VIRT_VIEWER_WINDOW(value), i); > > i++; > > } > > For what it's worth, the move of > virt_viewer_app_get_initial_monitor_for_display() in > app_window_try_fullscreen() could have been done in a preparatory > commit. > Good point, I'll pull that out into a separate patch. Jonathon _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
