From dabbc8d5bdef57d018488e55f82468142bce693f Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Thu, 15 Mar 2018 21:22:14 -0400 Subject: [PATCH] engine: Remove centralized conn.open handling Move connection opening logic to each caller, since needs are slightly different. --- tests/uitests/connect.py | 4 +- virt-manager | 2 +- virtManager/baseclass.py | 5 + virtManager/connect.py | 46 ++++++-- virtManager/connectauth.py | 67 +++++++++++ virtManager/connection.py | 25 ++-- virtManager/engine.py | 232 ++++++++----------------------------- virtManager/error.py | 9 +- virtManager/manager.py | 14 ++- virtManager/systray.py | 4 - 10 files changed, 190 insertions(+), 218 deletions(-) diff --git a/tests/uitests/connect.py b/tests/uitests/connect.py index e3535950..554974b4 100644 --- a/tests/uitests/connect.py +++ b/tests/uitests/connect.py @@ -57,7 +57,7 @@ class VMMConnect(uiutils.UITestCase): urilabel.text == "xen+tcp://fribuser@%s:12345/" % fakehost) connect.click() - uiutils.check_in_loop(lambda: win.showing is False) + uiutils.check_in_loop(lambda: win.showing is True) c = self.app.root.find_fuzzy(fakehost, "table cell") uiutils.check_in_loop(lambda: "Connecting..." not in c.text, timeout=10) @@ -70,7 +70,7 @@ class VMMConnect(uiutils.UITestCase): # This time say 'yes' connect.click() - uiutils.check_in_loop(lambda: win.showing is False) + uiutils.check_in_loop(lambda: win.showing is True) c = self.app.root.find_fuzzy(fakehost, "table cell") uiutils.check_in_loop(lambda: "Connecting..." not in c.text, timeout=10) diff --git a/virt-manager b/virt-manager index 8b9989c2..79fcf6cd 100755 --- a/virt-manager +++ b/virt-manager @@ -52,7 +52,7 @@ def _show_startup_error(msg, details): logging.debug("Error starting virt-manager: %s\n%s", msg, details, exc_info=True) from virtManager.error import vmmErrorDialog - err = vmmErrorDialog() + err = vmmErrorDialog(None) title = _("Error starting Virtual Machine Manager") err.show_err(title + ": " + msg, details=details, diff --git a/virtManager/baseclass.py b/virtManager/baseclass.py index 2f9d6e29..6e0402c8 100644 --- a/virtManager/baseclass.py +++ b/virtManager/baseclass.py @@ -74,6 +74,11 @@ class vmmGObject(GObject.GObject): if config.vmmConfig.is_initialized() and self._leak_check: self.config.add_object(self.object_key) + def _get_err(self): + from . import error + return error.vmmErrorDialog.get_instance() + err = property(_get_err) + def cleanup(self): if self.__cleaned_up: return diff --git a/virtManager/connect.py b/virtManager/connect.py index e5dc7857..e5283f38 100644 --- a/virtManager/connect.py +++ b/virtManager/connect.py @@ -29,6 +29,7 @@ from gi.repository import Gtk from . import uiutil from .baseclass import vmmGObjectUI +from .connmanager import vmmConnectionManager (HV_QEMU, HV_XEN, @@ -72,11 +73,6 @@ class vmmConnect(vmmGObjectUI): def is_initialized(cls): return bool(cls._instance) - __gsignals__ = { - "completed": (vmmGObjectUI.RUN_FIRST, None, [str, bool]), - "cancelled": (vmmGObjectUI.RUN_FIRST, None, []), - } - def __init__(self): vmmGObjectUI.__init__(self, "connect.ui", "vmm-open-connection") self._cleanup_on_app_close() @@ -144,7 +140,6 @@ class vmmConnect(vmmGObjectUI): def cancel(self, ignore1=None, ignore2=None): logging.debug("Cancelling open connection") self.close() - self.emit("cancelled") return 1 def close(self, ignore1=None, ignore2=None): @@ -158,14 +153,13 @@ class vmmConnect(vmmGObjectUI): self.browser = None - def show(self, parent, reset_state=True): + def show(self, parent): logging.debug("Showing open connection") if self.topwin.is_visible(): self.topwin.present() return - if reset_state: - self.reset_state() + self.reset_state() self.topwin.set_transient_for(parent) self.topwin.present() self.start_browse() @@ -463,21 +457,49 @@ class vmmConnect(vmmGObjectUI): return True + def _conn_open_completed(self, conn, ConnectError): + if not ConnectError: + self.close() + self.reset_finish_cursor() + return True + + msg, details, title = ConnectError + msg += "\n\n" + msg += _("Would you still like to remember this connection?") + + + remember = self.err.show_err(msg, details, title, + buttons=Gtk.ButtonsType.YES_NO, + dialog_type=Gtk.MessageType.QUESTION, modal=True) + self.reset_finish_cursor() + if remember: + self.close() + else: + vmmConnectionManager.get_instance().remove_conn(conn.get_uri()) + return True + def open_conn(self, ignore): if not self.validate(): return auto = False if self.widget("autoconnect").get_sensitive(): - auto = self.widget("autoconnect").get_active() + auto = bool(self.widget("autoconnect").get_active()) if self.widget("uri-label").is_visible(): uri = self.generate_uri() else: uri = self.widget("uri-entry").get_text() logging.debug("Generate URI=%s, auto=%s", uri, auto) - self.close() - self.emit("completed", uri, auto) + + conn = vmmConnectionManager.get_instance().add_conn(uri) + conn.set_autoconnect(auto) + if conn.is_active(): + return + + conn.connect_opt_out("open-completed", self._conn_open_completed) + self.set_finish_cursor() + conn.open() def sanitize_hostname(self, host): if host == "linux" or host == "localhost": diff --git a/virtManager/connectauth.py b/virtManager/connectauth.py index d959c870..e50f9aab 100644 --- a/virtManager/connectauth.py +++ b/virtManager/connectauth.py @@ -18,8 +18,10 @@ # MA 02110-1301 USA. # +import collections import logging import os +import re import time from gi.repository import GLib @@ -165,3 +167,68 @@ def acquire_tgt(): logging.info("Cannot acquire tgt %s", str(e)) ret = False return ret + + +def connect_error(conn, errmsg, tb, warnconsole): + """ + Format connection error message + """ + errmsg = errmsg.strip(" \n") + tb = tb.strip(" \n") + hint = "" + show_errmsg = True + config = conn.config + + if conn.is_remote(): + logging.debug("connect_error: conn transport=%s", + conn.get_uri_transport()) + if re.search(r"nc: .* -- 'U'", tb): + hint += _("The remote host requires a version of netcat/nc " + "which supports the -U option.") + show_errmsg = False + elif (conn.get_uri_transport() == "ssh" and + re.search(r"ssh-askpass", tb)): + + askpass = (config.askpass_package and + config.askpass_package[0] or + "openssh-askpass") + hint += _("You need to install %s or " + "similar to connect to this host.") % askpass + show_errmsg = False + else: + hint += _("Verify that the 'libvirtd' daemon is running " + "on the remote host.") + + elif conn.is_xen(): + hint += _("Verify that:\n" + " - A Xen host kernel was booted\n" + " - The Xen service has been started") + + else: + if warnconsole: + hint += _("Could not detect a local session: if you are " + "running virt-manager over ssh -X or VNC, you " + "may not be able to connect to libvirt as a " + "regular user. Try running as root.") + show_errmsg = False + elif re.search(r"libvirt-sock", tb): + hint += _("Verify that the 'libvirtd' daemon is running.") + show_errmsg = False + + msg = _("Unable to connect to libvirt %s." % conn.get_uri()) + if show_errmsg: + msg += "\n\n%s" % errmsg + if hint: + msg += "\n\n%s" % hint + + msg = msg.strip("\n") + details = msg + details += "\n\n" + details += "Libvirt URI is: %s\n\n" % conn.get_uri() + details += tb + + title = _("Virtual Machine Manager Connection Failure") + + ConnectError = collections.namedtuple("ConnectError", + ["msg", "details", "title"]) + return ConnectError(msg, details, title) diff --git a/virtManager/connection.py b/virtManager/connection.py index fd2cc05c..e295e013 100644 --- a/virtManager/connection.py +++ b/virtManager/connection.py @@ -174,9 +174,7 @@ class vmmConnection(vmmGObject): "nodedev-removed": (vmmGObject.RUN_FIRST, None, [str]), "resources-sampled": (vmmGObject.RUN_FIRST, None, []), "state-changed": (vmmGObject.RUN_FIRST, None, []), - "connect-error": (vmmGObject.RUN_FIRST, None, - [str, str, bool]), - "priority-tick": (vmmGObject.RUN_FIRST, None, [object]), + "open-completed": (vmmGObject.RUN_FIRST, None, [object]), } (_STATE_DISCONNECTED, @@ -194,6 +192,9 @@ class vmmConnection(vmmGObject): self._backend = virtinst.VirtualConnection(self._uri) self._closing = False + # Error strings are stored here if open() fails + self.connect_error = None + self._init_object_count = None self._init_object_event = None @@ -1051,8 +1052,9 @@ class vmmConnection(vmmGObject): if retry_for_tgt and connectauth.acquire_tgt(): self._do_open(retry_for_tgt=False) - connectError = (str(exc), tb, warnconsole) - return False, connectError + ConnectError = connectauth.connect_error( + self, str(exc), tb, warnconsole) + return False, ConnectError def _populate_initial_state(self): logging.debug("libvirt version=%s", @@ -1102,8 +1104,9 @@ class vmmConnection(vmmGObject): self._init_object_count = None def _open_thread(self): + ConnectError = None try: - is_active, connectError = self._do_open() + is_active, ConnectError = self._do_open() if is_active: self._populate_initial_state() @@ -1112,11 +1115,10 @@ class vmmConnection(vmmGObject): except Exception as e: is_active = False self._schedule_close() - connectError = (str(e), "".join(traceback.format_exc()), False) + ConnectError = connectauth.connect_error(self, str(e), + "".join(traceback.format_exc()), False) - if not is_active: - if connectError: - self.idle_emit("connect-error", *connectError) + self.idle_emit("open-completed", ConnectError) ####################### @@ -1431,7 +1433,8 @@ class vmmConnection(vmmGObject): def schedule_priority_tick(self, **kwargs): - self.idle_emit("priority-tick", kwargs) + from .engine import vmmEngine + vmmEngine.get_instance().schedule_priority_tick(self, kwargs) def tick_from_engine(self, *args, **kwargs): e = None diff --git a/virtManager/engine.py b/virtManager/engine.py index 8f6b4a54..93eeb572 100644 --- a/virtManager/engine.py +++ b/virtManager/engine.py @@ -19,7 +19,6 @@ # import logging -import re import queue import threading import traceback @@ -30,9 +29,8 @@ from gi.repository import Gtk from . import packageutils from .baseclass import vmmGObject -from .connmanager import vmmConnectionManager from .connect import vmmConnect -from .error import vmmErrorDialog +from .connmanager import vmmConnectionManager from .inspection import vmmInspection DETAILS_PERF = 1 @@ -64,8 +62,6 @@ class vmmEngine(vmmGObject): def __init__(self): vmmGObject.__init__(self) - self.err = vmmErrorDialog() - self._exiting = False self._window_count = 0 @@ -88,7 +84,6 @@ class vmmEngine(vmmGObject): def _cleanup(self): - self.err = None if self._timer is not None: GLib.source_remove(self._timer) @@ -110,9 +105,6 @@ class vmmEngine(vmmGObject): self._timer_changed_cb)) self._schedule_timer() - for _uri in self._connobjs: - self._add_conn(_uri, False, force_init=True) - self._tick_thread.start() self._tick() @@ -168,24 +160,26 @@ class vmmEngine(vmmGObject): manager.set_startup_error(msg) return - warnmsg = _("The 'libvirtd' service will need to be started.\n\n" - "After that, virt-manager will connect to libvirt on\n" - "the next application start up.") + # packagekit API via gnome-software doesn't even work nicely these + # days. Not sure what the state of this warning is... + # + # warnmsg = _("The 'libvirtd' service will need to be started.\n\n" + # "After that, virt-manager will connect to libvirt on\n" + # "the next application start up.") + # if not connected and not libvirtd_started: + # manager.err.ok(_("Libvirt service must be started"), warnmsg) - # Do the initial connection in an idle callback, so the - # packagekit async dialog has a chance to go away def idle_connect(): - libvirtd_started = packageutils.start_libvirtd() - connected = False - try: - self._connect_to_uri(tryuri, autoconnect=True) - connected = True - except Exception: - logging.exception("Error connecting to %s", tryuri) - - if not connected and not libvirtd_started: - manager.err.ok(_("Libvirt service must be started"), warnmsg) + def _open_completed(c, ConnectError): + if ConnectError: + self._handle_conn_error(c, ConnectError) + return True + packageutils.start_libvirtd() + conn = vmmConnectionManager.get_instance().add_conn(tryuri) + conn.set_autoconnect(True) + conn.connect_opt_out("open-completed", _open_completed) + conn.open() self.idle_add(idle_connect) def _autostart_conns(self): @@ -205,10 +199,13 @@ class vmmEngine(vmmGObject): else: connections_queue.put(auto_conns.pop(0)) - def state_change_cb(conn): - if conn.is_active(): - add_next_to_queue() - return True + def conn_open_completed(_conn, ConnectError): + # Explicitly ignore connection errors, we've done that + # for a while and it can be noisy + logging.debug("Autostart connection error: %s", + ConnectError.details) + add_next_to_queue() + return True def handle_queue(): while True: @@ -222,8 +219,8 @@ class vmmEngine(vmmGObject): continue conn = self._connobjs[uri] - conn.connect_opt_out("state-changed", state_change_cb) - self.idle_add(self._connect_to_uri, uri) + conn.connect_opt_out("open-completed", conn_open_completed) + self.idle_add(conn.open) add_next_to_queue() self._start_thread(handle_queue, "Conn autostart thread") @@ -307,7 +304,8 @@ class vmmEngine(vmmGObject): self._tick_counter, obj, kwargs)) - def _schedule_priority_tick(self, conn, kwargs): + def schedule_priority_tick(self, conn, kwargs): + # Called directly from connection self._add_obj_to_tick_queue(conn, True, **kwargs) def _tick(self): @@ -403,128 +401,6 @@ class vmmEngine(vmmGObject): # false positive self.idle_add(_do_exit) - def _add_conn(self, uri, probe, force_init=False): - is_init = (uri not in self._connobjs) - conn = vmmConnectionManager.get_instance().add_conn(uri) - if is_init or force_init: - conn.connect("connect-error", self._connect_error) - conn.connect("priority-tick", self._schedule_priority_tick) - setattr(conn, "_from_connect_wizard", probe) - return conn - - def _connect_to_uri(self, uri, autoconnect=None, probe=False): - conn = self._add_conn(uri, probe=probe) - - if autoconnect is not None: - conn.set_autoconnect(bool(autoconnect)) - - conn.open() - - def _connect_error(self, conn, errmsg, tb, warnconsole): - errmsg = errmsg.strip(" \n") - tb = tb.strip(" \n") - hint = "" - show_errmsg = True - - if conn.is_remote(): - logging.debug("connect_error: conn transport=%s", - conn.get_uri_transport()) - if re.search(r"nc: .* -- 'U'", tb): - hint += _("The remote host requires a version of netcat/nc " - "which supports the -U option.") - show_errmsg = False - elif (conn.get_uri_transport() == "ssh" and - re.search(r"ssh-askpass", tb)): - - askpass = (self.config.askpass_package and - self.config.askpass_package[0] or - "openssh-askpass") - hint += _("You need to install %s or " - "similar to connect to this host.") % askpass - show_errmsg = False - else: - hint += _("Verify that the 'libvirtd' daemon is running " - "on the remote host.") - - elif conn.is_xen(): - hint += _("Verify that:\n" - " - A Xen host kernel was booted\n" - " - The Xen service has been started") - - else: - if warnconsole: - hint += _("Could not detect a local session: if you are " - "running virt-manager over ssh -X or VNC, you " - "may not be able to connect to libvirt as a " - "regular user. Try running as root.") - show_errmsg = False - elif re.search(r"libvirt-sock", tb): - hint += _("Verify that the 'libvirtd' daemon is running.") - show_errmsg = False - - msg = _("Unable to connect to libvirt %s." % conn.get_uri()) - if show_errmsg: - msg += "\n\n%s" % errmsg - if hint: - msg += "\n\n%s" % hint - - msg = msg.strip("\n") - details = msg - details += "\n\n" - details += "Libvirt URI is: %s\n\n" % conn.get_uri() - details += tb - - _from_connect_wizard = getattr(conn, "_from_connect_wizard", False) - if _from_connect_wizard: - msg += "\n\n" - msg += _("Would you still like to remember this connection?") - - title = _("Virtual Machine Manager Connection Failure") - if _from_connect_wizard: - remember_connection = self.err.show_err(msg, details, title, - buttons=Gtk.ButtonsType.YES_NO, - dialog_type=Gtk.MessageType.QUESTION, modal=True) - if remember_connection: - setattr(conn, "_from_connect_wizard", False) - else: - self._edit_connect(conn.get_uri()) - else: - if self._can_exit(): - self.err.show_err(msg, details, title, modal=True) - self._exit_app_if_no_windows() - else: - self.err.show_err(msg, details, title) - - - ################################## - # callbacks and dialog launchers # - ################################## - - def _connect_completed(self, _src, uri, autoconnect): - self._connect_to_uri(uri, autoconnect, probe=True) - - def _connect_cancelled(self, _src): - if not self._connobjs: - self.exit_app() - - def _show_connect_dialog(self, src, reset_state): - is_init = vmmConnect.is_initialized() - obj = vmmConnect.get_instance(src) - if not is_init: - obj.connect("completed", self._connect_completed) - obj.connect("cancelled", self._connect_cancelled) - obj.show(src.topwin, reset_state) - - def do_show_connect(self, src, reset_state=True): - try: - self._show_connect_dialog(src, reset_state) - except Exception as e: - src.err.show_err(_("Error launching connect dialog: %s") % str(e)) - - def _edit_connect(self, uri): - vmmConnectionManager.get_instance().remove_conn(uri) - self.do_show_connect(self._get_manager(), reset_state=False) - ########################################## # Window launchers from virt-manager cli # @@ -620,28 +496,16 @@ class vmmEngine(vmmGObject): # In case of cli error, we may need to exit the app self._exit_app_if_no_windows() - def _cli_conn_connected_cb(self, conn, uri, show_window, domain): - try: - ignore = conn - - if conn.is_disconnected(): - raise RuntimeError("failed to connect to cli uri=%s" % uri) - - if conn.is_active(): - self._launch_cli_window(uri, show_window, domain) - return True - - return False - except Exception: - # In case of cli error, we may need to exit the app - logging.debug("Error in cli connection callback", exc_info=True) - self._exit_app_if_no_windows() - return True + def _handle_conn_error(self, _conn, ConnectError): + msg, details, title = ConnectError + modal = self._can_exit() + self.err.show_err(msg, details, title, modal=modal) + self._exit_app_if_no_windows() def _do_handle_cli_command(self, actionobj, variant): ignore = actionobj uri = variant[0] - show_window = variant[1] + show_window = variant[1] or self.CLI_SHOW_MANAGER domain = variant[2] logging.debug("processing cli command uri=%s show_window=%s domain=%s", @@ -651,21 +515,21 @@ class vmmEngine(vmmGObject): self._get_manager().show() return - conn = self._add_conn(uri, False) + conn = vmmConnectionManager.get_instance().add_conn(uri) + if conn.is_active(): + self.idle_add(self._launch_cli_window, + uri, show_window, domain) + return - if conn.is_disconnected(): - # Schedule connection open - self.idle_add(self._connect_to_uri, uri) - - if show_window: - if conn.is_active(): - self.idle_add(self._launch_cli_window, - uri, show_window, domain) + def _open_completed(_c, ConnectError): + if ConnectError: + self._handle_conn_error(conn, ConnectError) else: - conn.connect_opt_out("state-changed", - self._cli_conn_connected_cb, uri, show_window, domain) - else: - self._show_manager_uri(uri) + self._launch_cli_window(uri, show_window, domain) + return True + + conn.connect_opt_out("open-completed", _open_completed) + conn.open() def _handle_cli_command(self, actionobj, variant): try: diff --git a/virtManager/error.py b/virtManager/error.py index 4a913545..99624234 100644 --- a/virtManager/error.py +++ b/virtManager/error.py @@ -49,7 +49,14 @@ def _launch_dialog(dialog, primary_text, secondary_text, title, class vmmErrorDialog(vmmGObject): - def __init__(self, parent=None): + # singleton instance for non-UI classes + @classmethod + def get_instance(cls): + if not cls._instance: + cls._instance = cls(None) + return cls._instance + + def __init__(self, parent): vmmGObject.__init__(self) self._parent = parent self._simple = None diff --git a/virtManager/manager.py b/virtManager/manager.py index a3b2068e..d1dee650 100644 --- a/virtManager/manager.py +++ b/virtManager/manager.py @@ -133,7 +133,7 @@ class vmmManager(vmmGObjectUI): "on_vm_manager_delete_event": self.close, "on_vmm_manager_configure_event": self.window_resized, - "on_menu_file_add_connection_activate": self.new_conn, + "on_menu_file_add_connection_activate": self.open_newconn, "on_menu_new_vm_activate": self.new_vm, "on_menu_file_quit_activate": self.exit_app, "on_menu_file_close_activate": self.close, @@ -458,8 +458,9 @@ class vmmManager(vmmGObjectUI): def exit_app(self, src_ignore=None, src2_ignore=None): vmmEngine.get_instance().exit_app() - def new_conn(self, _src): - vmmEngine.get_instance().do_show_connect(self) + def open_newconn(self, _src): + from .connect import vmmConnect + vmmConnect.get_instance(self).show(self.topwin) def new_vm(self, _src): from .create import vmmCreate @@ -482,6 +483,12 @@ class vmmManager(vmmGObjectUI): def show_vm(self, _src): vmmenu.VMActionUI.show(self, self.current_vm()) + def _conn_open_completed(self, _conn, ConnectError): + if ConnectError: + msg, details, title = ConnectError + self.err.show_err(msg, details, title) + return True + def row_activated(self, _src, *args): ignore = args conn = self.current_conn() @@ -492,6 +499,7 @@ class vmmManager(vmmGObjectUI): if vm: self.show_vm(_src) elif conn.is_disconnected(): + conn.connect_opt_out("open-completed", self._conn_open_completed) conn.open() else: self.show_host(_src) diff --git a/virtManager/systray.py b/virtManager/systray.py index f7fe2dde..b101100e 100644 --- a/virtManager/systray.py +++ b/virtManager/systray.py @@ -26,7 +26,6 @@ from . import vmmenu from .baseclass import vmmGObject from .connmanager import vmmConnectionManager from .engine import vmmEngine -from .error import vmmErrorDialog class vmmSystray(vmmGObject): @@ -41,7 +40,6 @@ class vmmSystray(vmmGObject): self._cleanup_on_app_close() self.topwin = None - self.err = vmmErrorDialog() self.conn_menuitems = {} self.conn_vm_menuitems = {} @@ -68,8 +66,6 @@ class vmmSystray(vmmGObject): self.systray_icon.is_embedded()) def _cleanup(self): - self.err = None - if self.systray_menu: self.systray_menu.destroy() self.systray_menu = None