From 0b851a3f25d6e2ac7e6d06e342d0823c206ee25a Mon Sep 17 00:00:00 2001
From: Vladimir Slavik <vslavik@redhat.com>
Date: Wed, 21 Apr 2021 20:00:54 +0200
Subject: [PATCH] Another attempt at the X thing. This gives up the exception
 handler test temporarily, and solves almost everything.

The main problem of other solutions is that once X starts,
it steals the screen by going to tty6. If the exception handler
test-invoking handler is not returned back immediately,
an "after-timeout" handler can be installed instead, which switches back to tty1.

With that in place, it's also safe to terminate Xorg once
it's clear it's not coming in time. The termination will happen later,
but that does not matter any more.

Finally, with the termination happening,
it is also safe to return the crash report text handler.

Resolves: rhbz#1918702

Previous work: #3107, #3132, #3141, #3295. Thanks to @bitcoffeeiux and @poncovka.

The one avenue left unexplored is using the -displayfd option.
---
 pyanaconda/core/util.py | 62 ++++++++++++++++++++++++++++++++---------
 pyanaconda/display.py   | 29 +++++++++++++++++--
 2 files changed, 75 insertions(+), 16 deletions(-)

diff --git a/pyanaconda/core/util.py b/pyanaconda/core/util.py
index b7a1731..3013cd8 100644
--- a/pyanaconda/core/util.py
+++ b/pyanaconda/core/util.py
@@ -47,7 +47,7 @@ from pyanaconda.core.constants import DRACUT_SHUTDOWN_EJECT, TRANSLATIONS_UPDATE
     IPMI_ABORTED, X_TIMEOUT, TAINT_HARDWARE_UNSUPPORTED, TAINT_SUPPORT_REMOVED, \
     WARNING_HARDWARE_UNSUPPORTED, WARNING_SUPPORT_REMOVED
 from pyanaconda.core.constants import SCREENSHOTS_DIRECTORY, SCREENSHOTS_TARGET_DIRECTORY
-from pyanaconda.errors import RemovedModuleError, ExitError
+from pyanaconda.errors import RemovedModuleError
 
 from pyanaconda.anaconda_logging import program_log_lock
 from pyanaconda.anaconda_loggers import get_module_logger, get_program_logger
@@ -204,6 +204,19 @@ def startProgram(argv, root='/', stdin=None, stdout=subprocess.PIPE, stderr=subp
                             preexec_fn=preexec, cwd=root, env=env, **kwargs)
 
 
+class X11Status:
+    """Status of Xorg launch.
+
+    Values of an instance can be modified from the handler functions.
+    """
+    def __init__(self):
+        self.started = False
+        self.timed_out = False
+
+    def needs_waiting(self):
+        return not (self.started or self.timed_out)
+
+
 def startX(argv, output_redirect=None, timeout=X_TIMEOUT):
     """ Start X and return once X is ready to accept connections.
 
@@ -217,28 +230,36 @@ def startX(argv, output_redirect=None, timeout=X_TIMEOUT):
         :param output_redirect: file or file descriptor to redirect stdout and stderr to
         :param timeout: Number of seconds to timing out.
     """
-    # Use a list so the value can be modified from the handler function
-    x11_started = [False]
+    x11_status = X11Status()
 
-    def sigusr1_handler(num, frame):
+    # Handle successful start before timeout
+    def sigusr1_success_handler(num, frame):
         log.debug("X server has signalled a successful start.")
-        x11_started[0] = True
+        x11_status.started = True
 
     # Fail after, let's say a minute, in case something weird happens
     # and we don't receive SIGUSR1
     def sigalrm_handler(num, frame):
         # Check that it didn't make it under the wire
-        if x11_started[0]:
+        if x11_status.started:
             return
+        x11_status.timed_out = True
         log.error("Timeout trying to start %s", argv[0])
-        raise ExitError("Timeout trying to start %s" % argv[0])
 
-    # preexec_fn to add the SIGUSR1 handler in the child
+    # Handle delayed start after timeout
+    def sigusr1_too_late_handler(num, frame):
+        if x11_status.timed_out:
+            log.debug("SIGUSR1 received after X server timeout. Switching back to tty1. "
+                      "SIGUSR1 now again initiates test of exception reporting.")
+            signal.signal(signal.SIGUSR1, old_sigusr1_handler)
+
+    # preexec_fn to add the SIGUSR1 handler in the child we are starting
+    # see man page XServer(1), section "signals"
     def sigusr1_preexec():
         signal.signal(signal.SIGUSR1, signal.SIG_IGN)
 
     try:
-        old_sigusr1_handler = signal.signal(signal.SIGUSR1, sigusr1_handler)
+        old_sigusr1_handler = signal.signal(signal.SIGUSR1, sigusr1_success_handler)
         old_sigalrm_handler = signal.signal(signal.SIGALRM, sigalrm_handler)
 
         # Start the timer
@@ -249,16 +270,31 @@ def startX(argv, output_redirect=None, timeout=X_TIMEOUT):
                                  preexec_fn=sigusr1_preexec)
         WatchProcesses.watch_process(childproc, argv[0])
 
-        # Wait for SIGUSR1
-        while not x11_started[0]:
+        # Wait for SIGUSR1 or SIGALRM
+        while x11_status.needs_waiting():
             signal.pause()
 
     finally:
-        # Put everything back where it was
+        # Stop the timer
         signal.alarm(0)
-        signal.signal(signal.SIGUSR1, old_sigusr1_handler)
         signal.signal(signal.SIGALRM, old_sigalrm_handler)
 
+        # Handle outcome of X start attempt
+        if x11_status.started:
+            signal.signal(signal.SIGUSR1, old_sigusr1_handler)
+        elif x11_status.timed_out:
+            signal.signal(signal.SIGUSR1, sigusr1_too_late_handler)
+            # Kill Xorg because from now on we will not use it. It will exit only after sending
+            # the signal, but at least we don't have to track that.
+            WatchProcesses.unwatch_process(childproc)
+            childproc.terminate()
+            log.debug("Exception handler test suspended to prevent accidental activation by "
+                      "delayed Xorg start. Next SIGUSR1 will be handled as delayed Xorg start.")
+            # Raise an exception to notify the caller that things went wrong. This affects
+            # particularly pyanaconda.display.do_startup_x11_actions(), where the window manager
+            # is started immediately after this. The WM would just wait forever.
+            raise TimeoutError("Timeout trying to start %s" % argv[0])
+
 
 def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None, log_output=True,
                  binary_output=False, filter_stderr=False):
diff --git a/pyanaconda/display.py b/pyanaconda/display.py
index 8379d9c..b577eb8 100644
--- a/pyanaconda/display.py
+++ b/pyanaconda/display.py
@@ -22,6 +22,7 @@
 import os
 import subprocess
 import time
+import textwrap
 import pkgutil
 
 from pyanaconda.core.configuration.anaconda import conf
@@ -49,6 +50,14 @@ from pyanaconda.anaconda_loggers import get_module_logger, get_stdout_logger
 log = get_module_logger(__name__)
 stdout_log = get_stdout_logger()
 
+X_TIMEOUT_ADVICE = \
+    "Do not load the stage2 image over a slow network link.\n" \
+    "Wait longer for the X server startup with the inst.xtimeout=<SECONDS> boot option." \
+    "The default is 60 seconds.\n" \
+    "Load the stage2 image into memory with the rd.live.ram boot option to decrease access " \
+    "time.\n" \
+    "Enforce text mode when installing from remote media with the inst.text boot option."
+#  on RHEL also: "Use the customer portal download URL in ilo/drac devices for greater speed."
 
 # Spice
 
@@ -78,7 +87,7 @@ def ask_vnc_question(anaconda, vnc_server, message):
     App.initialize()
     loop = App.get_event_loop()
     loop.set_quit_callback(tui_quit_callback)
-    spoke = AskVNCSpoke(anaconda.ksdata, message)
+    spoke = AskVNCSpoke(anaconda.ksdata, message=message)
     ScreenHandler.schedule_screen(spoke)
     App.run()
 
@@ -314,9 +323,23 @@ def setup_display(anaconda, options):
         try:
             start_x11(xtimeout)
             do_startup_x11_actions()
-        except (OSError, RuntimeError) as e:
+        except TimeoutError as e:
             log.warning("X startup failed: %s", e)
-            stdout_log.warning("X startup failed, falling back to text mode")
+            print("\nX did not start in the expected time, falling back to text mode. There are "
+                  "multiple ways to avoid this issue:")
+            wrapper = textwrap.TextWrapper(initial_indent=" * ", subsequent_indent="   ",
+                                           width=os.get_terminal_size().columns - 3)
+            for line in X_TIMEOUT_ADVICE.split("\n"):
+                print(wrapper.fill(line))
+            util.vtActivate(1)
+            anaconda.display_mode = constants.DisplayModes.TUI
+            anaconda.gui_startup_failed = True
+            time.sleep(2)
+
+        except (OSError, RuntimeError) as e:
+            log.warning("X or window manager startup failed: %s", e)
+            print("\nX or window manager startup failed, falling back to text mode.")
+            util.vtActivate(1)
             anaconda.display_mode = constants.DisplayModes.TUI
             anaconda.gui_startup_failed = True
             time.sleep(2)
-- 
2.23.0