[master 1/9] Do not retry calls to close or dup2

dashea installerbot-noreply at redhat.com
Thu Jun 18 21:03:47 UTC 2015


From: David Shea <dshea at redhat.com>

Due to the details of how Linux implements closing file descriptors and
EINTR, retrying these calls is considered bad.
---
 anaconda                      |  2 +-
 pyanaconda/anaconda.py        |  2 +-
 pyanaconda/bootloader.py      | 10 +++++-----
 pyanaconda/exception.py       | 10 +++++-----
 pyanaconda/iutil.py           |  2 +-
 pyanaconda/kickstart.py       |  2 +-
 pyanaconda/vnc.py             |  4 ++--
 tests/gui/outside/__init__.py | 13 +------------
 8 files changed, 17 insertions(+), 28 deletions(-)

diff --git a/anaconda b/anaconda
index 6c3fb56..9f5447a 100755
--- a/anaconda
+++ b/anaconda
@@ -969,7 +969,7 @@ if __name__ == "__main__":
         pidfile_created = True
         pid_string = "%s\n" % os.getpid()
         iutil.eintr_retry_call(os.write, pidfile, pid_string.encode("utf-8"))
-        iutil.eintr_retry_call(os.close, pidfile)
+        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 3a91528..278bffa 100644
--- a/pyanaconda/anaconda.py
+++ b/pyanaconda/anaconda.py
@@ -202,7 +202,7 @@ def dumpState(self):
         dump_text += threads
         dump_text = dump_text.encode("utf-8")
         iutil.eintr_retry_call(os.write, fd, dump_text)
-        iutil.eintr_retry_call(os.close, fd)
+        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 1f1efac..f6e36f6 100644
--- a/pyanaconda/bootloader.py
+++ b/pyanaconda/bootloader.py
@@ -61,7 +61,7 @@ def get_boot_block(device, seek_blocks=0):
     if seek_blocks:
         os.lseek(fd, seek_blocks * block_size, 0)
     block = iutil.eintr_retry_call(os.read, fd, 512)
-    iutil.eintr_retry_call(os.close, fd)
+    os.close(fd)
     if not status:
         try:
             device.teardown(recursive=True)
@@ -1347,11 +1347,11 @@ def install(self, args=None):
                       "stage2dev": self.grub_device_name(stage2dev)})
             (pread, pwrite) = os.pipe()
             iutil.eintr_retry_call(os.write, pwrite, cmd.encode("utf-8"))
-            iutil.eintr_retry_call(os.close, pwrite)
+            os.close(pwrite)
             args = ["--batch", "--no-floppy",
                     "--device-map=%s" % self.device_map_file]
             rc = iutil.execInSysroot("grub", args, stdin=pread)
-            iutil.eintr_retry_call(os.close, pread)
+            os.close(pread)
             if rc:
                 raise BootLoaderError("boot loader install failed")
 
@@ -1540,11 +1540,11 @@ def _encrypt_password(self):
         (pread, pwrite) = os.pipe()
         passwords = "%s\n%s\n" % (self.password, self.password)
         iutil.eintr_retry_call(os.write, pwrite, passwords.encode("utf-8"))
-        iutil.eintr_retry_call(os.close, pwrite)
+        os.close(pwrite)
         buf = iutil.execWithCapture("grub2-mkpasswd-pbkdf2", [],
                                     stdin=pread,
                                     root=iutil.getSysroot())
-        iutil.eintr_retry_call(os.close, pread)
+        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")
diff --git a/pyanaconda/exception.py b/pyanaconda/exception.py
index 3bdb1a9..2fa3661 100644
--- a/pyanaconda/exception.py
+++ b/pyanaconda/exception.py
@@ -196,11 +196,11 @@ def runDebug(self, exc_info):
             iutil.vtActivate(1)
 
         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!
+        os.dup2(0, 1)                        # reclaim stdout
+        os.dup2(0, 2)                        # reclaim stderr
+        #   ^
+        #   |
+        #   +------ dup2 is magic, I tells ya!
 
         # bring back the echo
         import termios
diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py
index e8328c1..1ddf342 100644
--- a/pyanaconda/iutil.py
+++ b/pyanaconda/iutil.py
@@ -1288,7 +1288,7 @@ def ipmi_report(event):
     # Event data 2 & 3 - always 0x0 for us
     event_string = "0x4 0x1F 0x0 0x6f %#x 0x0 0x0\n" % event
     eintr_retry_call(os.write, fd, event_string.encode("utf-8"))
-    eintr_retry_call(os.close, fd)
+    os.close(fd)
 
     execWithCapture("ipmitool", ["sel", "add", path])
 
diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
index a490105..788950d 100644
--- a/pyanaconda/kickstart.py
+++ b/pyanaconda/kickstart.py
@@ -104,7 +104,7 @@ def run(self, chroot):
         (fd, path) = tempfile.mkstemp("", "ks-script-", scriptRoot + "/tmp")
 
         iutil.eintr_retry_call(os.write, fd, self.script.encode("utf-8"))
-        iutil.eintr_retry_call(os.close, fd)
+        os.close(fd)
         iutil.eintr_retry_call(os.chmod, path, 0o700)
 
         # Always log stdout/stderr from scripts.  Using --log just lets you
diff --git a/pyanaconda/vnc.py b/pyanaconda/vnc.py
index 2905268..fad5435 100644
--- a/pyanaconda/vnc.py
+++ b/pyanaconda/vnc.py
@@ -87,8 +87,8 @@ def setVNCPassword(self):
             rc = iutil.execWithRedirect("vncpasswd", ["-f"],
                     stdin=r, stdout=pw_file, binary_output=True, log_output=False)
 
-            iutil.eintr_retry_call(os.close, r)
-            iutil.eintr_retry_call(os.close, w)
+            os.close(r)
+            os.close(w)
 
         return rc
 
diff --git a/tests/gui/outside/__init__.py b/tests/gui/outside/__init__.py
index c563a74..e37ecdd 100644
--- a/tests/gui/outside/__init__.py
+++ b/tests/gui/outside/__init__.py
@@ -29,17 +29,6 @@
 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
        test against as well as handles creating and running that VM, inspecting
@@ -111,7 +100,7 @@ def makeDrives(self):
         """
         for (drive, size) in self.drives:
             (fd, diskimage) = tempfile.mkstemp(dir=self.tempdir)
-            eintr_retry_call(os.close, fd)
+            os.close(fd)
 
             # For now we are using qemu-img to create these files but specifying
             # sizes in blivet Size objects.  Unfortunately, qemu-img wants sizes


-- 
To view this commit on github, visit https://github.com/rhinstaller/anaconda/commit/f184fbb717f9f3933e48c5c3d0cdc5b103a63c2e


More information about the anaconda-patches mailing list