[virt-tools-list] [PATCH] Drop user back to 'open conn' dialog if connecting fails

[Cole Robinson]

On 08/06/2013 09:36 AM, Giuseppe Scrivano wrote:
> Now a new connection is not immediately added to the UI list but
> postponed until the connection is probed.  In case of failure,
> the user can either register the connection or re-try to add a
> connection.
> 
> Split up `add_connection_to_ui' into `make_conn' and `register_conn'
> to handle separately the object creation and its registration in the
> list of connections (ui and conf).
> 
> Solves: https://bugzilla.redhat.com/show_bug.cgi?id=617386
> Signed-off-by: Giuseppe Scrivano <gscrivano at gnu.org>
> ---
>  virtManager/engine.py | 57 ++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 41 insertions(+), 16 deletions(-)
> 

Thanks Giuseppe, generally looks fine, couple comments after playing with it.

After running the 'connect' wizard, the connection isn't immediately added to
the manager connection list, so the user doesn't get that feedback of the
'connecting' text. If I try it with a non-existent remote connection which
might take a long while to time out, it looks like the wizard didn't do
anything. I'd say we can just unconditionally add the URI to gsettings and
stick it in the UI, but if initial connection fails, undo it all.

The other bit is that falling back to the 'connect' wizard, all it's state has
been reset. So if the user just typo'd a hostname or something, they have to
retype it all in. Maybe add a flag to connect.py:show() that disabled the call
to 'reset_state'.

Also, please try to split lines that are longer than 80 characters. I try to
avoid adding them if avoidable. Fixed example:

            rememberConnection = self.err.show_err(msg, details, title,
                    buttons=Gtk.ButtonsType.YES_NO,
                    dialog_type=Gtk.MessageType.QUESTION, async=False)

Thanks,
Cole

> diff --git a/virtManager/engine.py b/virtManager/engine.py
> index 456f597..14dfc67 100644
> --- a/virtManager/engine.py
> +++ b/virtManager/engine.py
> @@ -136,7 +136,8 @@ class vmmEngine(vmmGObject):
>          self.application.add_window(self._appwindow)
>  
>          if self.uri_at_startup:
> -            conn = self.add_conn_to_ui(self.uri_at_startup)
> +            conn = self.make_conn(self.uri_at_startup)
> +            self.register_conn(conn, skip_config=True)
>              if conn and self.uri_cb:
>                  conn.connect_opt_out("resources-sampled", self.uri_cb)
>  
> @@ -222,7 +223,8 @@ class vmmEngine(vmmGObject):
>              return
>          logging.debug("About to connect to uris %s", uris)
>          for uri in uris:
> -            self.add_conn_to_ui(uri)
> +            conn = self.make_conn(uri)
> +            self.register_conn(conn, skip_config=True)
>  
>      def autostart_conns(self):
>          for uri in self.conns:
> @@ -240,6 +242,10 @@ class vmmEngine(vmmGObject):
>          del(self.conns[hvuri]["windowDetails"][vmuuid])
>  
>      def _do_conn_changed(self, conn):
> +        if (conn.get_state() == conn.STATE_ACTIVE and
> +            self.conns[conn.get_uri()]["probeConnection"]):
> +            self.register_conn(conn)
> +
>          if (conn.get_state() == conn.STATE_ACTIVE or
>              conn.get_state() == conn.STATE_CONNECTING):
>              return
> @@ -439,8 +445,7 @@ class vmmEngine(vmmGObject):
>          return
>  
>  
> -    def add_conn_to_ui(self, uri):
> -        # Public method called from virt-manager.py
> +    def make_conn(self, uri, probe=False):
>          conn = self._check_conn(uri)
>          if conn:
>              return conn
> @@ -451,23 +456,30 @@ class vmmEngine(vmmGObject):
>              "windowHost": None,
>              "windowDetails": {},
>              "windowClone": None,
> +            "probeConnection": probe
>          }
>  
>          conn.connect("vm-removed", self._do_vm_removed)
>          conn.connect("state-changed", self._do_conn_changed)
>          conn.connect("connect-error", self._connect_error)
>          conn.connect("priority-tick", self._schedule_priority_tick)
> -        self.emit("conn-added", conn)
>  
>          return conn
>  
>  
> -    def connect_to_uri(self, uri, autoconnect=None, do_start=True):
> -        try:
> -            conn = self.add_conn_to_ui(uri)
> +    def register_conn(self, conn, skip_config=False):
> +        # if `skip_config' then the connection is only showed in the ui and not added to
> +        # the config.
> +        if not skip_config and conn.get_uri() not in (self.config.get_conn_uris() or []):
> +            self.config.add_conn(conn.get_uri())
> +        self.emit("conn-added", conn)
>  
> -            if conn.get_uri() not in (self.config.get_conn_uris() or []):
> -                self.config.add_conn(conn.get_uri())
> +
> +    def connect_to_uri(self, uri, autoconnect=None, do_start=True, probe=False):
> +        try:
> +            conn = self.make_conn(uri, probe=probe)
> +            if not probe:
> +                self.register_conn(conn)
>  
>              if autoconnect is not None:
>                  conn.set_autoconnect(bool(autoconnect))
> @@ -573,7 +585,9 @@ class vmmEngine(vmmGObject):
>                  hint += _("Verify that the 'libvirtd' daemon is running.")
>                  show_errmsg = False
>  
> +        probeConnection = self.conns[conn.get_uri()]["probeConnection"]
>          msg = _("Unable to connect to libvirt.")
> +
>          if show_errmsg:
>              msg += "\n\n%s" % errmsg
>          if hint:
> @@ -585,13 +599,24 @@ class vmmEngine(vmmGObject):
>          details += "Libvirt URI is: %s\n\n" % conn.get_uri()
>          details += tb
>  
> -        title = _("Virtual Machine Manager Connection Failure")
> +        if probeConnection:
> +            msg += "\n\n%s" % _("Would you still like to remember this connection?")
>  
> -        if self._can_exit():
> -            self.err.show_err(msg, details, title, async=False)
> -            self.idle_add(self.exit_app, conn)
> +        title = _("Virtual Machine Manager Connection Failure")
> +        if probeConnection:
> +            rememberConnection = self.err.show_err(msg, details, title, buttons=Gtk.ButtonsType.YES_NO,
> +                                                   dialog_type=Gtk.MessageType.QUESTION, async=False)
> +            if rememberConnection:
> +                self.conns[conn.get_uri()]["probeConnection"] = False
> +                self.register_conn(conn)
> +            else:
> +                self.idle_add(self._do_show_connect, self.windowManager)
>          else:
> -            self.err.show_err(msg, details, title)
> +            if self._can_exit():
> +                self.err.show_err(msg, details, title, async=False)
> +                self.idle_add(self.exit_app, conn)
> +            else:
> +                self.err.show_err(msg, details, title)
>  
>      ####################
>      # Dialog launchers #
> @@ -648,7 +673,7 @@ class vmmEngine(vmmGObject):
>  
>          def completed(src, uri, autoconnect):
>              ignore = src
> -            return self.connect_to_uri(uri, autoconnect)
> +            return self.connect_to_uri(uri, autoconnect, probe=True)
>  
>          def cancelled(src):
>              if len(self.conns.keys()) == 0:
>