virt-manager: Fix CLI window launching options

Drop the thread event hack, use connection callbacks to accomplish
what we need more simply

src/virt-manager.py.in

@@ -211,55 +211,85 @@ def parse_commandline():
 
     return optParser.parse_args()
 
+
 def launch_specific_window(engine, show, uri, uuid):
-    if not engine.wait_for_open(uri):
+    logging.debug("Launching requested window '%s'" % show)
-        # Connection failed, don't attempt to continue
+    if show == 'creator':
+        engine.show_domain_creator(uri)
+    elif show == 'editor':
+        engine.show_domain_editor(uri, uuid)
+    elif show == 'performance':
+        engine.show_domain_performance(uri, uuid)
+    elif show == 'console':
+        engine.show_domain_console(uri, uuid)
+    elif show == 'summary':
+        engine.show_host_summary(uri)
+
+def _conn_state_changed(conn, engine, show, uri, uuid):
+    if conn.state == conn.STATE_DISCONNECTED:
+        return True
+    if conn.state != conn.STATE_ACTIVE:
         return
-    if show=='creator':
+
-        engine.show_create(uri)
+    launch_specific_window(engine, show, uri, uuid)
-    elif show=='editor':
+    return True
-        engine.show_details_config(uri, uuid)
-    elif show=='performance':
-        engine.show_details_performance(uri, uuid)
-    elif show=='console':
-        engine.show_console(uri, uuid)
 
 # maps --show-* to engine (ie local instance) methods
 def show_engine(engine, show, uri, uuid, no_conn_auto):
-    if show=='creator' or show=='editor' \
+    conn = None
-       or show=='performance' or show=='console':
+
-        # Create a thread so we can wait for connection to _fully_ start,
+    # Do this regardless
-        # to prevent races accessing uninit'd vars for ex. the create wizard
+    engine.show_manager()
-        # which expects the connection to be active.
+
-        thread = threading.Thread(target=launch_specific_window,
+    if uri:
-                                  args=(engine, show, uri, uuid),
+        conn = engine.add_connection(uri)
-                                  name="Launching '%s' window" % show)
+
-        thread.start()
+        if conn and show:
+            import virtManager.util as util
+            util.connect_opt_out(conn, "state-changed",
+                                 _conn_state_changed,
+                                 engine, show, uri, uuid)
 
-    elif show=='summary' or uri:
         engine.connect_to_uri(uri)
-    else:
-        engine.show_manager()
 
     if not no_conn_auto:
         engine.autostart_connections()
 
-
 # maps --show-* to remote manager (ie dbus call) methods
 def show_remote(managerObj, show, uri, uuid):
-    if show=='creator' or show=='editor' \
+    # Do this regardless
-       or show=='performance' or show=='console':
+    managerObj.show_manager()
-        # Create a thread so we can wait for connection to _fully_ start,
+
-        # to prevent races accessing uninit'd vars for ex. the create wizard
+    if show or uri or uuid:
-        # which expects the connection to be active.
+        launch_specific_window(managerObj, show, uri, uuid)
-        thread = threading.Thread(target=launch_specific_window,
+
-                                  args=(managerObj, show, uri, uuid),
+def dbus_config(engine):
-                                  name="Launching '%s' window" % show)
+    """
-        thread.start()
+    Setup dbus interface
-    elif show=='summary' or uri:
+    """
-        managerObj.show_host_summary(uri)
+    import dbus
+    from virtManager.remote import vmmRemote
+    bus = None
+
+    if os.getenv("DBUS_STARTER_ADDRESS") is None:
+        bus = dbus.SessionBus()
     else:
-        managerObj.show_manager()
+        bus = dbus.StarterBus()
+
+    dbusProxy = bus.get_object("org.freedesktop.DBus", "/org/freedesktop/DBus")
+    dbusObj = dbus.Interface(dbusProxy, "org.freedesktop.DBus")
+
+    if dbusObj.NameHasOwner("com.redhat.virt.manager"):
+        # We're already running, so just talk to existing process
+        managerProxy = bus.get_object("com.redhat.virt.manager",
+                                      "/com/redhat/virt/manager")
+        managerObj = dbus.Interface(managerProxy, "com.redhat.virt.manager")
+        return managerObj
+
+    else:
+        # Grab the service to allow others to talk to us later
+        name = dbus.service.BusName("com.redhat.virt.manager", bus=bus)
+        vmmRemote(engine, name)
 
 # Generic OptionParser callback for all --show-* options
 # This routine stores UUID to options.uuid for all --show-* options
@@ -352,12 +382,11 @@ def main():
         signal.signal(signal.SIGHUP, signal.SIG_IGN)
 
     from virtManager.engine import vmmEngine
-    from virtManager.remote import vmmRemote
 
     gtk.window_set_default_icon_from_file(icon_dir + "/" +
                                           appname + "-icon.svg")
 
-    if options.show and options.uri==None:
+    if options.show and options.uri == None:
         raise OptionValueError("can't use --show-* options without --connect")
 
     engine = vmmEngine(config)
@@ -367,42 +396,18 @@ def main():
              (os.getenv("DBUS_STARTER_ADDRESS") is None))):
 
         try:
-            bus = None
+            managerObj = dbus_config(engine)
-            if os.getenv("DBUS_STARTER_ADDRESS") is None:
+            if managerObj:
-                bus = dbus.SessionBus()
-            else:
-                bus = dbus.StarterBus()
-
-            dbusProxy = bus.get_object("org.freedesktop.DBus",
-                                       "/org/freedesktop/DBus")
-            dbusObj = dbus.Interface(dbusProxy, "org.freedesktop.DBus")
-
-            if dbusObj.NameHasOwner("com.redhat.virt.manager"):
-                # We're already running, so just talk to existing process
-
-                managerProxy = bus.get_object("com.redhat.virt.manager",
-                                              "/com/redhat/virt/manager")
-                managerObj = dbus.Interface(managerProxy,
-                                            "com.redhat.virt.manager")
-
-                show_remote(managerObj, options.show,
-                            options.uri, options.uuid)
-
                 # yes, we exit completely now - remote service is in charge
                 logging.debug("Connected to already running instance.")
+                show_remote(managerObj, options.show,
+                            options.uri, options.uuid)
                 return
-
-            else:
-                # Grab the service to allow others to talk to us later
-                name = dbus.service.BusName("com.redhat.virt.manager", bus=bus)
-                vmmRemote(engine, name)
-
         except:
             # Something went wrong doing dbus setup, just ignore & carry on
-            logging.warning("Could not get connection to session bus, "
+            logging.exception("Could not get connection to session bus, "
-                            "disabling DBus service " +
+                              "disabling DBus service")
-                            str(sys.exc_info()[0]) + " " +
+
-                            str(sys.exc_info()[1]))
 
     # At this point we're either starting a brand new controlling instance,
     # or the dbus comms to existing instance has failed

src/virtManager/connection.py

@@ -107,8 +107,6 @@ class vmmConnection(gobject.GObject):
         self.engine = engine
 
         self.connectThread = None
-        self.connectThreadEvent = threading.Event()
-        self.connectThreadEvent.set()
         self.connectError = None
         self.uri = uri
         if self.uri is None or self.uri.lower() == "xen":
@@ -846,7 +844,6 @@ class vmmConnection(gobject.GObject):
         self._change_state(self.STATE_CONNECTING)
 
         logging.debug("Scheduling background open thread for " + self.uri)
-        self.connectThreadEvent.clear()
         self.connectThread = threading.Thread(target = self._open_thread,
                                               name = "Connect %s" % self.uri)
         self.connectThread.setDaemon(True)
@@ -1034,27 +1031,24 @@ class vmmConnection(gobject.GObject):
     def _open_notify(self):
         logging.debug("Notifying open result")
 
-        try:
+        util.safe_idle_add(util.idle_emit, self, "state-changed")
-            util.safe_idle_add(util.idle_emit, self, "state-changed")
 
-            if self.state == self.STATE_ACTIVE:
+        if self.state == self.STATE_ACTIVE:
-                caps = self.get_capabilities_xml()
+            caps = self.get_capabilities_xml()
-                logging.debug("%s capabilities:\n%s" %
+            logging.debug("%s capabilities:\n%s" %
-                              (self.get_uri(), caps))
+                          (self.get_uri(), caps))
 
-                self.tick()
+            self.tick()
-                # If VMs disappeared since the last time we connected to
+            # If VMs disappeared since the last time we connected to
-                # this uri, remove their gconf entries so we don't pollute
+            # this uri, remove their gconf entries so we don't pollute
-                # the database
+            # the database
-                self.config.reconcile_vm_entries(self.get_uri(),
+            self.config.reconcile_vm_entries(self.get_uri(),
-                                                 self.vms.keys())
+                                             self.vms.keys())
 
-            if self.state == self.STATE_DISCONNECTED:
+        if self.state == self.STATE_DISCONNECTED:
-                util.safe_idle_add(util.idle_emit, self, "connect-error",
+            util.safe_idle_add(util.idle_emit, self, "connect-error",
-                                   self.connectError)
+                               self.connectError)
-                self.connectError = None
+            self.connectError = None
-        finally:
-            self.connectThreadEvent.set()
 
 
     #######################

src/virtManager/engine.py

@@ -491,15 +491,6 @@ class vmmEngine(gobject.GObject):
         logging.debug("Exiting app normally.")
         gtk.main_quit()
 
-    def wait_for_open(self, uri):
-        # Used to ensure connection fully starts before running
-        # ONLY CALL FROM WITHIN A THREAD
-        conn = self.connect_to_uri(uri)
-        conn.connectThreadEvent.wait()
-        if conn.state != conn.STATE_ACTIVE:
-            return False
-        return True
-
     def add_connection(self, uri, readOnly=None, autoconnect=False):
         conn = vmmConnection(self.get_config(), uri, readOnly, self)
         self.connections[uri] = {
@@ -768,30 +759,33 @@ class vmmEngine(gobject.GObject):
     def show_manager(self):
         self._do_show_manager(None)
 
-    def show_create(self, uri):
+    def show_connect(self):
-        win = self._do_show_create(self.get_manager(), uri)
+        self._do_show_connect(self.get_manager())
-        if not win:
-            return
-        win.show()
 
-    def show_console(self, uri, uuid):
+    def show_host_summary(self, uri):
+        self._do_show_host(self.get_manager(), uri)
+
+    def show_domain_creator(self, uri):
+        self._do_show_create(self.get_manager(), uri)
+
+    def show_domain_console(self, uri, uuid):
         win = self._do_show_details(self.get_manager(), uri, uuid)
         if not win:
             return
         win.activate_console_page()
 
-    def show_details_performance(self, uri, uuid):
+    def show_domain_editor(self, uri, uuid):
-        win = self._do_show_details(self.get_manager(), uri, uuid)
-        if not win:
-            return
-        win.activate_performance_page()
-
-    def show_details_config(self, uri, uuid):
         win = self._do_show_details(self.get_manager(), uri, uuid)
         if not win:
             return
         win.activate_config_page()
 
+    def show_domain_performance(self, uri, uuid):
+        win = self._do_show_details(self.get_manager(), uri, uuid)
+        if not win:
+            return
+        win.activate_performance_page()
+
     #######################################
     # Domain actions run/destroy/save ... #
     #######################################

src/virtManager/remote.py

@@ -28,23 +28,23 @@ class vmmRemote(dbus.service.Object):
 
     @dbus.service.method("com.redhat.virt.manager", in_signature="s")
     def show_domain_creator(self, uri):
-        self.engine.show_create(uri)
+        self.engine.show_domain_creator(str(uri))
 
     @dbus.service.method("com.redhat.virt.manager", in_signature="ss")
     def show_domain_editor(self, uri, uuid):
-        self.engine.show_details_config(uri, uuid)
+        self.engine.show_domain_editor(str(uri), str(uuid))
 
     @dbus.service.method("com.redhat.virt.manager", in_signature="ss")
     def show_domain_performance(self, uri, uuid):
-        self.engine.show_details_performance(uri, uuid)
+        self.engine.show_domain_performance(str(uri), str(uuid))
 
     @dbus.service.method("com.redhat.virt.manager", in_signature="ss")
     def show_domain_console(self, uri, uuid):
-        self.engine.show_console(uri, uuid)
+        self.engine.show_domain_console(str(uri), str(uuid))
 
     @dbus.service.method("com.redhat.virt.manager", in_signature="s")
     def show_host_summary(self, uri):
-        self.engine.show_host(uri)
+        self.engine.show_host_summary(str(uri))
 
     @dbus.service.method("com.redhat.virt.manager", in_signature="")
     def show_manager(self):