[PATCH 3/3] restore CalledProcessError handling

Brian C. Lane bcl at redhat.com
Wed Aug 22 20:01:47 UTC 2012


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

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 5c1d2a9..850350e 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 6a6c40f..cbd4de8 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 8032213..d23de5c 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 52f849a..1649d58 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 460d742..a72ffc1 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):
-- 
1.7.11.4



More information about the anaconda-patches mailing list