From f184fbb717f9f3933e48c5c3d0cdc5b103a63c2e Mon Sep 17 00:00:00 2001 From: David Shea Date: Thu, 11 Jun 2015 15:50:28 -0400 Subject: [PATCH] Do not retry calls to close or dup2 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 6c3fb568595..9f5447a7eff 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 3a9152837bd..278bffa67ca 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 1f1efacc01d..f6e36f65f29 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 3bdb1a9a787..2fa366125c5 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 e8328c196a3..1ddf3429676 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 a490105ff55..788950d4874 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 2905268b133..fad5435b222 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 c563a74c2e8..e37ecdd2da5 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