[PATCH 2/4] clean up command execution

Brian C. Lane bcl at redhat.com
Fri Jul 27 20:50:42 UTC 2012


From: "Brian C. Lane" <bcl at redhat.com>

Switch to using execWith* so that the command and its output can be
logged. To capture the output setup a logger named "program"

livemedia-creator captures all of this into program.log
---
 src/pylorax/__init__.py    |    7 +++--
 src/pylorax/executils.py   |   26 +++++++++++++++++++---
 src/pylorax/imgutils.py    |   49 ++++++++++++++++++-------------------------
 src/pylorax/ltmpl.py       |   18 +++++++++-------
 src/pylorax/sysutils.py    |    4 ++-
 src/pylorax/treebuilder.py |   17 ++++++++-------
 src/sbin/livemedia-creator |   14 ++++--------
 7 files changed, 74 insertions(+), 61 deletions(-)

diff --git a/src/pylorax/__init__.py b/src/pylorax/__init__.py
index cf48f0e..5c1d2a9 100644
--- a/src/pylorax/__init__.py
+++ b/src/pylorax/__init__.py
@@ -30,7 +30,7 @@ import os
 import ConfigParser
 import tempfile
 import locale
-import subprocess
+from subprocess import CalledProcessError
 import selinux
 
 from base import BaseLoraxClass, DataHolder
@@ -47,6 +47,7 @@ from treebuilder import RuntimeBuilder, TreeBuilder
 from buildstamp import BuildStamp
 from treeinfo import TreeInfo
 from discinfo import DiscInfo
+from executils import execWithRedirect
 
 class ArchData(DataHolder):
     lib64_arches = ("x86_64", "ppc64", "sparc64", "s390x", "ia64")
@@ -140,8 +141,8 @@ class Lorax(BaseLoraxClass):
 
         if domacboot:
             try:
-                subprocess.check_call(["rpm", "-q", "hfsplus-tools"])
-            except subprocess.CalledProcessError:
+                execWithRedirect("rpm", ["-q", "hfsplus-tools"])
+            except CalledProcessError:
                 logger.critical("you need to install hfsplus-tools to create mac images")
                 sys.exit(1)
 
diff --git a/src/pylorax/executils.py b/src/pylorax/executils.py
index f1be30c..2845509 100644
--- a/src/pylorax/executils.py
+++ b/src/pylorax/executils.py
@@ -70,9 +70,11 @@ class tee(threading.Thread):
 # @param stdout The file descriptor to redirect stdout to.
 # @param stderr The file descriptor to redirect stderr to.
 # @param root The directory to chroot to before running command.
+# @param preexec_fn function to pass to Popen
+# @param cwd working directory to pass to Popen
 # @return The return code of command.
 def execWithRedirect(command, argv, stdin = None, stdout = None,
-                     stderr = None, root = '/'):
+                     stderr = None, root = None, preexec_fn=None, cwd=None):
     def chroot ():
         os.chroot(root)
 
@@ -115,6 +117,13 @@ def execWithRedirect(command, argv, stdin = None, stdout = None,
     env = os.environ.copy()
     env.update({"LC_ALL": "C"})
 
+    if root:
+        preexec_fn = chroot
+        cwd = root
+        program_log.info("chrooting into %s" % (cwd,))
+    elif cwd:
+        program_log.info("chdiring into %s" % (cwd,))
+
     try:
         #prepare tee proceses
         proc_std = tee(pstdout, stdout, program_log.info, command)
@@ -127,7 +136,7 @@ def execWithRedirect(command, argv, stdin = None, stdout = None,
         proc = subprocess.Popen([command] + argv, stdin=stdin,
                                 stdout=pstdin,
                                 stderr=perrin,
-                                preexec_fn=chroot, cwd=root,
+                                preexec_fn=preexec_fn, cwd=cwd,
                                 env=env)
 
         proc.wait()
@@ -170,8 +179,10 @@ def execWithRedirect(command, argv, stdin = None, stdout = None,
 # @param stdin The file descriptor to read stdin from.
 # @param stderr The file descriptor to redirect stderr to.
 # @param root The directory to chroot to before running command.
+# @param preexec_fn function to pass to Popen
+# @param cwd working directory to pass to Popen
 # @return The output of command from stdout.
-def execWithCapture(command, argv, stdin = None, stderr = None, root='/'):
+def execWithCapture(command, argv, stdin = None, stderr = None, root=None, preexec_fn=None, cwd=None):
     def chroot():
         os.chroot(root)
 
@@ -207,11 +218,18 @@ def execWithCapture(command, argv, stdin = None, stderr = None, root='/'):
     env = os.environ.copy()
     env.update({"LC_ALL": "C"})
 
+    if root:
+        preexec_fn = chroot
+        cwd = root
+        program_log.info("chrooting into %s" % (cwd,))
+    elif cwd:
+        program_log.info("chdiring into %s" % (cwd,))
+
     try:
         proc = subprocess.Popen([command] + argv, stdin=stdin,
                                 stdout=subprocess.PIPE,
                                 stderr=subprocess.PIPE,
-                                preexec_fn=chroot, cwd=root,
+                                preexec_fn=preexec_fn, cwd=cwd,
                                 env=env)
 
         while True:
diff --git a/src/pylorax/imgutils.py b/src/pylorax/imgutils.py
index 5ec6e8a..8da4b8c 100644
--- a/src/pylorax/imgutils.py
+++ b/src/pylorax/imgutils.py
@@ -22,12 +22,14 @@ logger = logging.getLogger("pylorax.imgutils")
 
 import os, tempfile
 from os.path import join, dirname
-from pylorax.sysutils import cpfile
-from subprocess import *
+from subprocess import CalledProcessError
 import sys
 import traceback
 from time import sleep
 
+from pylorax.sysutils import cpfile
+from pylorax.executils import execWithRedirect, execWithCapture
+
 ######## Functions for making container images (cpio, squashfs) ##########
 
 def mkcpio(rootdir, outfile, compression="xz", compressargs=["-9"]):
@@ -36,7 +38,6 @@ def mkcpio(rootdir, outfile, compression="xz", compressargs=["-9"]):
     compressargs will be used on the compression commandline.'''
     if compression not in (None, "xz", "gzip", "lzma"):
         raise ValueError, "Unknown compression type %s" % compression
-    chdir = lambda: os.chdir(rootdir)
     if compression == "xz":
         compressargs.insert(0, "--check=crc32")
     if compression is None:
@@ -44,9 +45,9 @@ def mkcpio(rootdir, outfile, compression="xz", compressargs=["-9"]):
         compressargs = []
     logger.debug("mkcpio %s | %s %s > %s", rootdir, compression,
                                         " ".join(compressargs), outfile)
-    find = Popen(["find", ".", "-print0"], stdout=PIPE, preexec_fn=chdir)
+    find = Popen(["find", ".", "-print0"], stdout=PIPE, cwd=rootdir)
     cpio = Popen(["cpio", "--null", "--quiet", "-H", "newc", "-o"],
-                 stdin=find.stdout, stdout=PIPE, preexec_fn=chdir)
+                 stdin=find.stdout, stdout=PIPE, cwd=rootdir)
     comp = Popen([compression] + compressargs,
                  stdin=cpio.stdout, stdout=open(outfile, "wb"))
     comp.wait()
@@ -56,9 +57,7 @@ def mksquashfs(rootdir, outfile, compression="default", compressargs=[]):
     '''Make a squashfs image containing the given rootdir.'''
     if compression != "default":
         compressargs = ["-comp", compression] + compressargs
-    cmd = ["mksquashfs", rootdir, outfile] + compressargs
-    logger.debug(" ".join(cmd))
-    return call(cmd)
+    return execWithRedirect("mksquashfs", [rootdir, outfile] + compressargs)
 
 ######## Utility functions ###############################################
 
@@ -70,17 +69,17 @@ def mksparse(outfile, size):
 def loop_attach(outfile):
     '''Attach a loop device to the given file. Return the loop device name.
     Raises CalledProcessError if losetup fails.'''
-    dev = check_output(["losetup", "--find", "--show", outfile], stderr=PIPE)
+    dev = execWithCapture("losetup", ["--find", "--show", outfile])
     return dev.strip()
 
 def loop_detach(loopdev):
     '''Detach the given loop device. Return False on failure.'''
-    return (call(["losetup", "--detach", loopdev]) == 0)
+    return (execWithRedirect("losetup", ["--detach", loopdev]) == 0)
 
 def get_loop_name(path):
     '''Return the loop device associated with the path.
     Raises RuntimeError if more than one loop is associated'''
-    buf = check_output(["losetup", "-j", path], stderr=PIPE)
+    buf = execWithCapture("losetup", ["-j", path])
     if len(buf.splitlines()) > 1:
         # there should never be more than one loop device listed
         raise RuntimeError("multiple loops associated with %s" % path)
@@ -93,15 +92,14 @@ def dm_attach(dev, size, name=None):
     raises CalledProcessError if dmsetup fails.'''
     if name is None:
         name = tempfile.mktemp(prefix="lorax.imgutils.", dir="")
-    check_call(["dmsetup", "create", name, "--table",
-                "0 %i linear %s 0" % (size/512, dev)],
-                stdout=PIPE, stderr=PIPE)
+    execWithRedirect("dmsetup", ["create", name, "--table",
+                                 "0 %i linear %s 0" % (size/512, dev)])
     return name
 
 def dm_detach(dev):
     '''Detach the named devicemapper device. Returns False if dmsetup fails.'''
     dev = dev.replace("/dev/mapper/", "") # strip prefix, if it's there
-    return call(["dmsetup", "remove", dev], stdout=PIPE, stderr=PIPE)
+    return execWithRedirect("dmsetup", ["remove", dev])
 
 def mount(dev, opts="", mnt=None):
     '''Mount the given device at the given mountpoint, using the given opts.
@@ -116,8 +114,7 @@ def mount(dev, opts="", mnt=None):
     if opts:
         mount += ["-o", opts]
     mount += [dev, mnt]
-    logger.debug(" ".join(mount))
-    check_call(mount)
+    execWithRedirect(mount[0], mount[1:])
     return mnt
 
 def umount(mnt,  lazy=False, maxretry=3, retrysleep=1.0):
@@ -127,11 +124,10 @@ def umount(mnt,  lazy=False, maxretry=3, retrysleep=1.0):
     umount = ["umount"]
     if lazy: umount += ["-l"]
     umount += [mnt]
-    logger.debug(" ".join(umount))
     count = 0
     while maxretry > 0:
         try:
-            rv = check_call(umount)
+            rv = execWithRedirect(umount[0], umount[1:])
         except CalledProcessError:
             count += 1
             if count == maxretry:
@@ -139,8 +135,7 @@ def umount(mnt,  lazy=False, maxretry=3, retrysleep=1.0):
             logger.warn("failed to unmount %s. retrying (%d/%d)...",
                          mnt, count, maxretry)
             if logger.getEffectiveLevel() <= logging.DEBUG:
-                fuser = check_output(["fuser", "-vm", mnt],
-                                     stderr=STDOUT)
+                fuser = execWithCapture("fuser", ["-vm", mnt])
                 logger.debug("fuser -vm:\n%s\n", fuser)
             sleep(retrysleep)
         else:
@@ -155,9 +150,9 @@ def copytree(src, dest, preserve=True):
     links, acls, sparse files, xattrs, selinux contexts, etc.
     If preserve is False, uses cp -R (useful for modeless filesystems)'''
     logger.debug("copytree %s %s", src, dest)
-    chdir = lambda: os.chdir(src)
     cp = ["cp", "-a"] if preserve else ["cp", "-R", "-L"]
-    check_call(cp + [".", os.path.abspath(dest)], preexec_fn=chdir)
+    cp += [".", os.path.abspath(dest)]
+    execWithRedirect(cp[0], cp[1:], cwd=src)
 
 def do_grafts(grafts, dest, preserve=True):
     '''Copy each of the items listed in grafts into dest.
@@ -256,9 +251,7 @@ class PartitionMount(object):
         # kpartx -p p -v -a /tmp/diskV2DiCW.im
         # add map loop2p1 (253:2): 0 3481600 linear /dev/loop2 2048
         # add map loop2p2 (253:3): 0 614400 linear /dev/loop2 3483648
-        cmd = [ "kpartx", "-v", "-p", "p", "-a", self.disk_img ]
-        logger.debug(cmd)
-        kpartx_output = check_output(cmd)
+        kpartx_output = execWithCapture("kpartx", ["-v", "-p", "p", "-a", self.disk_img])
         logger.debug(kpartx_output)
 
         # list of (deviceName, sizeInBytes)
@@ -296,7 +289,7 @@ class PartitionMount(object):
             umount( self.mount_dir )
             os.rmdir(self.mount_dir)
             self.mount_dir = None
-        call(["kpartx", "-d", self.disk_img])
+        execWithRedirect("kpartx", ["-d", self.disk_img])
 
 
 ######## Functions for making filesystem images ##########################
@@ -312,7 +305,7 @@ def mkfsimage(fstype, rootdir, outfile, size=None, mkfsargs=[], mountargs="", gr
         size = estimate_size(rootdir, graft, fstype)
     with LoopDev(outfile, size) as loopdev:
         try:
-            check_output(["mkfs.%s" % fstype] + mkfsargs + [loopdev])
+            execWithRedirect("mkfs.%s" % fstype, mkfsargs + [loopdev])
         except CalledProcessError as e:
             logger.error("mkfs exited with a non-zero return code: %d" % e.returncode)
             logger.error(e.output)
diff --git a/src/pylorax/ltmpl.py b/src/pylorax/ltmpl.py
index 0edd5ab..8032213 100644
--- a/src/pylorax/ltmpl.py
+++ b/src/pylorax/ltmpl.py
@@ -25,11 +25,12 @@ logger = logging.getLogger("pylorax.ltmpl")
 
 import os, re, glob, shlex, fnmatch
 from os.path import basename, isdir
-from subprocess import check_call, check_output, CalledProcessError, STDOUT
+from subprocess import CalledProcessError
 
 from sysutils import joinpaths, cpfile, mvfile, replace, remove
 from yumhelper import * # Lorax*Callback classes
 from base import DataHolder
+from pylorax.executils import execWithRedirect, execWithCapture
 
 from mako.lookup import TemplateLookup
 from mako.exceptions import text_error_template
@@ -368,9 +369,10 @@ class LoraxTemplateRunner(object):
         '''
         if outfile is None:
             outfile = self._out("etc/gconf/gconf.xml.defaults")
-        check_call(["gconftool-2", "--direct",
+        cmd = ["gconftool-2", "--direct",
                     "--config-source=xml:readwrite:%s" % outfile,
-                    "--set", "--type", keytype, path, value])
+                    "--set", "--type", keytype, path, value]
+        execWithRedirect(cmd[0], cmd[1:])
 
     def log(self, msg):
         '''
@@ -404,16 +406,15 @@ class LoraxTemplateRunner(object):
                 remove ${f}
             %endfor
         '''
-        chdir = lambda: None
+        cwd = None
         cmd = cmdlist
         logger.debug('running command: %s', cmd)
         if cmd[0].startswith("--chdir="):
-            dirname = cmd[0].split('=',1)[1]
-            chdir = lambda: os.chdir(dirname)
+            cwd = cmd[0].split('=',1)[1]
             cmd = cmd[1:]
 
         try:
-            output = check_output(cmd, preexec_fn=chdir, stderr=STDOUT)
+            output = execWithCapture(cmd[0], cmd[1:], cwd=cwd)
             if output:
                 logger.debug('command output:\n%s', output)
             logger.debug("command finished successfully")
@@ -550,6 +551,7 @@ class LoraxTemplateRunner(object):
                      '--quiet', cmd)
         # XXX for some reason 'systemctl enable/disable' always returns 1
         try:
-            check_call(systemctl + units)
+            cmd = systemctl + units
+            execWithRedirect(cmd[0], cmd[1:])
         except CalledProcessError:
             pass
diff --git a/src/pylorax/sysutils.py b/src/pylorax/sysutils.py
index d35906c..52f849a 100644
--- a/src/pylorax/sysutils.py
+++ b/src/pylorax/sysutils.py
@@ -32,6 +32,7 @@ import glob
 import shutil
 import subprocess
 
+from pylorax.executils import execWithRedirect
 
 def joinpaths(*args, **kwargs):
     path = os.path.sep.join(args)
@@ -105,4 +106,5 @@ def remove(target):
         os.unlink(target)
 
 def linktree(src, dst):
-    subprocess.check_call(["/bin/cp", "-al", src, dst])
+    execWithRedirect("/bin/cp", ["-al", src, dst])
+
diff --git a/src/pylorax/treebuilder.py b/src/pylorax/treebuilder.py
index 3061f15..460d742 100644
--- a/src/pylorax/treebuilder.py
+++ b/src/pylorax/treebuilder.py
@@ -22,13 +22,13 @@ logger = logging.getLogger("pylorax.treebuilder")
 
 import os, re
 from os.path import basename, isdir
-from subprocess import check_call, check_output
 
 from sysutils import joinpaths, remove
 from shutil import copytree, copy2
 from base import DataHolder
 from ltmpl import LoraxTemplateRunner
 import imgutils
+from pylorax.executils import execWithRedirect, execWithCapture
 
 templatemap = {
     'i386':    'x86.tmpl',
@@ -45,7 +45,8 @@ templatemap = {
 
 def generate_module_info(moddir, outfile=None):
     def module_desc(mod):
-        return check_output(["modinfo", "-F", "description", mod]).strip()
+        output = execWithCapture("modinfo", ["-F", "description", mod])
+        return output.strip()
     def read_module_set(name):
         return set(l.strip() for l in open(joinpaths(moddir,name)) if ".ko" in l)
     modsets = {'scsi':read_module_set("modules.block"),
@@ -148,7 +149,7 @@ class RuntimeBuilder(object):
         for kver in os.listdir(moddir):
             ksyms = joinpaths(root, "boot/System.map-%s" % kver)
             logger.info("doing depmod and module-info for %s", kver)
-            check_call(["depmod", "-a", "-F", ksyms, "-b", root, kver])
+            execWithRedirect("depmod", ["-a", "-F", ksyms, "-b", root, kver])
             generate_module_info(moddir+kver, outfile=moddir+"module-info")
 
     def create_runtime(self, outfile="/tmp/squashfs.img", compression="xz", compressargs=[], size=1):
@@ -165,8 +166,8 @@ class RuntimeBuilder(object):
         # Reset selinux context on new rootfs
         with imgutils.LoopDev( joinpaths(workdir, "LiveOS/rootfs.img") ) as loopdev:
             with imgutils.Mount(loopdev) as mnt:
-                cmd = ["chroot", mnt, "setfiles", "-e", "/proc", "-e", "/sys", "-e", "/dev", "-e", "/selinux", "/etc/selinux/targeted/contexts/files/file_contexts", "/"]
-                check_call(cmd)
+                cmd = [ "setfiles", "-e", "/proc", "-e", "/sys", "-e", "/dev", "-e", "/selinux", "/etc/selinux/targeted/contexts/files/file_contexts", "/"]
+                execWithRedirect(cmd[0], cmd[1:], root=mnt)
 
         # squash the live rootfs and clean up workdir
         imgutils.mksquashfs(workdir, outfile, compression, compressargs)
@@ -206,8 +207,8 @@ class TreeBuilder(object):
             if backup:
                 initrd = joinpaths(self.vars.inroot, kernel.initrd.path)
                 os.rename(initrd, initrd + backup)
-            check_call(["chroot", self.vars.inroot] + \
-                       dracut + [kernel.initrd.path, kernel.version])
+            cmd = dracut + [kernel.initrd.path, kernel.version]
+            execWithRedirect(cmd[0], cmd[1:], root=self.vars.inroot)
         os.unlink(joinpaths(self.vars.inroot,"/proc/modules"))
 
     def build(self):
@@ -220,7 +221,7 @@ class TreeBuilder(object):
         for section, data in self.treeinfo_data.items():
             if 'boot.iso' in data:
                 iso = joinpaths(self.vars.outroot, data['boot.iso'])
-                check_call(["implantisomd5", iso])
+                execWithRedirect("implantisomd5", [iso])
 
     @property
     def dracut_hooks_path(self):
diff --git a/src/sbin/livemedia-creator b/src/sbin/livemedia-creator
index 1dfc75f..6717ed3 100755
--- a/src/sbin/livemedia-creator
+++ b/src/sbin/livemedia-creator
@@ -178,8 +178,7 @@ class IsoMountpoint(object):
         if not self.mount_dir:
             raise Exception("Error creating temporary directory")
 
-        cmd = ["mount","-o","loop",self.iso_path,self.mount_dir]
-        execWithRedirect( cmd[0], cmd[1:] )
+        execWithRedirect("mount", ["-o", "loop", self.iso_path, self.mount_dir])
 
         self.kernel = self.mount_dir+"/isolinux/vmlinuz"
         self.initrd = self.mount_dir+"/isolinux/initrd.img"
@@ -201,16 +200,14 @@ class IsoMountpoint(object):
         self.get_iso_label()
 
     def umount( self ):
-        cmd = ["umount", self.mount_dir]
-        execWithRedirect( cmd[0], cmd[1:] )
+        execWithRedirect("umount", [self.mount_dir])
         os.rmdir( self.mount_dir )
 
     def get_iso_label( self ):
         """
         Get the iso's label using isoinfo
         """
-        cmd = [ "isoinfo", "-d", "-i", self.iso_path ]
-        isoinfo_output = execWithCapture( cmd[0], cmd[1:] )
+        isoinfo_output = execWithCapture("isoinfo", ["-d", "-i", self.iso_path])
         log.debug( isoinfo_output )
         for line in isoinfo_output.splitlines():
             if line.startswith("Volume id: "):
@@ -491,9 +488,8 @@ def make_livecd( disk_img, squashfs_args="", templatedir=None,
         # Link /images to work_dir/images to make the templates happy
         if os.path.islink( joinpaths( installroot, "images" ) ):
             os.unlink( joinpaths( installroot, "images" ) )
-        cmd = ["/bin/ln", "-s", joinpaths(work_dir, "images"),
-               joinpaths(installroot, "images")]
-        execWithRedirect(cmd[0], cmd[1:])
+        execWithRedirect("/bin/ln", ["-s", joinpaths(work_dir, "images"),
+                                     joinpaths(installroot, "images")])
 
         isolabel = isolabel or "{0.name} {0.version} {1.basearch}".format(product, arch)
         if len(isolabel) > 32:
-- 
1.7.7.6



More information about the anaconda-patches mailing list