[PATCH 10/22] Always use iutil to start processes.

David Shea dshea at redhat.com
Wed Jun 3 16:03:34 UTC 2015


Replace direct uses of the subprocess module and os.system with our
iutil wrappers to ensure consistency in process starting. Use
execWithRedirect for things that just want to run a command to
completion (most things), execWithCapture for the realm command that
uses stdout, execConsole for a debug shell in the TUI, and startProgram
for anything that manages the Popen object itself (nm-connection-editor,
yelp) or does something complicated (vncconfig).

(cherry picked from commit c5d7cb605eb4c3370480f2f2f8af732e4f425079)

Related: rhbz#1188287
---
 anaconda                                | 30 +++++++++++-------------------
 pyanaconda/anaconda_log.py              |  4 +++-
 pyanaconda/ihelp.py                     |  4 ++--
 pyanaconda/kickstart.py                 | 32 +++++++++-----------------------
 pyanaconda/rescue.py                    |  4 +---
 pyanaconda/ui/gui/spokes/network.py     |  6 +++---
 pyanaconda/ui/tui/spokes/askvnc.py      |  5 +++--
 pyanaconda/ui/tui/spokes/shell_spoke.py |  6 ++----
 pyanaconda/vnc.py                       |  2 +-
 9 files changed, 35 insertions(+), 58 deletions(-)

diff --git a/anaconda b/anaconda
index e414d6f..09355eb 100755
--- a/anaconda
+++ b/anaconda
@@ -44,7 +44,7 @@ if ("debug=1" in proc_cmdline) or ("debug" in proc_cmdline):
     cov.start()
 
 
-import atexit, sys, os, time, subprocess, signal
+import atexit, sys, os, time, signal
 
 def exitHandler(rebootData, storage, exitCode=None):
     # Clear the list of watched PIDs.
@@ -87,7 +87,7 @@ def exitHandler(rebootData, storage, exitCode=None):
     if not flags.imageInstall and not flags.livecdInstall \
        and not flags.dirInstall:
         from pykickstart.constants import KS_SHUTDOWN, KS_WAIT
-        from pyanaconda.iutil import dracut_eject, get_mount_paths
+        from pyanaconda.iutil import dracut_eject, get_mount_paths, execWithRedirect
 
         if flags.eject or rebootData.eject:
             for cdrom in storage.devicetree.getDevicesByType("cdrom"):
@@ -95,11 +95,11 @@ def exitHandler(rebootData, storage, exitCode=None):
                     dracut_eject(cdrom.path)
 
         if rebootData.action == KS_SHUTDOWN:
-            subprocess.Popen(["systemctl", "--no-wall", "poweroff"])
+            execWithRedirect("systemctl", ["--no-wall", "poweroff"])
         elif rebootData.action == KS_WAIT:
-            subprocess.Popen(["systemctl", "--no-wall", "halt"])
+            execWithRedirect("systemctl", ["--no-wall", "halt"])
         else:  # reboot action is KS_REBOOT or None
-            subprocess.Popen(["systemctl", "--no-wall", "reboot"])
+            execWithRedirect("systemctl", ["--no-wall", "reboot"])
 
 def startSpiceVDAgent():
     status = iutil.execWithRedirect("spice-vdagent", [])
@@ -134,18 +134,6 @@ def doStartupX11Actions():
     childproc = iutil.startProgram(["metacity", "--display", ":1", "--sm-disable"])
     iutil.watchProcess(childproc, "metacity")
 
-def startAuditDaemon():
-    childpid = os.fork()
-    if not childpid:
-        cmd = '/sbin/auditd'
-        try:
-            os.execl(cmd, cmd)
-        except OSError as e:
-            log.error("Error running the audit daemon: %s", e)
-        os._exit(0)
-    # auditd will turn into a daemon so catch the immediate child pid now:
-    iutil.eintr_retry_call(os.waitpid, childpid, 0)
-
 def set_x_resolution(runres):
     if runres and opts.display_mode == 'g' and not flags.usevnc:
         try:
@@ -959,7 +947,11 @@ if __name__ == "__main__":
         sys.excepthook = _earlyExceptionHandler
 
     if can_touch_runtime_system("start audit daemon"):
-        startAuditDaemon()
+        # auditd will turn into a daemon and exit. Ignore startup errors
+        try:
+            iutil.execWithRedirect("/sbin/auditd", [])
+        except OSError:
+            pass
 
     # setup links required for all install types
     for i in ("services", "protocols", "nsswitch.conf", "joe", "selinux",
@@ -973,7 +965,7 @@ if __name__ == "__main__":
     log.info("anaconda called with cmdline = %s", sys.argv)
     log.info("Default encoding = %s ", sys.getdefaultencoding())
 
-    os.system("udevadm control --env=ANACONDA=1")
+    iutil.execWithRedirect("udevadm", ["control", "--env=ANACONDA=1"])
 
     # Collect all addon paths
     addon_paths = collect_addon_paths(constants.ADDON_PATHS)
diff --git a/pyanaconda/anaconda_log.py b/pyanaconda/anaconda_log.py
index 5c7b78b..937b12f 100644
--- a/pyanaconda/anaconda_log.py
+++ b/pyanaconda/anaconda_log.py
@@ -217,7 +217,9 @@ class AnacondaLog:
                 message, category, filename, lineno, line))
 
     def restartSyslog(self):
-        os.system("systemctl restart rsyslog.service")
+        # Import here instead of at the module level to avoid an import loop
+        from pyanaconda.iutil import execWithRedirect
+        execWithRedirect("systemctl", ["restart", "rsyslog.service"])
 
     def updateRemote(self, remote_syslog):
         """Updates the location of remote rsyslogd to forward to.
diff --git a/pyanaconda/ihelp.py b/pyanaconda/ihelp.py
index cb02a84..5326048 100644
--- a/pyanaconda/ihelp.py
+++ b/pyanaconda/ihelp.py
@@ -21,11 +21,11 @@
 Anaconda built-in help module
 """
 import os
-import subprocess
 
 from pyanaconda.flags import flags
 from pyanaconda.localization import find_best_locale_match
 from pyanaconda.constants import DEFAULT_LANG
+from pyanaconda.iutil import startProgram
 
 import logging
 log = logging.getLogger("anaconda")
@@ -124,7 +124,7 @@ def start_yelp(help_path):
     # under some extreme circumstances (placeholders missing)
     # the help path can be None and we need to prevent Popen
     # receiving None as an argument instead of a string
-    yelp_process = subprocess.Popen(["yelp", help_path or ""])
+    yelp_process = startProgram(["yelp", help_path or ""])
 
 def kill_yelp():
     """Try to kill any existing yelp processes"""
diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
index 8213fb0..1904101 100644
--- a/pyanaconda/kickstart.py
+++ b/pyanaconda/kickstart.py
@@ -41,7 +41,6 @@ from pyanaconda import iutil
 import os
 import os.path
 import tempfile
-import subprocess
 from pyanaconda.flags import flags, can_touch_runtime_system
 from pyanaconda.constants import ADDON_PATHS, IPMI_ABORTED
 import shlex
@@ -511,17 +510,13 @@ class Realm(commands.realm.F19_Realm):
             return
 
         try:
-            argv = ["realm", "discover", "--verbose"] + \
+            argv = ["discover", "--verbose"] + \
                     self.discover_options + [self.join_realm]
-            proc = subprocess.Popen(argv, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-            output, stderr = proc.communicate()
-            # might contain useful information for users who use
-            # use the realm kickstart command
-            log.info("Realm discover stderr:\n%s", stderr)
-        except OSError as msg:
+            output = iutil.execWithCapture("realm", argv, filter_stderr=True)
+        except OSError:
             # TODO: A lousy way of propagating what will usually be
             # 'no such realm'
-            log.error("Error running realm %s: %s", argv, msg)
+            # The error message is logged by iutil
             return
 
         # Now parse the output for the required software. First line is the
@@ -557,21 +552,12 @@ class Realm(commands.realm.F19_Realm):
                pw_args + self.join_args
         rc = -1
         try:
-            proc = subprocess.Popen(argv, stdout=subprocess.PIPE,
-                                    stderr=subprocess.PIPE)
-            stderr = proc.communicate()[1]
-            # might contain useful information for users who use
-            # use the realm kickstart command
-            log.info("Realm join stderr:\n%s", stderr)
-            rc = proc.returncode
-        except OSError as msg:
-            log.error("Error running %s: %s", argv, msg)
+            rc = iutil.execWithRedirect("realm", argv)[0]
+        except OSError:
+            pass
 
-        if rc != 0:
-            log.error("Command failure: %s: %d", argv, rc)
-            return
-
-        log.info("Joined realm %s", self.join_realm)
+        if rc == 0:
+            log.info("Joined realm %s", self.join_realm)
 
 
 class ClearPart(commands.clearpart.F21_ClearPart):
diff --git a/pyanaconda/rescue.py b/pyanaconda/rescue.py
index 63bc0c1..a07a4ab 100644
--- a/pyanaconda/rescue.py
+++ b/pyanaconda/rescue.py
@@ -25,7 +25,6 @@ from pyanaconda import iutil
 import shutil
 import time
 import re
-import subprocess
 
 from snack import ButtonChoiceWindow, ListboxChoiceWindow,SnackScreen
 
@@ -190,8 +189,7 @@ def runShell(screen = None, msg=""):
     proc = None
 
     if os.path.exists("/usr/bin/firstaidkit-qs"):
-        proc = subprocess.Popen(["/usr/bin/firstaidkit-qs"])
-        proc.wait()
+        iutil.execWithRedirect("/usr/bin/firstaidkit-qs", [])
 
     if proc is None or proc.returncode!=0:
         if os.path.exists("/bin/bash"):
diff --git a/pyanaconda/ui/gui/spokes/network.py b/pyanaconda/ui/gui/spokes/network.py
index 18ee01e..6c51e23 100644
--- a/pyanaconda/ui/gui/spokes/network.py
+++ b/pyanaconda/ui/gui/spokes/network.py
@@ -41,6 +41,7 @@ from pyanaconda.ui.categories.system import SystemCategory
 from pyanaconda.ui.gui.hubs.summary import SummaryHub
 from pyanaconda.ui.gui.utils import gtk_call_once, escape_markup
 from pyanaconda.ui.common import FirstbootSpokeMixIn
+from pyanaconda.iutil import startProgram
 
 from pyanaconda import network
 from pyanaconda import nm
@@ -48,7 +49,6 @@ from pyanaconda import nm
 from gi.repository import GLib, GObject, Pango, Gio, NetworkManager, NMClient
 import dbus
 import dbus.service
-import subprocess
 import string
 from uuid import uuid4
 from dbus.mainloop.glib import DBusGMainLoop
@@ -558,7 +558,7 @@ class NetworkControlBox(GObject.GObject):
         log.info("network: configuring connection %s device %s ssid %s",
                  uuid, devname, self.selected_ssid)
         self.kill_nmce(msg="Configure button clicked")
-        proc = subprocess.Popen(["nm-connection-editor", "--edit", "%s" % uuid])
+        proc = startProgram(["nm-connection-editor", "--edit", "%s" % uuid])
         self._running_nmce = proc
 
         GLib.child_watch_add(proc.pid, self.on_nmce_exited, activate)
@@ -648,7 +648,7 @@ class NetworkControlBox(GObject.GObject):
     def add_device(self, ty):
         log.info("network: adding device of type %s", ty)
         self.kill_nmce(msg="Add device button clicked")
-        proc = subprocess.Popen(["nm-connection-editor", "--create", "--type=%s" % ty])
+        proc = startProgram(["nm-connection-editor", "--create", "--type=%s" % ty])
         self._running_nmce = proc
 
         GLib.child_watch_add(proc.pid, self.on_nmce_exited)
diff --git a/pyanaconda/ui/tui/spokes/askvnc.py b/pyanaconda/ui/tui/spokes/askvnc.py
index 5d815e2..f9e087e 100644
--- a/pyanaconda/ui/tui/spokes/askvnc.py
+++ b/pyanaconda/ui/tui/spokes/askvnc.py
@@ -27,7 +27,8 @@ from pyanaconda.constants_text import INPUT_PROCESSED
 from pyanaconda.i18n import N_, _
 from pyanaconda.ui.communication import hubQ
 from pyanaconda.ui.tui import exception_msg_handler
-import getpass, subprocess
+from pyanaconda.iutil import execWithRedirect
+import getpass
 import sys
 
 def exception_msg_handler_and_exit(event, data):
@@ -104,7 +105,7 @@ class AskVNCSpoke(NormalTUISpoke):
             d = YesNoDialog(self.app, _(self.app.quit_message))
             self.app.switch_screen_modal(d)
             if d.answer:
-                subprocess.Popen(["systemctl", "--no-wall", "reboot"])
+                execWithRedirect("systemctl", ["--no-wall", "reboot"])
         else:
             return key
 
diff --git a/pyanaconda/ui/tui/spokes/shell_spoke.py b/pyanaconda/ui/tui/spokes/shell_spoke.py
index e0115ca..0037188 100644
--- a/pyanaconda/ui/tui/spokes/shell_spoke.py
+++ b/pyanaconda/ui/tui/spokes/shell_spoke.py
@@ -26,10 +26,9 @@ from pyanaconda.ui.tui.simpleline.widgets import TextWidget
 from pyanaconda.i18n import N_, _
 from pyanaconda.constants import ANACONDA_ENVIRON
 from pyanaconda.flags import flags
+from pyanaconda.iutil import execConsole
 from blivet import arch
 
-import subprocess
-
 class ShellSpoke(NormalTUISpoke):
     title = N_("Shell")
     category = SystemCategory
@@ -59,8 +58,7 @@ class ShellSpoke(NormalTUISpoke):
 
     def prompt(self, args=None):
         # run shell instead of printing prompt and close window on shell exit
-        proc = subprocess.Popen(["bash", "--login"], shell=True, cwd="/")
-        proc.wait()
+        execConsole()
         self.close()
 
         # suppress the prompt
diff --git a/pyanaconda/vnc.py b/pyanaconda/vnc.py
index 70800ad..4281ae6 100644
--- a/pyanaconda/vnc.py
+++ b/pyanaconda/vnc.py
@@ -160,7 +160,7 @@ class VncServer:
         vncconfigcommand = [self.root+"/usr/bin/vncconfig", "-display", ":%s"%self.display, "-connect", hostarg]
 
         for _i in range(maxTries):
-            vncconfp = subprocess.Popen(vncconfigcommand, stdout=subprocess.PIPE, stderr=subprocess.PIPE) # vncconfig process
+            vncconfp = iutil.startProgram(vncconfigcommand, stdout=subprocess.PIPE, stderr=subprocess.PIPE) # vncconfig process
             err = vncconfp.communicate()[1]
 
             if err == '':
-- 
2.1.0



More information about the anaconda-patches mailing list