From 134eec24d519341aee42f0f80f3264012fcea991 Mon Sep 17 00:00:00 2001 From: "Brian C. Lane" Date: Fri, 27 Jul 2012 07:29:34 -0700 Subject: [PATCH] clean up command execution 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 cf48f0e3..5c1d2a9c 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 f1be30c0..2845509f 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 5ec6e8af..8da4b8cb 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 0edd5aba..80322132 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 d35906c5..52f849a5 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 3061f154..460d7428 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 9d782bad..04b8ac5b 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: