From 12ad7d0d9987227d327411f7f674446ba00a3879 Mon Sep 17 00:00:00 2001 From: "Brian C. Lane" Date: Wed, 22 Aug 2012 11:09:37 -0700 Subject: [PATCH] restore CalledProcessError handling When I switched execution over to execWith* functions I failed to account for the use of CalledProcessError in various places. This patch restores that behavior. All places that used check_call or check_output now pass raise_err=True to the execWith* call. --- src/pylorax/__init__.py | 2 +- src/pylorax/imgutils.py | 30 +++++++++++++----------------- src/pylorax/ltmpl.py | 6 +++--- src/pylorax/sysutils.py | 2 +- src/pylorax/treebuilder.py | 10 +++++----- 5 files changed, 23 insertions(+), 27 deletions(-) diff --git a/src/pylorax/__init__.py b/src/pylorax/__init__.py index 5c1d2a9c..850350e1 100644 --- a/src/pylorax/__init__.py +++ b/src/pylorax/__init__.py @@ -141,7 +141,7 @@ class Lorax(BaseLoraxClass): if domacboot: try: - execWithRedirect("rpm", ["-q", "hfsplus-tools"]) + execWithRedirect("rpm", ["-q", "hfsplus-tools"], raise_err=True) except CalledProcessError: logger.critical("you need to install hfsplus-tools to create mac images") sys.exit(1) diff --git a/src/pylorax/imgutils.py b/src/pylorax/imgutils.py index 6a6c40ff..cbd4de88 100644 --- a/src/pylorax/imgutils.py +++ b/src/pylorax/imgutils.py @@ -69,7 +69,7 @@ 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 = execWithCapture("losetup", ["--find", "--show", outfile]) + dev = execWithCapture("losetup", ["--find", "--show", outfile], raise_err=True) return dev.strip() def loop_detach(loopdev): @@ -79,7 +79,7 @@ def loop_detach(loopdev): def get_loop_name(path): '''Return the loop device associated with the path. Raises RuntimeError if more than one loop is associated''' - buf = execWithCapture("losetup", ["-j", path]) + buf = execWithCapture("losetup", ["-j", path], raise_err=True) if len(buf.splitlines()) > 1: # there should never be more than one loop device listed raise RuntimeError("multiple loops associated with %s" % path) @@ -93,7 +93,7 @@ def dm_attach(dev, size, name=None): if name is None: name = tempfile.mktemp(prefix="lorax.imgutils.", dir="") execWithRedirect("dmsetup", ["create", name, "--table", - "0 %i linear %s 0" % (size/512, dev)]) + "0 %i linear %s 0" % (size/512, dev)], raise_err=True) return name def dm_detach(dev): @@ -114,7 +114,7 @@ def mount(dev, opts="", mnt=None): if opts: mount += ["-o", opts] mount += [dev, mnt] - execWithRedirect(mount[0], mount[1:]) + execWithRedirect(mount[0], mount[1:], raise_err=True) return mnt def umount(mnt, lazy=False, maxretry=3, retrysleep=1.0): @@ -127,18 +127,13 @@ def umount(mnt, lazy=False, maxretry=3, retrysleep=1.0): count = 0 while maxretry > 0: try: - rv = execWithRedirect(umount[0], umount[1:]) - except Exception: - # execWithRedirect will log what the errors was, so we can just - # ignore it and retry. - pass - - if rv != 0: + rv = execWithRedirect(umount[0], umount[1:], raise_err=True) + except CalledProcessError: count += 1 if count == maxretry: raise - logger.warn("failed to unmount %s. retrying (%d/%d)...", - mnt, count, maxretry) + logger.warn("failed to unmount %s (%d). retrying (%d/%d)...", + mnt, rv, count, maxretry) if logger.getEffectiveLevel() <= logging.DEBUG: fuser = execWithCapture("fuser", ["-vm", mnt]) logger.debug("fuser -vm:\n%s\n", fuser) @@ -153,11 +148,12 @@ def umount(mnt, lazy=False, maxretry=3, retrysleep=1.0): def copytree(src, dest, preserve=True): '''Copy a tree of files using cp -a, thus preserving modes, timestamps, links, acls, sparse files, xattrs, selinux contexts, etc. - If preserve is False, uses cp -R (useful for modeless filesystems)''' + If preserve is False, uses cp -R (useful for modeless filesystems) + raises CalledProcessError if copy fails.''' logger.debug("copytree %s %s", src, dest) cp = ["cp", "-a"] if preserve else ["cp", "-R", "-L"] cp += [".", os.path.abspath(dest)] - execWithRedirect(cp[0], cp[1:], cwd=src) + execWithRedirect(cp[0], cp[1:], cwd=src, raise_err=True) def do_grafts(grafts, dest, preserve=True): '''Copy each of the items listed in grafts into dest. @@ -256,7 +252,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 - kpartx_output = execWithCapture("kpartx", ["-v", "-p", "p", "-a", self.disk_img]) + kpartx_output = execWithCapture("kpartx", ["-v", "-p", "p", "-a", self.disk_img], raise_err=True) logger.debug(kpartx_output) # list of (deviceName, sizeInBytes) @@ -310,7 +306,7 @@ def mkfsimage(fstype, rootdir, outfile, size=None, mkfsargs=[], mountargs="", gr size = estimate_size(rootdir, graft, fstype) with LoopDev(outfile, size) as loopdev: try: - execWithRedirect("mkfs.%s" % fstype, mkfsargs + [loopdev]) + execWithRedirect("mkfs.%s" % fstype, mkfsargs + [loopdev], raise_err=True) 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 80322132..d23de5c2 100644 --- a/src/pylorax/ltmpl.py +++ b/src/pylorax/ltmpl.py @@ -372,7 +372,7 @@ class LoraxTemplateRunner(object): cmd = ["gconftool-2", "--direct", "--config-source=xml:readwrite:%s" % outfile, "--set", "--type", keytype, path, value] - execWithRedirect(cmd[0], cmd[1:]) + execWithRedirect(cmd[0], cmd[1:], raise_err=True) def log(self, msg): ''' @@ -414,7 +414,7 @@ class LoraxTemplateRunner(object): cmd = cmd[1:] try: - output = execWithCapture(cmd[0], cmd[1:], cwd=cwd) + output = execWithCapture(cmd[0], cmd[1:], cwd=cwd, raise_err=True) if output: logger.debug('command output:\n%s', output) logger.debug("command finished successfully") @@ -552,6 +552,6 @@ class LoraxTemplateRunner(object): # XXX for some reason 'systemctl enable/disable' always returns 1 try: cmd = systemctl + units - execWithRedirect(cmd[0], cmd[1:]) + execWithRedirect(cmd[0], cmd[1:], raise_err=True) except CalledProcessError: pass diff --git a/src/pylorax/sysutils.py b/src/pylorax/sysutils.py index 52f849a5..1649d582 100644 --- a/src/pylorax/sysutils.py +++ b/src/pylorax/sysutils.py @@ -106,5 +106,5 @@ def remove(target): os.unlink(target) def linktree(src, dst): - execWithRedirect("/bin/cp", ["-al", src, dst]) + execWithRedirect("/bin/cp", ["-al", src, dst], raise_err=True) diff --git a/src/pylorax/treebuilder.py b/src/pylorax/treebuilder.py index 460d7428..a72ffc12 100644 --- a/src/pylorax/treebuilder.py +++ b/src/pylorax/treebuilder.py @@ -45,7 +45,7 @@ templatemap = { def generate_module_info(moddir, outfile=None): def module_desc(mod): - output = execWithCapture("modinfo", ["-F", "description", mod]) + output = execWithCapture("modinfo", ["-F", "description", mod], raise_err=True) return output.strip() def read_module_set(name): return set(l.strip() for l in open(joinpaths(moddir,name)) if ".ko" in l) @@ -149,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) - execWithRedirect("depmod", ["-a", "-F", ksyms, "-b", root, kver]) + execWithRedirect("depmod", ["-a", "-F", ksyms, "-b", root, kver], raise_err=True) generate_module_info(moddir+kver, outfile=moddir+"module-info") def create_runtime(self, outfile="/tmp/squashfs.img", compression="xz", compressargs=[], size=1): @@ -167,7 +167,7 @@ class RuntimeBuilder(object): with imgutils.LoopDev( joinpaths(workdir, "LiveOS/rootfs.img") ) as loopdev: with imgutils.Mount(loopdev) as mnt: cmd = [ "setfiles", "-e", "/proc", "-e", "/sys", "-e", "/dev", "-e", "/selinux", "/etc/selinux/targeted/contexts/files/file_contexts", "/"] - execWithRedirect(cmd[0], cmd[1:], root=mnt) + execWithRedirect(cmd[0], cmd[1:], root=mnt, raise_err=True) # squash the live rootfs and clean up workdir imgutils.mksquashfs(workdir, outfile, compression, compressargs) @@ -208,7 +208,7 @@ class TreeBuilder(object): initrd = joinpaths(self.vars.inroot, kernel.initrd.path) os.rename(initrd, initrd + backup) cmd = dracut + [kernel.initrd.path, kernel.version] - execWithRedirect(cmd[0], cmd[1:], root=self.vars.inroot) + execWithRedirect(cmd[0], cmd[1:], root=self.vars.inroot, raise_err=True) os.unlink(joinpaths(self.vars.inroot,"/proc/modules")) def build(self): @@ -221,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']) - execWithRedirect("implantisomd5", [iso]) + execWithRedirect("implantisomd5", [iso], raise_err=True) @property def dracut_hooks_path(self):