[virt-tools-list] [PATCH virt-viewer 2/3] spice: implement can-auto-resize

[Daniel P. Berrange]

On Wed, Mar 12, 2014 at 01:05:15PM -0400, Marc-André Lureau wrote:
> 
> 
> ----- Original Message -----
> > On Wed, Mar 12, 2014 at 05:42:03PM +0100, Marc-André Lureau wrote:
> > > Always return TRUE for Spice displays. See rationale in method comment.
> > > ---
> > >  src/virt-viewer-display-spice.c | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/src/virt-viewer-display-spice.c
> > > b/src/virt-viewer-display-spice.c
> > > index d13fbda..81ce6de 100644
> > > --- a/src/virt-viewer-display-spice.c
> > > +++ b/src/virt-viewer-display-spice.c
> > > @@ -56,6 +56,7 @@ static GdkPixbuf
> > > *virt_viewer_display_spice_get_pixbuf(VirtViewerDisplay *displa
> > >  static void virt_viewer_display_spice_release_cursor(VirtViewerDisplay
> > >  *display);
> > >  static void virt_viewer_display_spice_close(VirtViewerDisplay *display
> > >  G_GNUC_UNUSED);
> > >  static gboolean virt_viewer_display_spice_selectable(VirtViewerDisplay
> > >  *display);
> > > +static gboolean
> > > virt_viewer_display_spice_can_auto_resize(VirtViewerDisplay *display);
> > >  
> > >  static void
> > >  virt_viewer_display_spice_finalize(GObject *obj)
> > > @@ -80,6 +81,7 @@
> > > virt_viewer_display_spice_class_init(VirtViewerDisplaySpiceClass *klass)
> > >      dclass->release_cursor = virt_viewer_display_spice_release_cursor;
> > >      dclass->close = virt_viewer_display_spice_close;
> > >      dclass->selectable = virt_viewer_display_spice_selectable;
> > > +    dclass->can_auto_resize = virt_viewer_display_spice_can_auto_resize;
> > >  
> > >      g_type_class_add_private(klass,
> > >      sizeof(VirtViewerDisplaySpicePrivate));
> > >  }
> > > @@ -335,6 +337,18 @@ virt_viewer_display_spice_selectable(VirtViewerDisplay
> > > *self)
> > >      return agent_connected;
> > >  }
> > >  
> > > +static gboolean
> > > +virt_viewer_display_spice_can_auto_resize(VirtViewerDisplay *self
> > > G_GNUC_UNUSED)
> > > +{
> > > +    /*
> > > +     * with xorg driver and windows, it needs the Spice agent but with
> > > +     * drm/kms driver, it is no longer required, however it requires
> > > +     * gnome-settings-daemon (or a similar service).  There is no easy
> > > +     * way to guess all that from client side, just assume it is
> > > +     * working:
> > > +     */
> > 
> > I don't think this is good enough. Given that QXL provides a plain VGA mode
> > fallback, applications like virt-manager/rhev/openstack will often enable
> > SPICE+QXL unconditionally for all guests. As such we cannot reasonably
> > assume that there is a SPICE agent running in all guests, particularly if
> > we're going to use this flag to disable functionality of the virt-viewer
> > client.
> > 
> > If we want to do this then we need a way to query from the SPICE server
> > whether the guest agent is actually present & operational.
> 
> You are refusing a change that is mainly aiming at VNC display.
> Currently, the item state is always sensitive. This series will just disable
> it for VNC.
> 
> And I really don't think we will ever have a reliable way to know whether
> the Spice guest will be able to auto-resize, as I tried to explain...

The rationale for disabling it is that the menu option doesn't do anything
in some cases. Just disabling it for VNC isn't really a complete fix for
the problem there - we're still left with a option that won't do anything
in some cases, merely fewer cases than before.

I actually question what the point in having this menu option is at all ?
Why would someone ever want to uncheck the 'automatically resize' menu
option if their guest actually supported this.

IMHO a better fix would be to simply remove this menu option completely
as it serves little purpose and we can't even inform the user whether
change it will have any effect at all.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|