Skip to content

Commit

Permalink
Do not retry calls to close or dup2
Browse files Browse the repository at this point in the history
Due to the details of how Linux implements closing file descriptors and
EINTR, retrying these calls is considered bad.
  • Loading branch information
dashea committed Jun 18, 2015
1 parent 739b95c commit f184fbb
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 28 deletions.
2 changes: 1 addition & 1 deletion anaconda
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pyanaconda/anaconda.py
Expand Up @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions pyanaconda/bootloader.py
Expand Up @@ -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)
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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")
Expand Down
10 changes: 5 additions & 5 deletions pyanaconda/exception.py
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pyanaconda/iutil.py
Expand Up @@ -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])

Expand Down
2 changes: 1 addition & 1 deletion pyanaconda/kickstart.py
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pyanaconda/vnc.py
Expand Up @@ -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

Expand Down
13 changes: 1 addition & 12 deletions tests/gui/outside/__init__.py
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit f184fbb

Please sign in to comment.