[virt-tools-list] [virt-manager PATCH 2/2] cli: stop forking into the background

[Daniel P. Berrangé]

The behaviour whereby virt-manager forks into the background was added
way back in:

  commit 99c92b9471a6a55859307071aa4a0e712991f158
  Author: Daniel P. Berrange <berrange at redhat.com>
  Date:   Mon Sep 10 20:10:20 2007 -0400

    Refactor startup to drop controlling TTY, avoiding annoying SSH prompts

While it achieves its stated goal, this is quite a big hammer to use
with unpleasant side effects. Most end users will launch virt-manager
from the desktop which will fork the app into the background already.
Even when running from the command line, modern desktop environments
will have things setup up so that all SSH prompts are intercepted and
presented via a graphical window. Forking into the background causes
extra pain for developers as warnings that would otherwise appear on
stderr get lost e.g.

  commit 24a8b66b35c92bed919a4a6beb7c7fb80e85b3b2
  Author: Daniel P. Berrangé <berrange at redhat.com>
  Date:   Wed Apr 4 14:35:40 2018 +0100

    avoid referencing ConnectError if it is None

    Currently it throws an exception at startup which is hidden unless you
    run with --no-fork

The limited benefit of forking is not worth the pain it causes, so
just start "normally" as any other GTK app would.

Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
---
 man/virt-manager.pod   |  7 +------
 tests/uitests/utils.py |  2 +-
 virt-manager           | 46 +---------------------------------------------
 3 files changed, 3 insertions(+), 52 deletions(-)

diff --git a/man/virt-manager.pod b/man/virt-manager.pod
index 1bbfb9ae..54172339 100644
--- a/man/virt-manager.pod
+++ b/man/virt-manager.pod
@@ -41,12 +41,7 @@ Specify the hypervisor connection C<URI>
 =item B<--debug>
 
 List debugging output to the console (normally this is only logged in
-~/.cache/virt-manager/virt-manager.log). This function implies --no-fork.
-
-=item B<--no-fork>
-
-Don't fork C<virt-manager> off into the background: run it blocking the
-current terminal. Useful for seeing possible errors dumped to stdout/stderr.
+~/.cache/virt-manager/virt-manager.log)
 
 =item B<--show-DIALOG-WINDOW>
 
diff --git a/tests/uitests/utils.py b/tests/uitests/utils.py
index 4af451b3..cd5c4fa8 100644
--- a/tests/uitests/utils.py
+++ b/tests/uitests/utils.py
@@ -360,7 +360,7 @@ class VMMDogtailApp(object):
             cmd += ["-m", "coverage", "run", "--append",
                     "--omit", "/usr/*"]
         cmd += [os.path.join(os.getcwd(), "virt-manager"),
-                "--test-first-run", "--no-fork", "--connect", self.uri]
+                "--test-first-run", "--connect", self.uri]
         cmd += extra_opts
 
         self._proc = subprocess.Popen(cmd, stdout=stdout, stderr=stderr)
diff --git a/virt-manager b/virt-manager
index bc26be5c..451bbc30 100755
--- a/virt-manager
+++ b/virt-manager
@@ -49,12 +49,6 @@ def _show_startup_error(msg, details):
 
 
 def _import_gtk(leftovers):
-    # The never ending fork+gsettings problems now require us to
-    # import Gtk _after_ the fork. This creates a funny race, since we
-    # need to parse the command line arguments to know if we need to
-    # fork, but need to import Gtk before cli processing so it can
-    # handle --g-fatal-args. We strip out our flags first and pass the
-    # left overs to gtk
     origargv = sys.argv
     try:
         sys.argv = origargv[:1] + leftovers[:]
@@ -90,29 +84,6 @@ def _import_gtk(leftovers):
     return leftovers
 
 
-def drop_tty():
-    # We fork and setsid so that we drop the controlling
-    # tty. This prevents libvirt's SSH tunnels from prompting
-    # for user input if SSH keys/agent aren't configured.
-    if os.fork() != 0:
-        os._exit(0)  # pylint: disable=protected-access
-
-    os.setsid()
-
-
-def drop_stdio():
-    # This is part of the fork process described in drop_tty()
-    for fd in range(0, 2):
-        try:
-            os.close(fd)
-        except OSError:
-            pass
-
-    os.open(os.devnull, os.O_RDWR)
-    os.dup2(0, 1)
-    os.dup2(0, 2)
-
-
 def parse_commandline():
     epilog = ("Also accepts standard GTK arguments like --g-fatal-warnings")
     parser = argparse.ArgumentParser(usage="virt-manager [options]",
@@ -148,10 +119,8 @@ def parse_commandline():
     parser.add_argument("-c", "--connect", dest="uri",
         help="Connect to hypervisor at URI", metavar="URI")
     parser.add_argument("--debug", action="store_true",
-        help="Print debug output to stdout (implies --no-fork)",
+        help="Print debug output to stdout",
         default=False)
-    parser.add_argument("--no-fork", action="store_true",
-        help="Don't fork into background on startup")
 
     parser.add_argument("--show-domain-creator", action="store_true",
         help="Show 'New VM' wizard")
@@ -187,22 +156,9 @@ def main():
     if options.test_first_run:
         os.environ["GSETTINGS_BACKEND"] = "memory"
 
-    # Now we've got basic environment up & running we can fork
-    do_drop_stdio = False
-    if not options.no_fork and not options.debug:
-        drop_tty()
-        do_drop_stdio = True
-
-        # Ignore SIGHUP, otherwise a serial console closing drops the whole app
-        signal.signal(signal.SIGHUP, signal.SIG_IGN)
-
     leftovers = _import_gtk(leftovers)
     Gtk = globals()["Gtk"]
 
-    # Do this after the Gtk import so the user has a chance of seeing any error
-    if do_drop_stdio:
-        drop_stdio()
-
     if leftovers:
         raise RuntimeError("Unhandled command line options '%s'" % leftovers)
 
-- 
2.14.3