[PATCH 17/17] Always use iutil to start processes.

David Shea dshea at redhat.com
Sun Sep 21 19:37:09 UTC 2014


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 shel in the TUI, and startProgram
for anything that manages the Popen object itself (nm-connection-editor,
yelp) or does something complicated (vncconfig).
---
 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 c2a17f2..5aaaa3e 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):
     # Clear the list of watched PIDs.
@@ -84,7 +84,7 @@ def exitHandler(rebootData, storage):
     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"):
@@ -92,11 +92,11 @@ def exitHandler(rebootData, storage):
                     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", [])
@@ -131,18 +131,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:
-    os.waitpid(childpid, 0)
-
 def set_x_resolution(runres):
     if runres and opts.display_mode == 'g' and not flags.usevnc:
         try:
@@ -854,7 +842,11 @@ def 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",
@@ -868,7 +860,7 @@ def 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
     from pyanaconda.addons import collect_addon_paths
diff --git a/pyanaconda/anaconda_log.py b/pyanaconda/anaconda_log.py
index 56ae9c7..498d64a 100644
--- a/pyanaconda/anaconda_log.py
+++ b/pyanaconda/anaconda_log.py
@@ -204,7 +204,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 c0cb61c..a1c691b 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
@@ -485,17 +484,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
@@ -531,21 +526,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 50b613e..271429c 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
 
@@ -545,7 +545,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)
@@ -632,7 +632,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_adding_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 7eedbf8..a7e2072 100644
--- a/pyanaconda/vnc.py
+++ b/pyanaconda/vnc.py
@@ -158,7 +158,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 == '':
-- 
1.9.3



More information about the anaconda-patches mailing list