[virt-tools-list] [PATCH] Use spice by default if SpiceGtk is available

[Cole Robinson]

Hi Marc,

I think I'd like to do this in a less hardcoded way. Add a dropdown in
the preferences dialog for default graphics type for new guests with an
option for VNC or SPICE. Upstream for now I'd like to default to keep
the default as VNC, though for fedora we can just tweak a gconf
parameter in the package to default to SPICE. Also helps if other
distros don't want to change the default.

We will still use VNC if spice-gtk isn't available or the libvirt
connection doesn't support it, maybe with a warning at the end of the
new vm dialog (not required for a first patch).

I'm going to work on a patch to easily change from VNC to SPICE for an
existing VM to make playing with this easier.

On 03/15/2011 08:34 AM, Marc-André Lureau wrote:
> ---
>  src/virtManager/config.py |    9 +++++++++
>  src/virtManager/create.py |   12 ++++++++----
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/src/virtManager/config.py b/src/virtManager/config.py
> index b9bb275..db3296f 100644
> --- a/src/virtManager/config.py
> +++ b/src/virtManager/config.py
> @@ -325,6 +325,15 @@ class vmmConfig(object):
>          except:
>              return False
>  
> +    # Check whether we have SpiceGtk.
> +    # TODO: check if QEMU/libvirt supports it too
> +    def spice_supported(self):
> +        try:
> +            import SpiceClientGtk
> +            return True
> +        except:
> +            return False
> +

We already have similar logic elsewhere, you can basically do

spice_supported = bool(config.get_spice_error())

To check libvirt support, virtinst has a support check:

virtinst.support.check_conn_support(conn,
  virtinst.support.SUPPORT_CONN_HV_GRAPHICS_SPICE)

though you don't want to do that check in config.py since you'd have to
pass in a connection

>      # Keys preferences
>      def get_keys_combination(self, syms=False):
>          val = self.conf.get_string(self.conf_dir + "/keys/grab-keys")
> diff --git a/src/virtManager/create.py b/src/virtManager/create.py
> index 5062085..74ecded 100644
> --- a/src/virtManager/create.py
> +++ b/src/virtManager/create.py
> @@ -1174,7 +1174,6 @@ class vmmCreate(vmmGObjectUI):
>      def build_guest(self, installer, name):
>          guest = installer.guest_from_installer()
>          guest.name = name
> -
>          # Generate UUID
>          try:
>              guest.uuid = virtinst.util.uuidToString(virtinst.util.randomUUID())

Please remove, I like whitespace between logical chunks

> @@ -1185,9 +1184,14 @@ class vmmCreate(vmmGObjectUI):
>  
>          # Set up graphics device
>          try:
> -            guest.add_device(virtinst.VirtualGraphics(
> -                                        type=virtinst.VirtualGraphics.TYPE_VNC,
> -                                        conn=guest.conn))
> +            if self.config.spice_supported():
> +                guest.add_device(virtinst.VirtualGraphics(
> +                        type=virtinst.VirtualGraphics.TYPE_SPICE,
> +                        conn=guest.conn))
> +            else:
> +                guest.add_device(virtinst.VirtualGraphics(
> +                        type=virtinst.VirtualGraphics.TYPE_VNC,
> +                        conn=guest.conn))

I'd prefer only initializing the device in one place, so

if spice:
  gtype = TYPE_SPICE
else:
  gtype = TYPE_VNC
guest.add_device(Graphics(gtype))

Thanks,
Cole

>              guest.add_device(virtinst.VirtualVideoDevice(conn=guest.conn))
>          except Exception, e:
>              self.err.show_err(_("Error setting up graphics device:") + str(e),