[virt-tools-list] [PATCH virt-viewer] app: Do not show usbredir button without session

[Pavel Grunt]

Hi,

On Fri, 2016-01-22 at 11:12 -0600, Jonathon Jongsma wrote:
> On Fri, 2016-01-22 at 11:04 -0600, Jonathon Jongsma wrote:
> > On Fri, 2016-01-22 at 17:56 +0100, Fabiano Fidêncio wrote:
> > > On Fri, Jan 22, 2016 at 3:00 PM, Pavel Grunt <pgrunt at redhat.com>
> > > wrote:
> > > > Avoid runtime warnings when waiting for a guest:
> > > > virt-viewer-CRITICAL **:
> > > > virt_viewer_session_usb_device_selection:
> > > > assertion
> > > > 'VIRT_VIEWER_IS_SESSION(self)' failed
> > > > 
> > > > And also do not show the usb redirection button in fullscreen
> > > > if spice-gtk is built without the usb redirection support.
> > > 
> > > I don't understand exactly from where these warnings are coming
> > > from ...
> 
> Forgot to respond to this part. Presumably these warnings only occur
> if you
> click the menu item before the session is set? In what situation is
> this
> possible?
> 
> Also, I don't quite understand the comment about fullscreen. Can you
> expand a
> little bit?

The button is in the fullscreen toolbar. You are able to click on it
when you are in the fullscreen mode and:
 * waiting for a domain: virt-viewer -f -w domain
or * 2+ clients monitors & no usbredir support - on one of monitors the
   toolbar contains the usbredir button, other toolbars are without the
   button

Pavel
> 
> 
> > > 
> > > > ---
> > > >  src/virt-viewer-app.c | 12 ++++++++----
> > > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> > > > index 653b30c..de7874b 100644
> > > > --- a/src/virt-viewer-app.c
> > > > +++ b/src/virt-viewer-app.c
> > > > @@ -903,6 +903,13 @@ viewer_window_focus_out_cb(GtkWindow
> > > > *window
> > > > G_GNUC_UNUSED,
> > > >      return FALSE;
> > > >  }
> > > > 
> > > > +static gboolean
> > > > +virt_viewer_app_has_usbredir(VirtViewerApp *self)
> > > > +{
> > > > +    return virt_viewer_app_has_session(self) &&
> > > > +          
> > > >  virt_viewer_session_get_has_usbredir(virt_viewer_app_get_sessi
> > > > on(self));
> > > > +}
> > > > +
> > > >  static VirtViewerWindow*
> > > >  virt_viewer_app_window_new(VirtViewerApp *self, gint nth)
> > > >  {
> > > > @@ -921,10 +928,7 @@ virt_viewer_app_window_new(VirtViewerApp
> > > > *self, gint
> > > > nth)
> > > >      self->priv->windows = g_list_append(self->priv->windows,
> > > > window);
> > > >      virt_viewer_app_set_window_subtitle(self, window, nth);
> > > >      virt_viewer_app_update_menu_displays(self);
> > > > -    if (self->priv->session) {
> > > > -        virt_viewer_window_set_usb_options_sensitive(window,
> > > > -                    virt_viewer_session_get_has_usbredir(self-
> > > > >priv
> > > > ->session));
> > > > -    }
> > > > +    virt_viewer_window_set_usb_options_sensitive(window,
> > > > virt_viewer_app_has_usbredir(self));
> > > 
> > > ... because you're doing exactly the same check here and then
> > > avoiding them.
> > > Probably I am missing something here. Would you mind to enlighten
> > > it a
> > > bit for me?
> > 
> > I initially thought so as well, but notice that in the previous
> > code,
> > _set_usb_options_sensitive() is not called at all if session is
> > NULL. Now it
> > is
> > called (with FALSE passed as the argument).
> > 
> > 
> > > 
> > > > 
> > > >      g_signal_emit(self, signals[SIGNAL_WINDOW_ADDED], 0,
> > > > window);
> > > > 
> > > > --
> > > > 2.5.0
> > > > 
> > > > _______________________________________________
> > > > 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
> > > 
> > > _______________________________________________
> > > virt-tools-list mailing list
> > > virt-tools-list at redhat.com
> > > https://www.redhat.com/mailman/listinfo/virt-tools-list
> > 
> > _______________________________________________
> > virt-tools-list mailing list
> > virt-tools-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/virt-tools-list