[virt-tools-list] [PATCH virt-viewer] session-spice: Create displays when channel connects

[Fabiano Fidencio]

----- Original Message -----
> From: "Fabiano Fidencio" <ffidenci at redhat.com>
> To: "Pavel Grunt" <pgrunt at redhat.com>
> Cc: virt-tools-list at redhat.com
> Sent: Wednesday, July 29, 2015 9:56:23 AM
> Subject: Re: [virt-tools-list] [PATCH virt-viewer] session-spice:	Create	displays when channel connects
> 
> 
> 
> ----- Original Message -----
> > From: "Pavel Grunt" <pgrunt at redhat.com>
> > To: virt-tools-list at redhat.com
> > Sent: Tuesday, July 28, 2015 6:31:41 PM
> > Subject: [virt-tools-list] [PATCH virt-viewer] session-spice: Create
> > 	displays when channel connects
> > 
> > The display channel emits "notify::monitor" only when its display is
> > enabled. If the display is not enabled then it is not listed in the
> > "View -> Displays" menu, because display widgets are created as a
> > reaction to the "notify::monitor".
> > Creating display widgets at the moment the channel connects makes sure
> > that displays are listed the "View -> Displays" menu and can be enabled.
> > 
> > Resolves:
> > https://bugs.freedesktop.org/show_bug.cgi?id=91489
> > ---
> >  src/virt-viewer-session-spice.c | 29 ++++++++++++++++++++++++++---
> >  1 file changed, 26 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/virt-viewer-session-spice.c
> > b/src/virt-viewer-session-spice.c
> > index 9976291..67d5a3f 100644
> > --- a/src/virt-viewer-session-spice.c
> > +++ b/src/virt-viewer-session-spice.c
> > @@ -766,9 +766,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_create_displays_for_channel(VirtViewerSessionSpice
> > *self,
> > +                                                      SpiceChannel
> > *channel)
> >  {
> >      GArray *monitors = NULL;
> >      GPtrArray *displays = NULL;
> > @@ -803,6 +802,29 @@
> > virt_viewer_session_spice_display_monitors(SpiceChannel
> > *channel,
> >          }
> >      }
> >  
> > +    g_clear_pointer(&monitors, g_array_unref);
> > +}
> > +
> > +static void
> > +virt_viewer_session_spice_display_monitors(SpiceChannel *channel,
> > +                                           GParamSpec *pspec
> > G_GNUC_UNUSED,
> > +                                           VirtViewerSessionSpice *self)
> > +{
> > +    GArray *monitors = NULL;
> > +    GPtrArray *displays = NULL;
> > +    GtkWidget *display;
> > +    guint i, monitors_max;
> > +
> > +    g_object_get(channel,
> > +                 "monitors", &monitors,
> > +                 "monitors-max", &monitors_max,
> > +                 NULL);
> > +    g_return_if_fail(monitors != NULL);
> > +    g_return_if_fail(monitors->len <= monitors_max);
> > +
> > +    virt_viewer_session_spice_create_displays_for_channel(self, channel);
> > +    displays = g_object_get_data(G_OBJECT(channel),
> > "virt-viewer-displays");
> > +
> >      for (i = 0; i < monitors->len; i++) {
> >          SpiceDisplayMonitorConfig *monitor = &g_array_index(monitors,
> >          SpiceDisplayMonitorConfig, i);
> >          display = g_ptr_array_index(displays, monitor->id);
> > @@ -856,6 +878,7 @@ virt_viewer_session_spice_channel_new(SpiceSession *s,
> >      }
> >  
> >      if (SPICE_IS_DISPLAY_CHANNEL(channel)) {
> > +        virt_viewer_session_spice_create_displays_for_channel(self,
> > channel);
> >          g_signal_emit_by_name(session, "session-initialized");
> >  
> >          virt_viewer_signal_connect_object(channel, "notify::monitors",
> > --
> > 2.4.6
> > 
> > _______________________________________________
> > virt-tools-list mailing list
> > virt-tools-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/virt-tools-list
> > 
> 
> Nice catch!
> ACK!

Taking my ACK back.
It may cause some problem related to fullscreen monitor mapping, so some tests on this area could be done and well described in the commit log before have it pushed upstream.
Probably Jonathon will have a clearer idea about what problems it can cause, but from a small (face to face) discussions with Pavel we realized that it may cause (it's racy, actually, so it may or may not be related to the patch) one screen showing up instead of two.

> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
> 

Best Regards,
--
Fabiano Fidêncio