[f21/master 1/2] Wrap interruptible system calls in a loop (#1160041)

David Shea dshea at redhat.com
Tue Nov 4 16:03:49 UTC 2014


Some system calls are interrupted by the receipt of a signal, for us
most commonly a SIGCHLD caused by a process run from a different thread,
which causes the call to return -1 and set errno to EINTR, which python
converts into an OSError. Handle this by retrying the call.
---
 anaconda                            |  6 +++---
 pyanaconda/anaconda.py              |  4 ++--
 pyanaconda/bootloader.py            | 22 +++++++++++-----------
 pyanaconda/exception.py             | 12 ++++++------
 pyanaconda/install.py               |  2 +-
 pyanaconda/iutil.py                 | 23 +++++++++++++++++------
 pyanaconda/kickstart.py             |  6 +++---
 pyanaconda/ntp.py                   |  3 ++-
 pyanaconda/packaging/livepayload.py |  6 +++---
 pyanaconda/simpleconfig.py          |  4 ++--
 pyanaconda/ui/lib/space.py          |  2 +-
 pyanaconda/users.py                 |  2 +-
 pyanaconda/vnc.py                   |  8 ++++----
 scripts/anaconda-yum                |  8 ++++----
 tests/gui/outside/__init__.py       | 14 +++++++++++++-
 15 files changed, 73 insertions(+), 49 deletions(-)

diff --git a/anaconda b/anaconda
index 667cf1f..ea5c077 100755
--- a/anaconda
+++ b/anaconda
@@ -892,11 +892,11 @@ if __name__ == "__main__":
     # Attempt to create the pidfile
     try:
         # Set all of the read/write bits and let umask make it make sense
-        pidfile = os.open(pidfile_path, os.O_WRONLY|os.O_CREAT|os.O_EXCL,
+        pidfile = iutil.eintr_retry_call(os.open, pidfile_path, os.O_WRONLY|os.O_CREAT|os.O_EXCL,
                 stat.S_IRUSR|stat.S_IWUSR|stat.S_IRGRP|stat.S_IWGRP|stat.S_IROTH|stat.S_IWOTH)
         pidfile_created = True
-        os.write(pidfile, "%s\n" % os.getpid())
-        os.close(pidfile)
+        iutil.eintr_retry_call(os.write, pidfile, "%s\n" % os.getpid())
+        iutil.eintr_retry_call(os.close, pidfile)
     except OSError as e:
         # If the failure was anything other than EEXIST during the open call,
         # just re-raise the exception
diff --git a/pyanaconda/anaconda.py b/pyanaconda/anaconda.py
index f44fd8f..57116bb 100644
--- a/pyanaconda/anaconda.py
+++ b/pyanaconda/anaconda.py
@@ -197,8 +197,8 @@ class Anaconda(object):
         dump_text = exn.traceback_and_object_dump(self)
         dump_text += threads
         dump_text = dump_text.encode("utf-8")
-        os.write(fd, dump_text)
-        os.close(fd)
+        iutil.eintr_retry_call(os.write, fd, dump_text)
+        iutil.eintr_retry_call(os.close, fd)
 
         # append to a given file
         with open("/tmp/anaconda-tb-all.log", "a+") as f:
diff --git a/pyanaconda/bootloader.py b/pyanaconda/bootloader.py
index a8f9130..d30a0f9 100644
--- a/pyanaconda/bootloader.py
+++ b/pyanaconda/bootloader.py
@@ -54,11 +54,11 @@ def get_boot_block(device, seek_blocks=0):
         except StorageError:
             return ""
     block_size = device.partedDevice.sectorSize
-    fd = os.open(device.path, os.O_RDONLY)
+    fd = iutil.eintr_retry_call(os.open, device.path, os.O_RDONLY)
     if seek_blocks:
         os.lseek(fd, seek_blocks * block_size, 0)
-    block = os.read(fd, 512)
-    os.close(fd)
+    block = iutil.eintr_retry_call(os.read, fd, 512)
+    iutil.eintr_retry_call(os.close, fd)
     if not status:
         try:
             device.teardown(recursive=True)
@@ -923,7 +923,7 @@ class BootLoader(object):
 
     def write_config_post(self):
         try:
-            os.chmod(iutil.getSysroot() + self.config_file, self.config_file_mode)
+            iutil.eintr_retry_call(os.chmod, iutil.getSysroot() + self.config_file, self.config_file_mode)
         except OSError as e:
             log.error("failed to set config file permissions: %s", e)
 
@@ -1320,12 +1320,12 @@ class GRUB(BootLoader):
                       "stage1dev": self.grub_device_name(stage1dev),
                       "stage2dev": self.grub_device_name(stage2dev)})
             (pread, pwrite) = os.pipe()
-            os.write(pwrite, cmd)
-            os.close(pwrite)
+            iutil.eintr_retry_call(os.write, pwrite, cmd)
+            iutil.eintr_retry_call(os.close, pwrite)
             args = ["--batch", "--no-floppy",
                     "--device-map=%s" % self.device_map_file]
             rc = iutil.execInSysroot("grub", args, stdin=pread)
-            os.close(pread)
+            iutil.eintr_retry_call(os.close, pread)
             if rc:
                 raise BootLoaderError("boot loader install failed")
 
@@ -1515,12 +1515,12 @@ class GRUB2(GRUB):
             raise RuntimeError("cannot encrypt empty password")
 
         (pread, pwrite) = os.pipe()
-        os.write(pwrite, "%s\n%s\n" % (self.password, self.password))
-        os.close(pwrite)
+        iutil.eintr_retry_call(os.write, pwrite, "%s\n%s\n" % (self.password, self.password))
+        iutil.eintr_retry_call(os.close, pwrite)
         buf = iutil.execWithCapture("grub2-mkpasswd-pbkdf2", [],
                                     stdin=pread,
                                     root=iutil.getSysroot())
-        os.close(pread)
+        iutil.eintr_retry_call(os.close, pread)
         self.encrypted_password = buf.split()[-1].strip()
         if not self.encrypted_password.startswith("grub.pbkdf2."):
             raise BootLoaderError("failed to encrypt boot loader password")
@@ -1541,7 +1541,7 @@ class GRUB2(GRUB):
         header.write("%s\n" % password_line)
         header.write("EOF\n")
         header.close()
-        os.chmod(users_file, 0o700)
+        iutil.eintr_retry_call(os.chmod, users_file, 0o700)
 
     def write_config(self):
         self.write_config_console(None)
diff --git a/pyanaconda/exception.py b/pyanaconda/exception.py
index 6a9b9cd..b8b8f4f 100644
--- a/pyanaconda/exception.py
+++ b/pyanaconda/exception.py
@@ -187,12 +187,12 @@ class AnacondaExceptionHandler(ExceptionHandler):
                 and self._intf_tty_num != 1:
             iutil.vtActivate(1)
 
-        os.open("/dev/console", os.O_RDWR)   # reclaim stdin
-        os.dup2(0, 1)                        # reclaim stdout
-        os.dup2(0, 2)                        # reclaim stderr
-        #   ^
-        #   |
-        #   +------ dup2 is magic, I tells ya!
+        iutil.eintr_retry_call(os.open, "/dev/console", os.O_RDWR)   # reclaim stdin
+        iutil.eintr_retry_call(os.dup2, 0, 1)                        # reclaim stdout
+        iutil.eintr_retry_call(os.dup2, 0, 2)                        # reclaim stderr
+        #                          ^
+        #                          |
+        #                          +------ dup2 is magic, I tells ya!
 
         # bring back the echo
         import termios
diff --git a/pyanaconda/install.py b/pyanaconda/install.py
index 40ead97..f6501f9 100644
--- a/pyanaconda/install.py
+++ b/pyanaconda/install.py
@@ -50,7 +50,7 @@ def _writeKS(ksdata):
         f.write(str(ksdata))
 
     # Make it so only root can read - could have passwords
-    os.chmod(path, 0o600)
+    iutil.eintr_retry_call(os.chmod, path, 0o600)
 
 def doConfiguration(storage, payload, ksdata, instClass):
     from pyanaconda.kickstart import runPostScripts
diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py
index 3f70860..bcd5b55 100644
--- a/pyanaconda/iutil.py
+++ b/pyanaconda/iutil.py
@@ -437,7 +437,7 @@ def _sigchld_handler(num=None, frame=None):
 
     for child_pid in _forever_pids:
         try:
-            pid_result, status = os.waitpid(child_pid, os.WNOHANG)
+            pid_result, status = eintr_retry_call(os.waitpid, child_pid, os.WNOHANG)
         except OSError as e:
             if e.errno == errno.ECHILD:
                 continue
@@ -724,7 +724,7 @@ def dracut_eject(device):
 
         f.write("eject %s\n" % (device,))
         f.close()
-        os.chmod(DRACUT_SHUTDOWN_EJECT, 0o755)
+        eintr_retry_call(os.chmod, DRACUT_SHUTDOWN_EJECT, 0o755)
         log.info("Wrote dracut shutdown eject hook for %s", device)
     except (IOError, OSError) as e:
         log.error("Error writing dracut shutdown eject hook for %s: %s", device, e)
@@ -985,11 +985,11 @@ def chown_dir_tree(root, uid, gid, from_uid_only=None, from_gid_only=None):
             return
 
         # UID and GID matching or not required
-        os.chown(path, uid, gid)
+        eintr_retry_call(os.chown, path, uid, gid)
 
     if not from_uid_only and not from_gid_only:
         # the easy way
-        dir_tree_map(root, lambda path: os.chown(path, uid, gid))
+        dir_tree_map(root, lambda path: eintr_retry_call(os.chown, path, uid, gid))
     else:
         # conditional chown
         dir_tree_map(root, lambda path: conditional_chown(path, uid, gid,
@@ -1184,9 +1184,20 @@ def ipmi_report(event):
     # Sensor num - passed in event
     # Event dir & type - always 0x0 for anaconda's purposes
     # Event data 1, 2, 3 - 0x0 for now
-    os.write(fd, "0x4 0x1F %#x 0x0 0x0 0x0 0x0\n" % event)
-    os.close(fd)
+    eintr_retry_call(os.write, fd, "0x4 0x1F %#x 0x0 0x0 0x0 0x0\n" % event)
+    eintr_retry_call(os.close, fd)
 
     execWithCapture("ipmitool", ["sel", "add", path])
 
     os.remove(path)
+
+# Copied from python's subprocess.py
+def eintr_retry_call(func, *args):
+    """Retry an interruptible system call if interrupted."""
+    while True:
+        try:
+            return func(*args)
+        except (OSError, IOError) as e:
+            if e.errno == errno.EINTR:
+                continue
+            raise
diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
index 9277c59..84818b9 100644
--- a/pyanaconda/kickstart.py
+++ b/pyanaconda/kickstart.py
@@ -95,9 +95,9 @@ class AnacondaKSScript(KSScript):
 
         (fd, path) = tempfile.mkstemp("", "ks-script-", scriptRoot + "/tmp")
 
-        os.write(fd, self.script)
-        os.close(fd)
-        os.chmod(path, 0o700)
+        iutil.eintr_retry_call(os.write, fd, self.script)
+        iutil.eintr_retry_call(os.close, fd)
+        iutil.eintr_retry_call(os.chmod, path, 0o700)
 
         # Always log stdout/stderr from scripts.  Using --log just lets you
         # pick where it goes.  The script will also be logged to program.log
diff --git a/pyanaconda/ntp.py b/pyanaconda/ntp.py
index f0c051b..40263ae 100644
--- a/pyanaconda/ntp.py
+++ b/pyanaconda/ntp.py
@@ -31,6 +31,7 @@ import ntplib
 import socket
 
 from pyanaconda import isys
+from pyanaconda import iutil
 from pyanaconda.threads import threadMgr, AnacondaThread
 from pyanaconda.constants import THREAD_SYNC_TIME_BASENAME
 
@@ -157,7 +158,7 @@ def save_servers_to_config(servers, conf_file_path=NTP_CONFIG_FILE,
             stat = os.stat(conf_file_path)
             # Use copy rather then move to get the correct selinux context
             shutil.copy(temp_path, conf_file_path)
-            os.chmod(conf_file_path, stat.st_mode)
+            iutil.eintr_retry_call(os.chmod, conf_file_path, stat.st_mode)
             os.unlink(temp_path)
 
         except OSError as oserr:
diff --git a/pyanaconda/packaging/livepayload.py b/pyanaconda/packaging/livepayload.py
index d6db00b..729d40c 100644
--- a/pyanaconda/packaging/livepayload.py
+++ b/pyanaconda/packaging/livepayload.py
@@ -80,7 +80,7 @@ class LiveImagePayload(ImagePayload):
         if rc != 0:
             raise PayloadInstallError("Failed to mount the install tree")
 
-        source = os.statvfs(INSTALL_TREE)
+        source = iutil.eintr_retry_call(os.statvfs, INSTALL_TREE)
         self.source_size = source.f_frsize * (source.f_blocks - source.f_bfree)
 
     def preInstall(self, packages=None, groups=None):
@@ -97,7 +97,7 @@ class LiveImagePayload(ImagePayload):
         while self.pct < 100:
             dest_size = 0
             for mnt in mountpoints:
-                mnt_stat = os.statvfs(iutil.getSysroot()+mnt)
+                mnt_stat = iutil.eintr_retry_call(os.statvfs, iutil.getSysroot()+mnt)
                 dest_size += mnt_stat.f_frsize * (mnt_stat.f_blocks - mnt_stat.f_bfree)
             if dest_size >= self._adj_size:
                 dest_size -= self._adj_size
@@ -349,7 +349,7 @@ class LiveImageKSPayload(LiveImagePayload):
                     blivet.util.mount(IMAGE_DIR+"/LiveOS/"+img_file, INSTALL_TREE,
                                       fstype="auto", options="ro")
 
-                    source = os.statvfs(INSTALL_TREE)
+                    source = iutil.eintr_retry_call(os.statvfs, INSTALL_TREE)
                     self.source_size = source.f_frsize * (source.f_blocks - source.f_bfree)
 
     def install(self):
diff --git a/pyanaconda/simpleconfig.py b/pyanaconda/simpleconfig.py
index 53617c0..47b7a95 100644
--- a/pyanaconda/simpleconfig.py
+++ b/pyanaconda/simpleconfig.py
@@ -27,7 +27,7 @@ import shutil
 import shlex
 from pipes import _safechars
 import tempfile
-from pyanaconda.iutil import upperASCII
+from pyanaconda.iutil import upperASCII, eintr_retry_call
 
 def unquote(s):
     return ' '.join(shlex.split(s))
@@ -121,7 +121,7 @@ class SimpleConfigFile(object):
             else:
                 m = int('0100644', 8)
             shutil.move(tmpf.name, filename)
-            os.chmod(filename, m)
+            eintr_retry_call(os.chmod, filename, m)
         else:
             # write directly to the file
             with open(filename, "w") as fobj:
diff --git a/pyanaconda/ui/lib/space.py b/pyanaconda/ui/lib/space.py
index 3251ba6..a133c8b 100644
--- a/pyanaconda/ui/lib/space.py
+++ b/pyanaconda/ui/lib/space.py
@@ -102,7 +102,7 @@ class DirInstallSpaceChecker(FileSystemSpaceChecker):
                             in the info bar at the bottom of a Hub.
         """
         self.reset()
-        stat = os.statvfs(iutil.getSysroot())
+        stat = iutil.eintr_retry_call(os.statvfs, iutil.getSysroot())
         free = Size(stat.f_bsize * stat.f_bfree)
         needed = self.payload.spaceRequired
         log.info("fs space: %s  needed: %s", free, needed)
diff --git a/pyanaconda/users.py b/pyanaconda/users.py
index 951c4e7..756a652 100644
--- a/pyanaconda/users.py
+++ b/pyanaconda/users.py
@@ -219,7 +219,7 @@ class Users:
     def _finishChroot(self, childpid):
         assert childpid > 0
         try:
-            status = os.waitpid(childpid, 0)[1]
+            status = iutil.eintr_retry_call(os.waitpid, childpid, 0)[1]
         except OSError as e:
             log.critical("exception from waitpid: %s %s", e.errno, e.strerror)
             return False
diff --git a/pyanaconda/vnc.py b/pyanaconda/vnc.py
index a7e2072..a3a70a7 100644
--- a/pyanaconda/vnc.py
+++ b/pyanaconda/vnc.py
@@ -80,15 +80,15 @@ class VncServer:
         """Set the vnc server password. Output to file. """
 
         r, w = os.pipe()
-        os.write(w, "%s\n" % self.password)
+        iutil.eintr_retry_call(os.write, w, "%s\n" % self.password)
 
         with open(self.pw_file, "w") as pw_file:
             # the -f option makes sure vncpasswd does not ask for the password again
             rc = iutil.execWithRedirect("vncpasswd", ["-f"],
                     stdin=r, stdout=pw_file, binary_output=True, log_output=False)
 
-            os.close(r)
-            os.close(w)
+            iutil.eintr_retry_call(os.close, r)
+            iutil.eintr_retry_call(os.close, w)
 
         return rc
 
@@ -137,7 +137,7 @@ class VncServer:
 
     def openlogfile(self):
         try:
-            fd = os.open(self.log_file, os.O_RDWR | os.O_CREAT)
+            fd = iutil.eintr_retry_call(os.open, self.log_file, os.O_RDWR | os.O_CREAT)
         except OSError as e:
             sys.stderr.write("error opening %s: %s\n", (self.log_file, e))
             fd = None
diff --git a/scripts/anaconda-yum b/scripts/anaconda-yum
index 41a4309..9c6dce1 100755
--- a/scripts/anaconda-yum
+++ b/scripts/anaconda-yum
@@ -27,7 +27,7 @@ import rpm
 import rpmUtils
 import yum
 from urlgrabber.grabber import URLGrabError
-from pyanaconda.iutil import xprogressive_delay
+from pyanaconda.iutil import xprogressive_delay, eintr_retry_call
 
 YUM_PLUGINS = ["fastestmirror", "langpacks"]
 
@@ -156,7 +156,7 @@ def run_yum_transaction(release, arch, yum_conf, install_root, ts_file, script_l
         yb.ts.clean()
 
         # Write scriptlet output to a file to be logged later
-        logfile = os.open(script_log, os.O_WRONLY|os.O_CREAT)
+        logfile = eintr_retry_call(os.open, script_log, os.O_WRONLY|os.O_CREAT)
         yb.ts.ts.scriptFd = logfile
         rpm.setLogFile(logfile)
 
@@ -183,7 +183,7 @@ def run_yum_transaction(release, arch, yum_conf, install_root, ts_file, script_l
             print("INFO: transaction complete")
         finally:
             yb.ts.close()
-            os.close(logfile)
+            eintr_retry_call(os.close, logfile)
     except YumBaseError as e:
         print("ERROR: transaction error: %s" % e)
     finally:
@@ -277,7 +277,7 @@ class RPMCallback(object):
             log_msg = msg_format % (txmbr.po,
                                     self.completed_actions,
                                     self.total_actions)
-            os.write(self.install_log, log_msg+"\n")
+            eintr_retry_call(os.write, self.install_log, log_msg+"\n")
             print("PROGRESS_INSTALL: %s" % progress_msg)
 
         try:
diff --git a/tests/gui/outside/__init__.py b/tests/gui/outside/__init__.py
index a5f5144..5c0f6db 100644
--- a/tests/gui/outside/__init__.py
+++ b/tests/gui/outside/__init__.py
@@ -26,6 +26,18 @@ import os
 import shutil
 import subprocess
 import tempfile
+import errno
+
+# Copied from python's subprocess.py
+def eintr_retry_call(func, *args):
+    """Retry an interruptible system call if interrupted."""
+    while True:
+        try:
+            return func(*args)
+        except (OSError, IOError) as e:
+            if e.errno == errno.EINTR:
+                continue
+            raise
 
 class Creator(object):
     """A Creator subclass defines all the parameters for making a VM to run a
@@ -98,7 +110,7 @@ class Creator(object):
         """
         for (drive, size) in self.drives:
             (fd, diskimage) = tempfile.mkstemp(dir=self.tempdir)
-            os.close(fd)
+            eintr_retry_call(os.close, fd)
 
             subprocess.call(["/usr/bin/qemu-img", "create", "-f", "qcow2", diskimage, "%sG" % size],
                             stdout=open("/dev/null", "w"))
-- 
2.1.0



More information about the anaconda-patches mailing list